Using `@vue/devtools`, `splitVendorChunkPlugin`, and having `stream` transitively installed causes Vite to output `import "emitter"` to the bundle

segevfiner commented 1 year ago

Describe the bug

Vite includes import "emitter" in the outputted bundle when you have conditional usage of @vue/devtools (That should be tree shaken out), splitVendorChunkPlugin and the module stream installed transitively.

Vite apparently includes chooses to include the node version of @vue/devtools in the bundle instead of the browser version. This then includes a require to the stream module, which due to installing pino-datadog resolved to the npm stream module, which includes an import "emitter" which gets included into the final bundle.

It doesn't get included without splitVendorChunkPlugin. Or some other usages of manualChunks that I wasn't able to pinpoint.

Reproduction

https://stackblitz.com/edit/vitejs-vite-u6bnd2?file=src/main.ts

Steps to reproduce

Run npm run build in the reproduction.
The outputted chunks include import "emitter" which will fail at runtime.

System Info

System:
    OS: macOS 13.2
    CPU: (10) arm64 Apple M1 Pro
    Memory: 89.91 MB / 16.00 GB
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 16.19.0 - ~/.nvm/versions/node/v16.19.0/bin/node
    Yarn: 1.22.19 - ~/.nvm/versions/node/v16.19.0/bin/yarn
    npm: 9.4.0 - ~/.nvm/versions/node/v16.19.0/bin/npm
  Browsers:
    Chrome: 109.0.5414.119
    Safari: 16.3
  npmPackages:
    @vitejs/plugin-vue: ^4.0.0 => 4.0.0 
    vite: ^4.0.0 => 4.0.4

Used Package Manager

pnpm

Logs

No response

Validations

[X] Follow our Code of Conduct
[X] Read the Contributing Guidelines.
[X] Read the docs.
[X] Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
[X] Make sure this is a Vite issue and not a framework-specific issue. For example, if it's a Vue SFC related bug, it should likely be reported to vuejs/core instead.
[X] Check that this is a concrete bug. For Q&A open a GitHub Discussion or join our Discord Chat Server.
[X] The provided reproduction is a minimal reproducible example of the bug.

Tanimodori commented 1 year ago

Workaround

pnpm i stream

/** eslint-env node */
import { defineConfig, splitVendorChunkPlugin } from "vite";
import vue from "@vitejs/plugin-vue";
import { resolve } from "node:path";

// https://vitejs.dev/config/
export default defineConfig({
  resolve: {
    alias: {
      stream: resolve(__dirname, "./node_modules/stream/index.js"),
    },
  },
  plugins: [vue(), splitVendorChunkPlugin()],
});

Tanimodori commented 1 year ago

patak-dev commented 1 year ago

This seems to be fixed in vite@4.3.0-beta.5: https://stackblitz.com/edit/vitejs-vite-6wetst

segevfiner commented 1 year ago

@patak-dev We are still getting a lot of warnings due to trying to build the Node version in the browser build though. Something is still wrong, it should try to build the browser build and strip it out due to tree-shaking, not build the Node build and strip it out...

sapphi-red commented 1 year ago

We are still getting a lot of warnings due to trying to build the Node version in the browser build though.

@segevfiner Would you elaborate this part? @vue/devtools only has main field and that file uses a runtime detection (typeof window !== 'undefined'). https://cdn.jsdelivr.net/npm/@vue/devtools@6.5.0/package.json https://cdn.jsdelivr.net/npm/@vue/devtools@6.5.0/index.js So it's expected that the node version gets in the build.

segevfiner commented 1 year ago

Maybe that's the bit that should be changed though. @vue/devtools itself might be better off using package.json exports instead of runtime detection?

sapphi-red commented 1 year ago

I think so. I'll close this issue as I think it needs to be fixed on the @vue/devtools side.

segevfiner commented 1 year ago

Should I open an issue on @vue/devtools?

sapphi-red commented 1 year ago

Yes, that would be nice.

vitejs / vite