-
-
Notifications
You must be signed in to change notification settings - Fork 6.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(resolve)!: remove resolve.browserField
#14733
Conversation
Run & review this pull request in StackBlitz Codeflow. |
const resolveOptions: NodeImportResolveOptions = { | ||
mainFields: ['main'], | ||
browserField: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if this was a mistake, the browserField
was used in SSR resolving.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before #10071, Vite used browser field even with ssrModuleLoader
and I wanted to preserve that behavior to avoid a breaking change. I think this change makes sense.
Looks good to me. I think it would be good to add a warning if the user has |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I forgot about this thing 😅
const resolveOptions: NodeImportResolveOptions = { | ||
mainFields: ['main'], | ||
browserField: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before #10071, Vite used browser field even with ssrModuleLoader
and I wanted to preserve that behavior to avoid a breaking change. I think this change makes sense.
/ecosystem-ci run |
📝 Ran ecosystem CI on
|
Description
resolve.browserField
was introduced deprecated by default in #10071. The plan was to updateresolve.mainFields
to['browser', 'module', 'jsnext:main', 'jsnext']
which I did in this PR.This particular change can be breaking for tooling as some tools (e.g.
vite-plugin-svelte
) wants to add additionalmainFields
, and the only way to not remove the existing['module', 'jsnext:main', 'jsnext']
was to hardcode it:https://github.com/sveltejs/vite-plugin-svelte/blob/b4b4fddf4d226856aa0f27fa4f32621dfb2938bf/packages/vite-plugin-svelte/src/utils/constants.js#L1
https://github.com/sveltejs/vite-plugin-svelte/blob/b4b4fddf4d226856aa0f27fa4f32621dfb2938bf/packages/vite-plugin-svelte/src/utils/options.js#L359-L362
Additional context
Noticed this while working on #14723
What is the purpose of this pull request?