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

Recent versions of vite-node fail to execute a script when ESM is imported #1208

Closed
6 tasks done
antoinerey opened this issue Apr 29, 2022 · 7 comments · Fixed by #1215
Closed
6 tasks done

Recent versions of vite-node fail to execute a script when ESM is imported #1208

antoinerey opened this issue Apr 29, 2022 · 7 comments · Fixed by #1215

Comments

@antoinerey
Copy link
Contributor

antoinerey commented Apr 29, 2022

Describe the bug

TLDR; vite-node higher than 0.8.5 fails to run a script where ESM is imported.

I'm using vite-node to execute some code that relies on several dependencies. One of those dependencies (in my case msw) provides both ESM and UMD bundles. When executing the script, vite-node imports the ESM bundle, and immediately crashes.

The script works fine when running vite-node@0.8.5, but fails with any higher version. I believe that the turning point is the c4a25ae commit, where some checks to inline dependencies where removed.

Reproduction

https://github.com/antoinerey/vite-node-reproduction-esm-import

System Info

System:
    OS: macOS 12.0.1
    CPU: (8) x64 Intel(R) Core(TM) i7-8557U CPU @ 1.70GHz
    Memory: 104.10 MB / 16.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 18.0.0 - /usr/local/bin/node
    Yarn: 1.22.17 - /usr/local/bin/yarn
    npm: 8.6.0 - /usr/local/bin/npm
    Watchman: 2022.03.21.00 - /usr/local/bin/watchman
  Browsers:
    Brave Browser: 100.1.37.116
    Chrome: 101.0.4951.41
    Firefox: 98.0.1
    Safari: 15.1
  npmPackages:
    vite-node: ^0.10.0 => 0.10.0

Used Package Manager

yarn

Validations

@sheremet-va
Copy link
Member

sheremet-va commented Apr 29, 2022

msw doesn't have "type": "module" in their package.json (or esm/package.json) (https://unpkg.com/browse/msw@0.39.2/package.json). Failing is correct behaviour. Please, ask them to fix it.

More about dual-packaging: https://github.com/sheremet-va/dual-packaging

I would recommend adding "exports" field, because "module" field is not respected.

@antoinerey
Copy link
Contributor Author

👍 Will do! Also, thanks for the dual-packaging example. In my opinion, the behaviour around CJS and ESM is quite tricky to get working, and your explanation definitely helps me!

Side question though, I'm trying to understand if the recent changes were expected or not. Since it's working with versions below 0.9.0, I'm wondering if the behaviour at that time was incorrect. In other words, was it working because vite-node was to permissive, and didn't not follow specifications?

@sheremet-va
Copy link
Member

sheremet-va commented Apr 29, 2022

Previously vite-node transformed files that didn't need transformation, because it was a valid ESM code inside node_modules. It just assumed that the code will not be a correct ESM, because it is usually wrong in the Node ecosystem :) But at some point we added isValidESM check, that follows the specification and is more reliable.

We try not to transform code inside node_modules for performance reasons.

We have two important options to fix this

  • fallbackCJS: true will try to find cjs versions of the file and load it instead of rellying on potentially wrong esm output
  • deps.inline will just always process files to make them compatible

@sheremet-va
Copy link
Member

sheremet-va commented Apr 29, 2022

Overall, ESM/CJS compatibility is a catastrophe :)

Because a lot of the packages relly on how bundlers treat esm (with "module" field) instead of rellying on the current standard ("exports" field with correct file extensions).

@antoinerey
Copy link
Contributor Author

Deeply appreciate the explanation! 🙇‍♂️

We have two important options to fix this

  • fallbackCJS: true will try to find cjs versions of the file and load it instead of rellying on potentially wrong esm output
  • deps.inline will just always process files to make them compatible

Is there a way to configure these for vite-node directly? Since the script does not run through vitest, I believe that everything under the test property in Vite/Vitest configuration is not taken into account, right?

@sheremet-va
Copy link
Member

Since the script does not run through vitest, I believe that everything under the test property in Vite/Vitest configuration is not taken into account, right?

Actually, you are correct! Unfortunately, right now it doesn't support this. But I don't see why we shouldn't allow it. PR welcome :)

@antoinerey
Copy link
Contributor Author

Aaaand, PR is here: #1215!

@github-actions github-actions bot locked and limited conversation to collaborators Jun 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants