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

Vitest loads multiple formats of the same package #747

Closed
6 tasks done
marvinhagemeister opened this issue Feb 13, 2022 · 18 comments
Closed
6 tasks done

Vitest loads multiple formats of the same package #747

marvinhagemeister opened this issue Feb 13, 2022 · 18 comments
Labels

Comments

@marvinhagemeister
Copy link

marvinhagemeister commented Feb 13, 2022

Describe the bug

During a test run it seems like libraries can be loaded twice when something requests a commonjs module and something else the ESM variant.

TypeError [Error]: Cannot read properties of null (reading 'context')
    at Proxy.exports.useContext (/app/node_modules/preact/hooks/dist/hooks.js:1:2437)
    at d.ExampleConsumer [as constructor] (/app/app/src/preact-bug.test.jsx:38:41)
    at d.M [as render] (/app/node_modules/preact/dist/preact.js:1:7837)
    at j (file:///app/node_modules/preact/dist/preact.mjs:1:5752)
    at file:///app/node_modules/preact/dist/preact.mjs:1:1494
    at Array.some (<anonymous>)
    at g (file:///app/node_modules/preact/dist/preact.mjs:1:1392)

As indicated by the stack trace, somehow preact.mjs AND preact.js are loaded. This sounds like vite or vitest is including both a CommonJS and an ESM Module of the same library.

(Originally reported at preactjs/preact#3220)

Reproduction

  1. Clone https://github.com/dancras/preact-bug
  2. Run npm install
  3. Run npm run vitest

System Info

System:
    OS: macOS 12.1
    CPU: (8) arm64 Apple M1
    Memory: 92.44 MB / 16.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 16.6.2 - ~/.nvm/versions/node/v16.6.2/bin/node
    Yarn: 1.22.17 - /opt/homebrew/bin/yarn
    npm: 7.20.3 - ~/.nvm/versions/node/v16.6.2/bin/npm
    Watchman: 2021.10.18.00 - /opt/homebrew/bin/watchman
  Browsers:
    Brave Browser: 89.1.21.73
    Chrome: 98.0.4758.80
    Chrome Canary: 100.0.4886.0
    Edge: 98.0.1108.51
    Firefox: 96.0.3
    Safari: 15.2
  npmPackages:
    vite: ^2.8.0 => 2.8.0 
    vitest: ^0.3.2 => 0.3.2 

Used Package Manager

npm

Validations

@sheremet-va
Copy link
Member

sheremet-va commented Feb 14, 2022

To import libraries we use native import. And node doesn't understand module field, so I added exports field, and now I get another error.

  "exports": {
    ".": {
      "import": "./dist/hooks.mjs",
      "require": "./dist/hooks.js"
    }
  },
TypeError: Cannot read properties of undefined (reading '__H')
❯ m node_modules/preact/hooks/dist/hooks.module.js:110:20

❯ p node_modules/preact/hooks/dist/hooks.module.js:138:2
❯ Object.l node_modules/preact/hooks/dist/hooks.module.js:135:19
❯ d.ExampleProvider [as constructor] src/preact-bug.test.jsx:12:30
❯ d.M [as render] node_modules/preact/dist/preact.js:1:7837
❯ I node_modules/preact/dist/preact.js:1:5598
❯ m node_modules/preact/dist/preact.js:1:2096
❯ I node_modules/preact/dist/preact.js:1:5831
❯ N node_modules/preact/dist/preact.js:1:7951
❯ node_modules/@testing-library/preact/dist/pure.js:90:26
❯ exports.act node_modules/preact/test-utils/dist/testUtils.js:1:790
❯ Proxy.render node_modules/@testing-library/preact/dist/pure.js:86:22
❯ src/preact-bug.test.jsx:43:4
❯ runTest node_modules/vitest/dist/entry.js:1728:40
❯ async runSuite node_modules/vitest/dist/entry.js:1792:13
❯ async runSuites node_modules/vitest/dist/entry.js:1820:5
❯ async startTests node_modules/vitest/dist/entry.js:1825:3
❯ async node_modules/vitest/dist/entry.js:1849:7
❯ async withEnv node_modules/vitest/dist/entry.js:1479:5
❯ async run node_modules/vitest/dist/entry.js:1848:5

@marvinhagemeister
Copy link
Author

That's interesting. I would've assumed that bundlers only look at the top level package.json of a package where all the export mapping happens.

@sheremet-va
Copy link
Member

sheremet-va commented Feb 14, 2022

That's interesting. I would've assumed that bundlers only look at the top level package.json of a package where all the export mapping happens.

Vitest is not a bundler tho, it's more of a runner, and we dont use Vite to bundle code, only to transpile it. Also we don't perform Vite's transformations on node_modules for perfomance reasons, but it's possible to deps.inline them using config.

@marvinhagemeister
Copy link
Author

marvinhagemeister commented Feb 14, 2022

FYI: The issue described here seems to relate to the "dual package hazard" section in Node's module docs: https://nodejs.org/api/packages.html#dual-package-hazard

@sheremet-va
Copy link
Member

FYI: The issue described here seems to relate to the "dual package hazard" section in Node's module docs: https://nodejs.org/api/packages.html#dual-package-hazard

Yes, I think this is due to testing-library being cjs only. It requires preact to render, then user code imports hooks from mjs, where currentComponent is undefined

@sheremet-va
Copy link
Member

sheremet-va commented Feb 14, 2022

So, to fix this we need to:

Either

  • Have exports field in all of preact packages
  • AND build ESM testing-library

Or

  • Somehow import only cjs versions in Vitest of these libraries, but this is harder, since we are ESM first, and it would be quite a rewrite - maybe some kind of config option will be necessary, but I would prefer the first variant, since it will work by default for most users

@marvinhagemeister
Copy link
Author

Agree that pushing for ESM rather than moving back to CJS seems like the preferable move. Maybe if testing-library is ESM that problem will be resolved.

@sheremet-va
Copy link
Member

Agree that pushing for ESM rather than moving back to CJS seems like the preferable move. Maybe if testing-library is ESM that problem will be resolved.

I see it wasn't updated for quite some time now. Will try and create an issue there. Also, having exports is highly encouraged for native Node module resolution. That way we don't need to guess what file to load :p

@danielweck
Copy link

Yep, my Preact hook tests are broken because of this :)
I am going to try using preact/text-utils directly instead...

@userquin
Copy link
Member

userquin commented Apr 1, 2022

  • Somehow import only cjs versions in Vitest of these libraries, but this is harder, since we are ESM first, and it would be quite a rewrite - maybe some kind of config option will be necessary, but I would prefer the first variant, since it will work by default for most users

maybe this can help to load cjs https://github.com/iconify/iconify/blob/master/packages/utils/vitest.config.cjs running:
vitest --config vitest.config.cjs

@danielweck
Copy link

Ah, my workaround is to delete / rename node_modules/preact/hooks/package.json, which interferes with the top-level import map in node_modules/preact/package.json (which itself references preact/hooks)
👍

@brianblakely
Copy link

Running vitest in --no-threads mode resolved this issue for me. Although there are other issues now as a result of doing that, they seem more manageable.

@danielweck
Copy link

@danielweck
Copy link

Another update: fixed now :)
testing-library/preact-testing-library#50 (comment)

@wight554
Copy link

Another update: fixed now :) testing-library/preact-testing-library#50 (comment)

