chore: remove define merging - #16819
Conversation
…odule path, so it never matches and is always `false`.
This commit fixes the issue reported at packages/kit/src/exports/vite/index.js:1773
## Bug
In `packages/kit/src/exports/vite/plugins/env-vars.js`, this PR moved the generated env directory:
```js
dir = posixify(
path.resolve(c.root, config.outDir, `generated/${is_build ? 'build' : 'dev'}/env`)
);
```
So in build mode the client env module is written to `${out_dir}/generated/build/env/public/client.js`.
But `packages/kit/src/exports/vite/index.js:1773` still looked up the pre-PR path:
```js
chunk.modules[`${out_dir}/generated/env/public/client.js`]
```
`chunk.modules` keys are absolute module paths, and `out_dir === posixify(kit.outDir)` matches the base env-vars.js resolves against — so the only difference is the missing `build/` segment. This block runs during the client build (`is_build` is always true here, consistent with line 461 which uses `generated/${is_build ? 'build' : 'dev'}`), so the lookup can never match.
## Impact
`uses_env_dynamic_public` becomes stuck at `false`. It is stored in `build_data.client` (lines ~1801 and ~1849) and controls whether the runtime prerendered public env module is loaded at runtime. Apps that import `$app/env/public` on the client **and** use dynamic (non-static) public env vars would silently get stale/missing runtime env values — a regression versus before this PR, where the path matched.
## Trigger
Build an app that imports `$app/env/public` in client code with at least one `public && !static` env var configured. Previously the chunk-module lookup matched and `uses_env_dynamic_public` was `true`; now it always resolves `false`.
## Fix
Updated the lookup to the new build-mode path:
```js
chunk.modules[`${out_dir}/generated/build/env/public/client.js`]
```
Co-authored-by: Vercel <vercel[bot]@users.noreply.github.com>
Co-authored-by: Rich-Harris <hello@rich-harris.dev>
|
Install the latest version of pnpm add https://pkg.svelte.dev/@sveltejs/kit/c/4c242541e990d32758890ae82ba926a7524d8509Open in |
|
|
Oh actually I tell a lie, it predates that — the |
| builder.environments.serviceWorker.config.resolve.alias = [ | ||
| ...get_config_aliases(kit, vite_config.root) | ||
| ]; |
There was a problem hiding this comment.
| builder.environments.serviceWorker.config.resolve.alias = [ | |
| ...get_config_aliases(kit, vite_config.root) | |
| ]; |
We ought to get rid of this too. It was only there to avoid resolving the __SERVER__ alias but we now use this setting for much more than that. Ideally, we'd still remove __SERVER__ somehow but maybe it doesn't matter?
There was a problem hiding this comment.
Here's what the service worker allows resolving to if not overridden:
[
{
find: '__SERVER__',
},
{
find: '$app',
},
{
find: '$env',
},
{
find: '<sveltekit:generated>',
},
{
find: '__sveltekit/server',
},
{
find: /^\/?@vite\/env/,
},
{
find: /^\/?@vite\/client/,
}
]
|
Marking this draft as it'll conflict with #16813. We need to add some defines to the service worker environment so that it can correctly build |
2 participants
follow-up to #16812 — just curious to see what breaks if we remove this