Better handling of dynamic imports in dead code

[blckngm]

Describe the bug

When working with SSR, one would expect patterns like this to work:

if (import.meta.env.SSR) {
  const { data } = await import('./loadServer');
  console.log(data);
} else {
  const data = await fetch('/data').then((r) => r.text());
  console.log(data);
}

I.e., ignore loadServer for the browser bundle. It actually works fine with the dev server, but when loadServer (transitively) imports node builtin modules, vite build fails with e.g.:

Error: 'readFile' is not exported by __vite-browser-external, imported by loadServer.js

It would be nice if vite could just figure out that loadServer will never be needed for the browser bundle, and don't try to analyze/resolve code in it. It would also make dev and build behavior more consistent and might perform better in some cases.

I also tried this with esbuild and it does just ignore loadServer in this case:

❯ npx esbuild --format=esm --bundle main.js --define:import.meta.env.SSR=false
// main.js
if (false) {
  const { data } = await null;
  console.log(data);
} else {
  const data = await fetch("/data").then((r) => r.text());
  console.log(data);
}

Reproduction

https://stackblitz.com/edit/vitejs-vite-ts9jc6?devtoolsheight=33&file=main.js

System Info

System:
    OS: Linux undefined
    CPU: (4) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
    Memory: 0 Bytes / 0 Bytes
    Shell: Unknown - /bin/jsh
  Binaries:
    Node: 14.16.0 - /usr/local/bin/node
    Yarn: 1.22.10 - /bin/yarn
    npm: 7.17.0 - /bin/npm
  npmPackages:
    vite: ^2.6.13 => 2.6.14

Used Package Manager

npm

Logs

No response

Validations

• Follow our Code of Conduct
• Read the Contributing Guidelines.
• Read the docs.
• Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
• 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 https://github.com/vuejs/vue-next instead.
• Check that this is a concrete bug. For Q&A open a GitHub Discussion or join our Discord Chat Server.
• The provided reproduction is a minimal reproducible example of the bug.

[blckngm]

This plugin mitigates the issue but breaks vue SFC styling:

// Replace import.meta.env.SSR in a pre plugin so that esbuild can eliminate
// dynamic imports in the dead branch.
function DefineSSR(): PluginOption {
  return {
    name: "define ssr",
    async transform(code, _id, ssr) {
      if (!ssr && code.includes("import.meta.env.SSR")) {
        return code.replace(/import.meta.env.SSR/g, "false");
      }
      return undefined;
    },
    enforce: "pre",
  };
}

[bluwy]

That's strange. Vite does replace import.meta.env.SSR here. Could be an issue with the order of Vite plugins being ran 🤔

[blckngm]

That's strange. Vite does replace import.meta.env.SSR here. Could be an issue with the order of Vite plugins being ran 🤔

Yes. Moving the define plugin before esbuild will solve this issue, but it breaks vue SFC styling.

[ygj6]

Use the following configuration as a workaround to solve the problem, and the dead code is removed after a successful build.

import { defineConfig } from 'vite'
import { builtinModules } from 'module'

export default defineConfig({
  build: {
    rollupOptions: {
      external: builtinModules
    }
  }
})

This may not be the replacement of import.meta.env.SSR, because I just change it to false in the source code and still get the same error.

Perhaps we should adjust the order in which dead code is removed at build time, removing the dead code first and then performing subsequent build steps.

In dev, the imported module is handled by vite only when it is executed, where errors do not occur because it is not executed.

In build, Vite uses the vite:esbuild-transpile plug-in to perform tree-shaking, which is posted. Until then, dead code will still be processed by other plugins and rollup.

After some experimentation, I found that I couldn't avoid this behavior, either by bringing the tree-shaking of vite:esbuild-transpile forward (which would fail due to relying on the processing of the previous plugin) or changing the default behavior of rollup to avoid errors (which I'm not familiar with).

[DaniilIsupov]

I use crypto inside if (import.meta.env.SSR) { check and get an warning: [plugin:vite:resolve] Module "crypto" has been externalized for browser compatibility, imported by "/path-to-script.ts". See http://vitejs.dev/guide/troubleshooting.html#module-externalized-for-browser-compatibility for more details.

in practice this code does not get into browser bundle, but this warning makes me worry

Info

vite/4.4.9 darwin-arm64 node-v20.5.1

[nestle49]

I use crypto inside if (import.meta.env.SSR) { check and get an warning: [plugin:vite:resolve] Module "crypto" has been externalized for browser compatibility, imported by "/path-to-script.ts". See http://vitejs.dev/guide/troubleshooting.html#module-externalized-for-browser-compatibility for more details.

in practice this code does not get into browser bundle, but this warning makes me worry

Info

vite/4.4.9 darwin-arm64 node-v20.5.1

I have the same problem

[DaniilIsupov]

Problem is still relevant

[KTibow]

You could use define instead

[Tnze]

You could use define instead

How do you dynamic setting it? Like yarn vite build --define XXX?

[DaniilIsupov]

You could use define instead

i didn't notice the difference at all, warning remained

[KTibow]

@Tnze either have multiple config files or have your config file change the defined value based on the mode or environment variable or something
@DaniilIsupov we can't help you if you don't share your code and config