fix: always bundle SvelteKit, always externalize CommonJS dependencies during development #9203
Conversation
Rich-Harris
changed the title
| @@ -252,14 +252,13 @@ function kit({ svelte_config }) { | |||
| '$app', | |||
| '$env' | |||
| ] | |||
| }, | |||
| ssr: { | |||
| // These packages must be bundled by Kit so that we can resolve using the proper aliases and conditions | |||
There was a problem hiding this comment.
I don't think it's true that we always need to bundle @sveltejs/kit. I expect it'd have negative performance implications. We really only need to do it if using Vitest to force it to go through the standard Vite pipeline instead of the vite-node pipeline so that resolveId is called
There was a problem hiding this comment.
Ok I'm back to being confused. Remind me why we need to ensure resolveId is called? @sveltejs/kit doesn't import any aliased modules. Is it about ensuring that we don't end up with two copies of HttpError (breaking instanceof checks) or something else?
There was a problem hiding this comment.
We need resolveId to be called to handle our virtual modules. @sveltejs/kit imports __sveltekit/paths and __sveltekit/environment internally
There was a problem hiding this comment.
No, it doesn't. @sveltejs/kit resolves to src/exports/index.js which imports esm-env and src/runtime/control.js, which doesn't import anything (but we do need to ensure that those modules are only imported once for the instanceof checks, which is why I raise it)
There was a problem hiding this comment.
Removing it appears to do no harm — tested it with a project that links @sveltejs/kit and uses redirect, and everything is fine
* fix: don't force externalize SvelteKit * bundle when using vitest * update changelog description * remove accidental comma * Update packages/kit/src/exports/vite/index.js * fix: always bundle SvelteKit, always externalize CommonJS dependencies during development (#9203) * simplify * add comments etc * no need to noExternal kit itself * update comment * reinstate old comment * work around vitest * Update packages/kit/src/exports/vite/index.js Co-authored-by: Ben McCann <322311+benmccann@users.noreply.github.com> --------- Co-authored-by: Rich Harris <richard.a.harris@gmail.com> Co-authored-by: Rich Harris <hello@rich-harris.dev>
Simplifies
noExternal, and reinstates theexternalconfig that #9172 incorrectly removes