Skip to content
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

Vite >=3.1.5 breaks Jotai babel plugins #10430

Closed
7 tasks done
gunters63 opened this issue Oct 11, 2022 · 7 comments · Fixed by #10528
Closed
7 tasks done

Vite >=3.1.5 breaks Jotai babel plugins #10430

gunters63 opened this issue Oct 11, 2022 · 7 comments · Fixed by #10528
Labels
p4-important Violate documented behavior or significantly improves performance (priority)

Comments

@gunters63
Copy link

Describe the bug

Vite 3.1.5 changed the processing of package exports. This breaks two Jotai plugins (debug-label and react-refresh).

vite dev chokes with the following error:

image

Reverting to 3.1.4 makes the error go away.

The author of Jotai thinks this may be a vite problem, not a problem with Jotai itself.

See discussion with initial analysis here: pmndrs/jotai#1475

Reproduction

https://github.com/gunters63/jotei-vite-import-error

Steps to reproduce

pnpm install, then pnpm dev

System Info

System:
    OS: Windows 10 10.0.22621
    CPU: (16) x64 AMD Ryzen 9 6900HS with Radeon Graphics        
    Memory: 6.27 GB / 31.26 GB
  Binaries:
    Node: 18.10.0 - C:\Program Files\nodejs\node.EXE
    npm: 8.19.2 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Edge: Spartan (44.22621.608.0), Chromium (106.0.1370.37)
    Internet Explorer: 11.0.22621.1
  npmPackages:
    vite: 3.1.7 => 3.1.4

Used Package Manager

pnpm

Logs

No response

Validations

@frsimond
Copy link

frsimond commented Oct 11, 2022

About this, isn't there a problem with the versioning on npm ?
The CHANGELOG.md mentions last stable to be 3.1.3. But on npm there are 3.1.4 to 3.1.7

@sapphi-red sapphi-red added the p4-important Violate documented behavior or significantly improves performance (priority) label Oct 17, 2022
sapphi-red added a commit to sapphi-red/vite that referenced this issue Oct 19, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Nov 4, 2022
@aleclarson aleclarson reopened this Nov 13, 2022
@vitejs vitejs unlocked this conversation Nov 13, 2022
@aleclarson
Copy link
Member

aleclarson commented Nov 13, 2022

It appears this is still an issue, as #10528 and #10683 only affect config bundling.

/cc @sapphi-red

@aleclarson
Copy link
Member

aleclarson commented Nov 13, 2022

I think the plan is to allow users to omit "module" from the resolve.conditions option in order to prevent it from being used (assuming I fully understand the intent of #10683).

In my opinion, Vite shouldn't use the module condition by default, and users can add it to resolve.conditions if they depend on a misconfigured package.

/cc @Dunqing @benmccann (since you two had opinions in #10442)

@Dunqing
Copy link
Contributor

Dunqing commented Nov 14, 2022

I think the plan is to allow users to omit "module" from the resolve.conditions option in order to prevent it from being used (assuming I fully understand the intent of #10683).

In my opinion, Vite shouldn't use the module condition by default, and users can add it to resolve.conditions if they depend on a misconfigured package.

I agree with you.

@bluwy
Copy link
Member

bluwy commented Nov 14, 2022

@aleclarson Isn't this issue about config bundling only? module being used in the normal Vite pipeline isn't part of this issue.

I think the plan is to allow users to omit "module" from the resolve.conditions option in order to prevent it from being used (assuming I fully understand the intent of #10683).

I don't think that's the intent of the PR. It's fixing an issue with config bundling only.

@sapphi-red
Copy link
Member

sapphi-red commented Nov 14, 2022

I think the plan is to allow users to omit "module" from the resolve.conditions option in order to prevent it from being used (assuming I fully understand the intent of #10683).

I implemented in a way that we could do like that in future. But that is not the main intent of that PR. The intent was to make module not be used when config bundling.

I think removing module condition from the normal Vite pipeline is a different thing from this issue. (IMO Vite should use module condition for isRequire: false because of the intent of this condition: webpack/webpack#11014, rollup/rollup#3514, https://github.com/rollup/plugins/tree/master/packages/node-resolve#exportconditions)

About chaging the resolve.conditions, I've left explanations about it in #10917.

@aleclarson
Copy link
Member

Isn't this issue about config bundling only?

Ah ok, my mistake!

I think the plan is to allow users to omit "module" from the resolve.conditions option in order to prevent it from being used (assuming I fully understand the intent of #10683).

I implemented in a way that we could do like that in future. But that is not the main intent of that PR. The intent was to make module not be used when config bundling.

Yeah, I worded my comment poorly. I wasn't saying that it was the only or even the main intent of #10683, but I noticed it was implied in the overrideConditions description.

@github-actions github-actions bot locked and limited conversation to collaborators Nov 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
p4-important Violate documented behavior or significantly improves performance (priority)
Projects
None yet
6 participants