Vite tries to aggressively prebundle even things that won't get used in the browser, and confuses itself

[Conduitry]

Describe the bug
Any import()s in an isomorphic part of the app are aggressively prebundled by Vite, even if they're never ultimately used in the browser (and even if they are inside an if (!browser) { }). This is a problem because when something cannot successfully be prebundled (e.g., if it uses Node APIs with no browser analogue), something within Vite crashes(?) or gets into a bad state, and - even if the offending import() is removed - Vite then does not perform other prebundling that actually is necessary until the server is restarted. This can lead to, for example, imports of CJS modules being left as import ... from '/node_modules/foo/...', which means the browser is being served CJS files when it is expecting ESM files.

Logs
The logs from Vite when it's failing to prebundle the package look like, e.g.:

 > node_modules/@sentry/node/esm/parsers.js:2:9: error: No matching export in "browser-external:fs" for import "readFile"
    2 │ import { readFile } from 'fs';
      ╵          ~~~~~~~~

 > node_modules/@sentry/node/esm/integrations/utils/http.js:3:9: error: No matching export in "browser-external:url" for import "URL"
    3 │ import { URL } from 'url';
      ╵          ~~~

 > node_modules/@sentry/node/esm/integrations/modules.js:2:9: error: No matching export in "browser-external:fs" for import "existsSync"
    2 │ import { existsSync, readFileSync } from 'fs';
      ╵          ~~~~~~~~~~

 > node_modules/@sentry/node/esm/integrations/modules.js:2:21: error: No matching export in "browser-external:fs" for import "readFileSync"
    2 │ import { existsSync, readFileSync } from 'fs';
      ╵                      ~~~~~~~~~~~~

 > node_modules/@sentry/node/esm/integrations/modules.js:3:9: error: No matching export in "browser-external:path" for import "dirname"
    3 │ import { dirname, join } from 'path';
      ╵          ~~~~~~~

 > node_modules/@sentry/node/esm/integrations/modules.js:3:18: error: No matching export in "browser-external:path" for import "join"
    3 │ import { dirname, join } from 'path';
      ╵                   ~~~~

The logs from the browser when it's attempting to load the CJS version of a module as a ESM when Vite gives up and serves that instead look like:

Uncaught (in promise) ReferenceError: exports is not defined
    <anonymous> http://localhost:3000/node_modules/decoders/index.js:3

To Reproduce
My reproduction uses two of the libraries I was seeing this with in a real project. @sentry/node is the library that can't (and shouldn't) be bundled for the browser. decoders is the library that's available only in CJS and that needs to be prebundled for the browser to understand it.

Clone https://github.com/Conduitry-Repros/kit-1570 and run npm install.

Start up the server with npm run dev and check the browser console, where you should see the results of console.log(await import('decoders'));.

Then, open src/routes/index.svelte and uncomment the indicated line. Refresh the browser if necessary. In the Vite console, you should now see errors about not being able to process @sentry/node. In the browser console, you should see errors that result from attempting to load CJS as ESM. If you look in the network tab, you'll see that http://localhost:3000/node_modules/decoders/index.js is being requested (the raw CJS version from the package) rather than the http://localhost:3000/node_modules/.vite/decoders.js you should be getting.

This broken state persists even if you re-comment the import('@sentry/node'). You don't get the prebundled version of decoders again until you restart the server.

Expected behavior
A clear and concise description of what you expected to happen.

Stacktraces
n/a

Information about your SvelteKit Installation:

Diagnostics

• The envinfo command hangs for me, I'm guessing because of WSL. This is a separate issue to be looked into, probably.
• Firefox 88.0.1, but this shouldn't be relevant
• No adapter, for now. This is in dev mode.

Severity
Fairly severe, as it was causing some very confusing behavior. I do have a workaround for now though, which is to use a helper function that I call instead of import():

export default async function serverImport(id) {
    const { createRequire } = await import('module');
    const require = createRequire('/absolute/path/package.json');
    return require(id);
}

This relies on a known absolute path to resolve relative to, which is fine in my case as I'm using Docker. Note that I cannot use import.meta.url as that is a path that looks absolute but is in fact relative to the root of my project. It's /src/whatever/..., not a real path in my whole filesystem.

Additional context
I don't really know enough to be able to tell whether this is a Vite bug or us calling Vite in the wrong way. It failing to prebundle certain dependencies makes sense. Some dependencies just won't work that way. However, it aggressively attempting to prebundle things before they're even requested by the browser seems questionable. And it then getting into a state where it won't prebundle anything else is definitely a problem.

