feat: use module runner to import the config

[sheremet-va]

Description

This PR implements an optional --configLoader=runner CLI flag and configLoader: 'runner' inline config property to opt-in to use the module runner instead of bundling the config file. runner only supports ESM syntax and cannot be used with CJS syntax like __filename/__dirname/module.

Some advantages:

• no need to store a temporary file
• can import ESM config even in a CJS repo (no type: module needed)
• importer directory resolved correctly because files are not bundled into a single file (important for monorepos) when an external package is imported

Benchmarking the vitest.config.ts file in the root:

• 5-6ms to call resolveConfig to construct a simple resolved config (environment requires a config, we can construct a small one just for this environment since we control all private plugins)
• 0.1ms to init plugin container
• 23ms is runner.import
  • 2ms to run the code in AsyncFunction
  • 5ms to ssrTransform
  • 16ms to transform with esbuild

Overall the new approach takes ~28-32ms and the old one takes ~14-22ms, but resolving the config is not a hot path fortunetly. We can also shave off 5-6ms if we provide a dummy config

This PR also adds an runnerImport experimental helper that creates a temporary environment with a module runner to import a single file.

[sapphi-red]

Probably the test are failing because we support __dirname / __filename in ESM files as well.

[bluwy]

This looks similar to what Astro has been doing, which starts up the Vite server and ssrLoadModules the config file, which I had always find hacky 😅 But the module runner feels trimmed down and lightweight enough that this seems better.

Question: Does the module runner handle CJS config files well?

Probably the test are failing because we support __dirname / __filename in ESM files as well.

I'd honestly love if we start to not support them 😅 Separately from this PR, maybe we should start warning if we see them and suggest using import.meta.dirname or import.meta.filename instead if supported, or fiddling with import.meta.url.

[sheremet-va]

Probably the test are failing because we support __dirname / __filename in ESM files as well.

Good catch! We can inject it with a different evaluator.

[sheremet-va]

Good catch! We can inject it with a different evaluator.

Hm, module runner only supports ESM. Injecting __dirname and __filename is not enough 🤔

To support a CJS config, we need to reimplement most of vite-node's "mix cjs-esm" code. Unless we forbid CJS altogether 😄

[sheremet-va]

I added --bundleConfig=false option to the CLI; I am open to suggestions on how we should name it. Also need to add a note somewhere in the documentation 🤔

[bluwy]

Shouldn't using the module runner be opt-in instead? IIUC there's features like __filename and CJS that won't work properly if this is used by default.

For the option name, it would be nice if it's --configLoader runner perhaps, so that we could also introduce other options like "native" too (if we want that).

[sheremet-va]

It is opt-in, bundleConfig is true by default 😄

[bluwy]

Oh didn't notice that, I thought it was false by default. It feels a bit inverted to me to "disable bundleConfig" to "enable import with module runner" though 😅

[sheremet-va]

Oh didn't notice that, I thought it was false by default. It feels a bit inverted to me to "disable bundleConfig" to "enable import with module runner" though 😅

Yeah, I agree. I just couldn't come up with a good name. I like configLoader

[sapphi-red]

Is this TODO comment still relevant?

[sheremet-va]

Open to discussion. The environment requires a config and we know every plugin that will be used - should we provide a dummy config instead to make it faster? (I mention the time in the description of the issue)

[sapphi-red]

Can we set configFile: false? It should skip trying to find a config file.

[sheremet-va]

You mean like this? https://github.com/vitejs/vite/pull/18637/files/b38365120a8c9615a19324ccac6b70573300312a#diff-11e17761d4ecfee8f8fde15c6d79b7bc0260176396a30dfd8e6f6bbaf5af4745R1712

😄

[sheremet-va]

^this still takes 5-6ms (bundle approach takes 0 for this step as it is not needed)

[sapphi-red]

Ah, so you're trying to reduce the resolveConfig call cost. I'll vote for keep using resolveConfig here than { ...thingseNeeded } as ResolvedConfig.

Maybe we can set envFile: false, cacheDir: process.cwd() and make this call only happen when needed?

vite/packages/vite/src/node/config.ts

Line 1246 in ae68958

const pkgDir = findNearestPackageData(resolvedRoot, packageCache)?.dir

That should eliminate the probably costly fs calls.

[sheremet-va]

Adding these shaved off ~1ms

[pkg-pr-new]

Open in Stackblitz

npm i https://pkg.pr.new/vite@18637

commit: a960fd4

[hi-ogawa]

If we close the environment here, dynamic import happening after the main module import throws Error: Vite module runner has been closed.. Here is a repro https://stackblitz.com/edit/93byylkl?file=index.js (and I pushed an example as test case)

Do we need to give up explicit close here and rely on proper gc instead?

(Also, dynamic import doesn't appear as dependencies. I'm not sure what is the case with bundled config though.)

[sheremet-va]

The environment also overrides global Error.captureStackTrace and close removes the override, so we would have to either disable this and patch the error if it happens or return the environment and document that it should be closed manually (or just return the close method)

dynamic import doesn't appear as dependencies

Dynamic import is not in dependencies because it wasn't executed. We can store ssrResult.dependencies as module.parsedDependencies maybe 🤷🏻

[patak-dev]

we decided to merge this feature and iterate on the edge cases later, as it is experimental, we can let the ecosystem try it out and give feedback