feat(resolve): add support for accessing default conditions using ... in overrideConditions
#17326
Conversation
….` in `overrideConditions`
|
|
magic-akari
changed the title
... in overrideConditions... in overrideConditions
|
Actually, both |
|
ping |
| } | ||
| return true | ||
| }) | ||
| if (options.overrideConditions && additionalConditions.has('...')) { |
There was a problem hiding this comment.
Thanks for the PR.
If I remember correctly, overrideConditions is mostly used for SSR externalConditions , so in other normal cases, this line is false, so ... is not doing anything? (at least on playground/resolve's example)
vite/packages/vite/src/node/ssr/ssrModuleLoader.ts
Lines 118 to 123 in 62d2e70
If this part is fixed in any sense, I suppose this feature should be a breaking change as users need to explicitly include ... on their config somewhere to get back old behaviors. (And such breaking changes should probably go together with others ... support for extensions and mainFields).
Btw, do you have anything concrete in your mind which requires this feature on Vite? or some general resolution issue of some tricky dependencies?
There was a problem hiding this comment.
If I remember correctly,
overrideConditionsis mostly used for SSRexternalConditions, so in other normal cases, this line isfalse, so...is not doing anything? (at least on playground/resolve's example)
You're right, the overrideConditions is an unnecessary check. We simply need to verify the presence of .... That's enough.
vite/packages/vite/src/node/ssr/ssrModuleLoader.ts
Lines 118 to 123 in 62d2e70
If this part is fixed in any sense, I suppose this feature should be a breaking change as users need to explicitly include
...on their config somewhere to get back old behaviors. (And such breaking changes should probably go together with others...support forextensionsandmainFields).
No, this is not a breaking change. If there are no ... in the conditions, the logic remains unchanged.
If ... is present, the default conditions will be appended, and this is where the new logic comes into play.
The original meaning of ... is to inherit the default conditions when the user overrides conditions.
You can refer to webpack docs and webpack usage.
Btw, do you have anything concrete in your mind which requires this feature on Vite? or some general resolution issue of some tricky dependencies?
I am using it in an internal project. Switching from webpack to vite, I use conditions to pass in process.arch and process.platform to select the precompiled node native modules. During the migration process, I found the absence of ....
The workaround solution is to copy the default conditions from Vite source code.
There was a problem hiding this comment.
If this part is fixed in any sense, I suppose this feature should be a breaking change as users need to explicitly include
...on their config somewhere to get back old behaviors. (And such breaking changes should probably go together with others...support forextensionsandmainFields).No, this is not a breaking change. If there are no
...in the conditions, the logic remains unchanged. If...is present, the default conditions will be appended, and this is where the new logic comes into play.
Interesting. I think I understand that user config's conditions: ["custom"] would mean like conditions: ["...", "custom"], so the behavior is same as now. But wouldn't this make conditions: ["...", "custom"] (not sure who does this explicitly though) to mean conditions: ["custom", "..."] since it's inserting set elements again?
There was a problem hiding this comment.
I may be wrong. In Webpack, user-defined conditions will remove the default conditions, requiring you to explicitly include ... to inherit default conditions, so I supposed that Vite behaves similarly.
However, if Vite always includes default conditions (so that users cannot delete the default conditions?), then this pull request is going the wrong way.
There was a problem hiding this comment.
Yeah, I worried that if Vite always included the default ones, then that would be still inconsistent with Webpack, so it would be probably simpler for this feature to be implemented as a breaking change if we need to introduce this.
Thanks for raising the PR anyways. If there are more known use cases for this, then we can revisit this.
Switching from webpack to vite, I use conditions to pass in process.arch and process.platform to select the precompiled node native modules.
Btw, can you show how exports entries of your native dependency look like? Is it something you manually customized or generated automatically by well-known native binding system like napi-rs? If it's the latter, then it's better for us to know how to deal with it.
There was a problem hiding this comment.
Also, I was assuming that this was not about SSR, but just realizing you're talking about process.arch and process.platform, so is your case actually about SSR?
There was a problem hiding this comment.
I used napi-rs to create a Node.js module, with JavaScript bindings, and then I updated the package.json file accordingly.
{
"imports": {
"#binding": {
"bundler": {
"win32": {
"arm64": "@foo/bar-win32-arm64-msvc",
"x64": "@foo/bar-win32-x64-msvc",
"ia32": "@foo/bar-win32-ia32-msvc"
},
"darwin": {
"x64": "@foo/bar-darwin-x64",
"arm64": "@foo/bar-darwin-arm64"
},
"linux": {
"x64": {
"gnu": "@foo/bar-linux-x64-gnu",
"default": "@foo/bar-linux-x64-musl"
},
"arm64": {
"gnu": "@foo/bar-linux-arm64-gnu",
"default": "@foo/bar-linux-arm64-musl"
}
}
},
"default": "./index.js" // <- napi generated binding entry
}
}
}I use main.js as the main entry point, and its content is very simple:
module.exports = require("#binding");In general cases, the import of #binging will directly resolve to index.js, which is the glue code generated by napi-rs. It contains a large switch case to filter the correct Node module.
When I pass in the conditions, the bundler (webpack) directly filters the node modules for me, bypassing the logic of index.js.
Additionally, imports and exports follow the same logic for resolving conditions, I'm referring specifically to imports here, I hope it does not cause any confusion.
|
I close this pull request since Vite always inherits the default conditions. |
Description
mimic webpack's behavior, providing users with access to default options.
webpack docs
webpack usage