Have you managed to get everything working ootb?
Last (>3.0.2) preact-testing library updates breaks preact/hooks import for me like here: preactjs/preact#2690

@wight554
Copy link

wight554 commented May 28, 2022

Another update: fixed now :) testing-library/preact-testing-library#50 (comment)

Have you managed to get everything working ootb? Last (>3.0.2) preact-testing library updates breaks preact/hooks import for me like here: preactjs/preact#2690

for those who came with typescript 4.7 and new module resolution config:
While @testing-library/preact fully supports esm, some of modules you use may not, eg recoil, mui (which is weird actually) in my case. That leads to CJS/ESM import issues (like preact/hooks error). To workaround this, use TypeScript CJS import interop:

import tlp = require('@testing-library/preact');

const { screen, waitFor, fireEvent, cleanup } = tlp;

Another solution is to use @testing-library/preact of version 3.0.2 and lower

That did help in my case. If anyone has better solution, please share. Forcing everyone to have proper ESM support is nice, but it may take a lot of time

@danielweck
Copy link

Thanks for the tip @wight554
As mentioned previously in this thread, what worked for me is the following (but I appreciate that projects with different dependency trees might experience different issues):

Ah, my workaround is to delete / rename node_modules/preact/hooks/package.json, which interferes with the top-level import map in node_modules/preact/package.json (which itself references preact/hooks)

@sheremet-va
Copy link
Member

Fixed, according to #747 (comment)

rhengles added a commit to arijs/react-router that referenced this issue Mar 8, 2023
This fixes an issue with Preact + Vite + SSR. Without the export maps, Vite SSR and Node.js
gets confused and loads both the ESM and CJS versions of Preact. This breaks the hooks system
of Preact as discussed here[1] and here[2].

During SSR, Vite starts as ESM and loads my modules (and Preact). But when I load React-Router,
as it does not have the exports map, Node loads the CJS version, which loads the CJS version of
Preact. Then Preact breaks because it can't find the hooks registered in the ESM code.

Also, in the version of Node that I tested, v16.14.0, Node printed the warning below, so I also
changed the extension of the ES module for node from ".js" to ".mjs".

(node:5753) Warning: To load an ES module, set "type": "module" in the package.json or use the .mjs extension.

[1] preactjs/preact#3220 (comment)
[2] vitest-dev/vitest#747 (comment)
rhengles added a commit to arijs/react-router that referenced this issue Mar 9, 2023
This fixes an issue with Preact + Vite + SSR. Without the export maps, Vite SSR and Node.js
gets confused and loads both the ESM and CJS versions of Preact. This breaks the hooks system
of Preact as discussed here[1] and here[2].

During SSR, Vite starts as ESM and loads my modules (and Preact). But when I load React-Router,
as it does not have the exports map, Node loads the CJS version, which loads the CJS version of
Preact. Then Preact breaks because it can't find the hooks registered in the ESM code.

Also, in the version of Node that I tested, v16.14.0, Node printed the warning below, so I also
changed the extension of the ES module for node from ".js" to ".mjs".

(node:5753) Warning: To load an ES module, set "type": "module" in the package.json or use the .mjs extension.

[1] preactjs/preact#3220 (comment)
[2] vitest-dev/vitest#747 (comment)
@github-actions github-actions bot locked and limited conversation to collaborators Jun 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

6 participants