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 6 change in behavior with noExternal & imports that share name with node builtins #18810

Closed
7 tasks done
thebanjomatic opened this issue Nov 27, 2024 · 0 comments · Fixed by #18821
Closed
7 tasks done
Labels
feat: ssr p3-minor-bug An edge case that only affects very specific usage (priority) regression The issue only appears after a new release

Comments

@thebanjomatic
Copy link
Contributor

thebanjomatic commented Nov 27, 2024

Describe the bug

When using build.ssr: true and ssr.noExternal: true, and importing from a package with the same name as a node built-in, vite 6 appears to treat the package as external, but resolves the import path as if it was a package import.

This was happening in some nested dependencies of mine. readable_stream does require('string_decoder/') (presumably to force the import to use the package dependency over the node built-in).

From what I can guess, it seems like there is a mismatch between two bits of code in the resolution logic. One part sees the package name "string_decoder" and sees that it matches a node built-in and so it gets flagged as external, however, when the import is resolved, it sees the / at the end and resolves to the main export in the output.

Input:

import { StringDecoder } from 'string_decoder/';

Produces:

import { StringDecoder } from 'string_decoder/lib/string_decoder.js';

which is not a valid import from the node built-in, but also isn't bundled so this fails to execute at runtime if string_decoder is not present in node_modules.

In Vite 5, the string_decoder package dependency is bundled and included in the output of ./dist/index.js as expected.

Reproduction

https://stackblitz.com/edit/vitejs-vite-s9cz6r?file=src%2Findex.js&view=editor

Steps to reproduce

First, npm install followed by npm run build

Running npm run test will delete string_decoder from node_modules and then execute the output. The expectation is that string_decoder should have been bundled into the dist/index.js output, but that is not the case so running dist/index.js fails.

System Info

System:
    OS: Linux 5.0 undefined
    CPU: (8) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
    Memory: 0 Bytes / 0 Bytes
    Shell: 1.0 - /bin/jsh
  Binaries:
    Node: 18.20.3 - /usr/local/bin/node
    Yarn: 1.22.19 - /usr/local/bin/yarn
    npm: 10.2.3 - /usr/local/bin/npm
    pnpm: 8.15.6 - /usr/local/bin/pnpm
  npmPackages:
    vite: ^6.0.1 => 6.0.1 

Used Package Manager

npm

Logs

No response

Validations

@thebanjomatic thebanjomatic changed the title Vite 6 change in behavior with noExternal and packages that share a name with node builtins Vite 6 change in behavior with noExternal & imports that share name with node builtins Nov 27, 2024
@sapphi-red sapphi-red added p3-minor-bug An edge case that only affects very specific usage (priority) regression The issue only appears after a new release labels Nov 28, 2024
esimkowitz added a commit to wavetermdev/waveterm that referenced this issue Dec 5, 2024
There was a bug in Vite 6.0.1
(vitejs/vite#18810) that was preventing NodeJS
libraries from being bundled in the main process. This meant that our
packaged app would refuse to run as it would be unable to find its Node
libraries.

This updates to Vite 6.0.2 to resolve this issue and also updates a few
other deps while we're at it.

Also removes the SASS modern-compiler config from our Vite config since
this is the default behavior in Vite 6.

closes #1373
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat: ssr p3-minor-bug An edge case that only affects very specific usage (priority) regression The issue only appears after a new release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants