feat!: vite 6 support #1020
feat!: vite 6 support #1020
Conversation
| config.resolve.conditions = [...VITE_CLIENT_RESOLVE_CONDITIONS]; | ||
| // These exports only exist in Vite 6 | ||
| const { defaultClientConditions, defaultServerConditions } = await import('vite'); | ||
| if (name === 'client' || opts.isSsrTargetWebworker) { |
There was a problem hiding this comment.
is name === 'client' a good way to check this? that would assume there can only be one client environment in an app.
There was a problem hiding this comment.
what if there was a tauri app and a web app at the same time, or two different clients?
There was a problem hiding this comment.
Ah true, I think it should check the consumer here, which is the same as Vite internally.
There was a problem hiding this comment.
is that a helper vite should expose?
There was a problem hiding this comment.
I'm not sure if it's needed imo
There was a problem hiding this comment.
well how are others going to use these exposed constants then? you'll always want the ones applicable to the environment you are in, no?
There was a problem hiding this comment.
I'm not really sure what you mean. Do you have a pseudocode in mind for this?
There was a problem hiding this comment.
if vite-plugin-svelte needs to check consumer (where exactly is that?) to decide which of the two constants to pick, wouldn't other plugins have to do the same? at this point it would be better to have a shared place for that logic.
There was a problem hiding this comment.
I've already updated the code to use consumer, it is the way Vite wants plugins to do to checks for client or server-based environments. You can get that from config.consumer
There was a problem hiding this comment.
Looks like I need a combination of config.consumer === 'client' and name === 'client' checks to work, since config.consumer could be undefined but it's documented to fallback to consumer = 'client' if name is 'client'
| // be properly used by svelte | ||
| if (!isDepExternaled('esm-env', config.ssr?.external ?? [])) { | ||
| noExternal.push('esm-env'); | ||
| } |
There was a problem hiding this comment.
add warning log that externalizing it will cause trouble?
(with filterable code)
There was a problem hiding this comment.
I'd personally defer this to later if someone stumbles on the issue 🤔
There was a problem hiding this comment.
ok, lets hope noone externalizes it.
Very rough support. There's some things I want to double-check first so marking as draft. But this should pass vite-ecosystem-ci now.