-
-
Notifications
You must be signed in to change notification settings - Fork 1.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
fix: re-apply default conditions if using vite 6 or later #7071
base: main
Are you sure you want to change the base?
Conversation
3c7604d
to
7bd5042
Compare
✅ Deploy Preview for vitest-dev ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
7bd5042
to
0dd0f17
Compare
Vite 6 no longer applies default conditions when you override resolve.conditions. This PR adds them back conditionally based on the vite version. Fixes vitest-dev#7070
0dd0f17
to
379f9a8
Compare
// we add back "node" explicitly in both client and server conditions. | ||
// this ensures that the conditions we pass on to vite doesn't include | ||
// both browser and node at the same time, or node twice in the server case. | ||
return conditions.filter(c => c !== 'browser' && c !== 'node') |
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 don't love that the vite migration docs say:
For example, if you previously specified ['custom'] for resolve.conditions, you need to specify ['custom', ...defaultClientConditions] instead.
which implies that we should just be doing:
clientConditions: ['node', ...defaultClientConditions],
but there is some subtlety in that in vite 5, 'node' and 'browser' weren't actually part of the default conditions, but instead it used the browser
option for the resolve function as follows:
resolve({
...
browser: targetWeb && !additionalConditions.has('node')
conditions,
...
});
So in the case of vitest passing "node" as an additional condition it would disable the browser resolve, but if node wasn't passed in, it would infer 'browser' resolve behavior from that rather than using conditions. You couldn't previously get in a case where you had both "browser" and "node" conditions, but when just using conditions in vite 6 you can.
If you want the same behavior as before, we need to remove "browser" from vite's default conditions, and add the node condition instead.
@sheremet-va assuming this change looks good for the vitest 3.x release, is there any chance of back-patching it to 2.1.9 while waiting for the official 3.x release? |
Vitest 2 doesn’t support Vite 6 |
The 2.1.7 release notes implied a soft support:
But that was under the assumption the api's were compatible which doesn't appear to be the case. I'm ok waiting, but thought I'd ask. |
The API is compatible, only default config values have breaking changes |
Fixes #7070
Description
Vite 6 no longer applies default conditions when you override resolve.conditions. This PR adds them back conditionally based on the vite version.
Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
pnpm-lock.yaml
unless you introduce a new test example.Tests
pnpm test:ci
.Documentation
pnpm run docs
command.Changesets
feat:
,fix:
,perf:
,docs:
, orchore:
.