[benmccann]

It seems like that's probably a Vite bug. Probably more useful to file a bug there in terms of someone seeing it who might know enough about Vite to fix it.

A good first step would be to see if you can reproduce it in a Vite project without SvelteKit. You can create a new Vite project with npm init @vitejs/app my-svelte-app --template svelte, but that's a client-side only project. If you want an isomorphic project, the best way I know how is probably to start with this one: https://github.com/sveltejs/vite-plugin-svelte/tree/main/packages/playground/vite-ssr

[Bradley-H]

I might be the minority, why not bundle Rollup with svelte again instead of going to someone whom is third party to fix our bugs and issues?
Plus as I understand Rich, whom made svelte also made rollup can tweak it to the communities needs.

[geek981108]

I might be the minority, why not bundle Rollup with svelte again instead of going to someone whom is third party to fix our bugs and issues?
Plus as I understand Rich, whom made svelte also made rollup can tweak it to the communities needs.

Same issue while using windicss

[dummdidumm]

Vite uses Rollup, so there are similarities. Using Rollup only is not an option because you'd lose the unbundled dev experience and you certainly don't want to go back to the long reload times while developing.

[Conduitry]

I've been looking at this again this morning. There's actually a way simpler workaround that I found shortly after opening this issue, which is:

export default function serverImport(id) {
    return import(id);
}

I.e., you can just hide the import from Vite by calling it in another function.

I've also been able to reproduce this from the Svelte vite-ssr template Ben linked above, so I don't think it's something SvelteKit-specific, anyway. It's still possible it's vite-plugin-svelte-specific.

By adding

            optimizeDeps: {
                exclude: ["@sentry/node"],
            },

to my configuration, I'm able to avoid Vite trying to prebundle the dependency for the browser, which means that running the app in development now works even without the workaround of hiding import from Vite. However, in the production build, Vite is trying to look into the @sentry/node dependency for some reason, and is getting tripped up by something.

[commonjs] Unexpected token (228:35) in /.../node_modules/@sentry/node/esm/handlers.js
file: /.../node_modules/@sentry/node/esm/handlers.js:228:35
226:     }
227:     if (options.serverName && !event.server_name) {
228:         event.server_name = global.process.env.SENTRY_NAME || os.hostname();
                                        ^
229:     }
230:     if (options.user) {
> Unexpected token (228:35) in /.../node_modules/@sentry/node/esm/handlers.js

I'm guessing this is because of the Vite replacing process.env with ({}) across the board as mentioned in vitejs/vite#3176 although I haven't actually looked into this at all. It remains a mystery why Vite is having Rollup try to bundle this file.

[Conduitry]

I've opened an issue on Vite's side - vitejs/vite#3715

[stolinski]

Would it make sense that this is also happening in reverse. ie. my node build is failing from a client only dep. Node is having an issue with a dynamic import of a video player I'm using.

[bluwy]

Would it make sense that this is also happening in reverse. ie. my node build is failing from a client only dep. Node is having an issue with a dynamic import of a video player I'm using.

I'm not sure if it's related since prebundling isn't used in the server build, but I'm guessing it has something to do with third-party side effects handling (similar vitejs/vite#6193 (comment)).

---

I think the correct solution to this issue is optimizeDeps.exclude as there's no way for Vite to tell a package is node-only. The errors could be better though, which should be covered by vitejs/vite#3715. So I'm closing in favour of that.

[Conduitry]

👍 optimizeDeps.exclude worked for me. I'm no longer seeing the issue I mentioned above. I'm guessing that this is because of the fix for the process.env. -> ({}). stuff in SSR that's happened in Vite in the meantime.

I'm fine with this being closed on our end - we can use the Vite issue for tracking more graceful handling of these errors.

[ffxsam]

@Conduitry Thanks! I feel like the dog in the "I have no idea what I'm doing" meme, but this worked for me.

  optimizeDeps: {
    exclude: ['path', 'fs', 'os', 'perf_hooks', 'util'],
  },

[raulfdm]

optimizeDeps

According to Vite docs, optimizeDeps.exclude is meant for: Dependencies to exclude from pre-bundling.

That means Vite will not try to include the dependencies you listed (path, fs, etc.) into the final index.js bundle (the file which is gonna be shipped to production.

[ffxsam]

No description provided.