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

Curious behaviour when loading CJS/ESM with vite.ssrLoadModule #11385

Closed
7 tasks done
mrm007 opened this issue Dec 15, 2022 · 10 comments · Fixed by #11409
Closed
7 tasks done

Curious behaviour when loading CJS/ESM with vite.ssrLoadModule #11385

mrm007 opened this issue Dec 15, 2022 · 10 comments · Fixed by #11409
Labels
feat: ssr p4-important Violate documented behavior or significantly improves performance (priority)

Comments

@mrm007
Copy link
Contributor

mrm007 commented Dec 15, 2022

Describe the bug

After upgrading to Vite 4 I am no longer able to consume certain packages, but the same packages worked in Vite 3:

See linked repo for instructions and reproduction.


But investigating a bit further why it fails with @vanilla-extract/css/adapter I've identified a curious behaviour.

I've recorded a CodeTour in the linked repo which explains the behaviour (requires VS Code).

TL;DR:

  • Vite 4 resolves @vanilla-extract/css/adapter main entry point (defined in package.json#exports) using the module condition (./dist/vanilla-extract-css-adapter.esm.mjs), then reverts to package.json#main (./dist/vanilla-extract-css-adapter.cjs.js).
  • In Vite 3 @vanilla-extract/css/adapter is resolved to ./dist/vanilla-extract-css-adapter.cjs.js using the default condition.

Reproduction

https://github.com/mrm007/vite-mjs-repro

Steps to reproduce

  1. Clone the repo
  2. Install dependencies: pnpm install
  3. Start the server with Vite 4: pnpm vite4

Instructions are also in the linked repo's README.

System Info

System:
    OS: macOS 12.5.1
    CPU: (10) arm64 Apple M1 Pro
    Memory: 323.44 MB / 32.00 GB
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 16.16.0 - ~/.volta/tools/image/node/16.16.0/bin/node
    Yarn: 1.22.19 - ~/.volta/tools/image/yarn/1.22.19/bin/yarn
    npm: 8.11.0 - ~/.volta/tools/image/node/16.16.0/bin/npm
    Watchman: 2022.12.05.00 - /opt/homebrew/bin/watchman
  Browsers:
    Chrome: 108.0.5359.124
    Edge: 108.0.1462.46
    Firefox: 108.0
    Firefox Developer Edition: 107.0
    Safari: 16.2
  npmPackages:
    vite: ^4.0.1 => 4.0.1

Used Package Manager

pnpm

Logs

With uuid:

(node:79886) Warning: To load an ES module, set "type": "module" in the package.json or use the .mjs extension.
TypeError: Cannot read properties of undefined (reading 'v4')
    at /___/vite-mjs-repro/routes/uuid.ts:3:11
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at async instantiateModule (file:///___/vite-mjs-repro/node_modules/.pnpm/[email protected]/node_modules/vite/dist/node/chunks/dep-2285ba4f.js:53234:9)

With @vanilla-extract/css:

(node:79886) Warning: To load an ES module, set "type": "module" in the package.json or use the .mjs extension.
TypeError: Cannot read properties of undefined (reading 'setAdapter')
    at /___/vite-mjs-repro/routes/vanilla.ts:3:0
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at async instantiateModule (file:///___/vite-mjs-repro/node_modules/.pnpm/[email protected]/node_modules/vite/dist/node/chunks/dep-2285ba4f.js:53234:9)

Logs are also in the linked repo's README.

Validations

@mrm007
Copy link
Contributor Author

mrm007 commented Dec 16, 2022

I just stumbled upon this comment #8484 (comment) that seems to be the curious behaviour I encountered.
@bluwy do you know why Vite 4 prefers pkg.main when the pkg.exports condition ends with .mjs?

@mrm007
Copy link
Contributor Author

mrm007 commented Dec 16, 2022

It seems the fix is in this PR comment #10504 (review)
@aleclarson is there a way to publish a snapshot/prerelease version from that branch? Something akin to this microsoft/TypeScript#51669 (comment) or this rollup/rollup#4624 (comment).

Sorry for tagging people, I'm just trying to connect all the dots and have the information in one place.

@vexkiddy
Copy link

i'm also having issues with vite 4 and uuid v4

in my service layer js

import uuid from 'uuid'
const dropId = uuid.v4()

Log from the Terminal

Cannot read properties of undefined (reading 'v4')
TypeError: Cannot read properties of undefined (reading 'v4')
    at Module.createDrop (/src/service/drop_service.js:76:19)

@nitedani
Copy link

@bluwy
Copy link
Member

bluwy commented Dec 18, 2022

I think publint is wrong on this one as Vite SSR shouldn't be using the module condition. I've created bluwy/publint#10 to track this.

This is an unintentional change from here as isRequire: false enables the module condition, which isn't compatible with nodejs.

I'll make a PR to remove this condition. I don't think it'll break packages in practice as module isn't expected to work in the first place.

I just stumbled upon this comment #8484 (comment) that seems to be the curious behaviour I encountered.
@bluwy do you know why Vite 4 prefers pkg.main when the pkg.exports condition ends with .mjs?

That's currently a bug that I tried to fix during Vite 4 beta but reverted as the ecosystem seem to had relied on the old behaviour. There's more info at #11114

@bluwy bluwy added p4-important Violate documented behavior or significantly improves performance (priority) feat: ssr and removed pending triage labels Dec 18, 2022
@mrm007
Copy link
Contributor Author

mrm007 commented Dec 18, 2022

Thanks @bluwy, very much appreciated.

The linked issue #11299 also highlights the fact that the output of many tools is incorrect, and many packages are affected. I certainly don't envy the amount of 🤬 guys you have to deal with, now that Vite is getting more and more popular.

@nhe23
Copy link

nhe23 commented Dec 21, 2022

@patak-dev @bluwy Thank you for fixing this so fast. Is there a timeline for when this fix will be released? As this bug broke probably a bunch of applications (including mine 😄) it would be great to have this fix available soon.

@patak-dev
Copy link
Member

@nhe23 released in [email protected], sorry for the regression 👍🏼

@nhe23
Copy link

nhe23 commented Dec 21, 2022

@patak-dev That was fast 😄. Thank you for your work 👍

@mrm007
Copy link
Contributor Author

mrm007 commented Dec 21, 2022

Thanks @patak-dev and @bluwy for the very speedy fix 🤝

@github-actions github-actions bot locked and limited conversation to collaborators Jan 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feat: ssr p4-important Violate documented behavior or significantly improves performance (priority)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants