Skip to content

Conversation

@ymeine
Copy link

@ymeine ymeine commented Apr 30, 2025

Fixes #19730

Here are a couple of points to consider:

Testing

While developing, I manually tested with both an ESM configuration file (vite.config.ts) and a CommonJS one (vite.config.cts).

Given the fact I directly executed vite\packages\vite\bin\vite.js, I could not test that way with Deno: it requires a stronger Node.js compatibility layer which is enabled only when executing a “real” npm package. So I tried with with file system links in node_modules and deno run -A npm:vite but still no luck. I tried an installation from a tarball after packing the package, again did not work. I am out of ideas in the short term. I assume it will work as in Node.js.

I had some struggles testing with Bun, since it worked with the dev version of Vite (pnpm run dev) but not the prod one (pnpm run build): checking the files output, there is indeed a strong difference between both versions.

Node.js worked all the time, which is the most important requirement here (especially since Deno and Bun support native TypeScript, so using --configLoader native should work fine).

Note that when I ran it on my machine, the full test suite passed.

Design

First of all, why supporting an import.meta shim in a CommonJS context? Well, on one hand the more the better (here I mean “more APIs”), but in fact it turns out that some Vite internal code being bundled still contains some import.meta.url references, even in a CommonJS context. Therefore I had to keep supporting some features of import.meta. So in the end, and to keep it simpler, more consistent and more powerful, the full import.meta shim is available in both ESM and CommonJS, with the same API. What’s nice too is that the implementation of the shim was CommonJS compatible anyways.

Now, regarding the shim API and its implementation, I went through different phases:

  • I initially cherry picked the properties to implement, to dynamically generate them based on the actual keys present on import.meta in the runtime invoked to perform the configuration bundling
    • but there were some inconsistencies in the process (Object.keys(import.meta) returns an empty list in Bun)
    • and I was not sure whether the runtime bundling the configuration file would always be the same as the runtime executing it
  • then I went on thinking “let’s generate a real Proxy instance, and dynamically invoke the shim implementation”
    • but that meant having the implementation injected in the bundle anyways, including the builtin imports from node: that it required
  • so then I was thinking, since all the code must be injected, let’s simplify and make the shim the same regardless of the runtime, since they are all compatible
    • Node.js and Deno have the same API for import.meta, with is a small set of properties and functions
    • Bun has a superset of it

So in the end I went on with the Bun API, which is the most complete.

Code style

I eventually chose to move this implementation in a dedicated file under a config/ folder. The config.ts file is already huge (more than 2000 lines of code), and, having several top level entities for the implementation, I did not want to complexify their naming to make sure they do not clash with the rest of the code and that they are identified as part of the same functional unit.

For the code itself, I went with a class, and it could easily be converted to objects and functions if really preferred. However the class felt more natural for me in this case, though in my initial implementation (more complex, with polymorphism, etc.) it had much more advantages than it has now.

The static part of the class contains elements used to configure the bundle as a whole, while the instance part contains elements to customize a given module being added to the bundle, and therefore is based on the path of that module.

Also, it gathers everything under a single imported entity (the class), which also helps with clarity and conciseness in config.ts.

What next?

I think this version is already a good start to know whether this is going to the right direction or not. Topics like adding tests or updating documentation would come only if this is adopted, otherwise it would be a loss of time right now.

Copy link
Member

@sapphi-red sapphi-red left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I noticed that I forgot to submit the review...

file: ${fileBasename},
url: ${fileUrl},
get env() { return ${varProcess}.env },
resolve(...args) { return ${varRequire}.resolve(...args) },
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

require.resolve and import.meta.resolve works differently, so we cannot do this.
require.resolve uses the require condition, while import.meta.resolve uses the import condition.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW #20962 solves this problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Provide an unaltered/corrected import.meta object in vite.config.ts and imported files

2 participants