chore: write env modules to disk - #16807
Merged
Merged
Conversation
|
Install the latest version of pnpm add https://pkg.svelte.dev/@sveltejs/kit/c/ea22879896bb09108bee6035c22288ce6b795c5aOpen in |
🦋 Changeset detectedLatest commit: ea22879 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
…osixified `out_dir` (backslashes), so index.js's posix-keyed `chunk.modules` lookup for the dynamic public env module never matches, forcing `uses_env_dynamic_public` to always be false.
This commit fixes the issue reported at packages/kit/src/exports/vite/plugins/env-vars.js:139
## Bug
`packages/kit/src/exports/vite/plugins/env-vars.js` computed:
```js
out_dir = path.resolve(c.root, out); // native separators
```
and its `resolveId` handler returns module ids derived from that value, e.g.:
```js
const dir = `
```
On **Windows**, `path.resolve` produces backslash-separated paths (`C:\proj.svelte-kit`), so the base of the returned module id contains backslashes: `C:\proj.svelte-kit/generated/env/public/client.js`. Rollup/rolldown store the module id exactly as returned by `resolveId`, so `chunk.modules` is keyed with that mixed-separator string.
Meanwhile `packages/kit/src/exports/vite/index.js` (~line 1768) looks the module up using a fully posix key:
```js
out_dir = posixify(kit.outDir); // line 386, forward slashes
// ...
const uses_env_dynamic_public =
has_explicit_dynamic_public_env &&
client_chunks.some(
(chunk) =>
chunk.type === 'chunk' &&
chunk.modules[`
```
Because the two keys differ in the base separators on Windows (`` vs `/`), the lookup never matches. **Trigger:** a Windows build with `bundleStrategy: 'split'` and a project that imports `$app/env/public` / uses a dynamic (non-static) public env var. `uses_env_dynamic_public` is forced to `false`, so the code that arranges correct preloading/loading of dynamic public env for the split bundle is skipped, breaking runtime public env vars on Windows.
The rest of the codebase consistently uses `posixify(kit.outDir)` for `out_dir` precisely so module ids match — this plugin was the sole divergence.
## Fix
Posixify `out_dir` in the plugin so the generated module ids use forward slashes and match index.js's lookup key:
```js
import { posixify } from '../../../utils/os.js';
// ...
out_dir = posixify(path.resolve(c.root, out));
```
`posixify` (from `packages/kit/src/utils/os.js`) simply replaces `\` with `/`, which is a no-op on POSIX systems and corrects the separators on Windows, restoring the key match.
Co-authored-by: Vercel <vercel[bot]@users.noreply.github.com>
Co-authored-by: Rich-Harris <hello@rich-harris.dev>
…unning is no longer handled: new env files are ignored until restart, and deleting an existing env file throws because `resolved_entry` is never re-resolved.
This commit fixes the issue reported at packages/kit/src/exports/vite/plugins/env-vars.js:149
## Bug
The `plugin_env_vars` plugin in `packages/kit/src/exports/vite/plugins/env-vars.js` computes `resolved_entry` only once, in `buildStart`, and only regenerates env modules via `handleHotUpdate` when the changed file is already in `deps`:
```js
async buildStart() {
resolved_entry = resolve_entry(path.join(resolved_config.root, entry)) ?? null;
await generate();
},
async handleHotUpdate(update) {
if (!deps.has(update.file)) return;
await generate();
},
```
This replaced a watcher previously in `plugin_virtual_modules.configureServer` (in `index.js`) that listened to `server.watcher.on('all', ...)`, re-resolved the entry via `resolve_explicit_env_entry`, regenerated, and triggered a full reload on any add/remove/change of the env entry. (`resolve_explicit_env_entry` is now only imported in `index.js`, never called.)
Vite's `handleHotUpdate` only fires for `change` events on files it already tracks — it is **not** called for file `add`/`unlink`. That produces two concrete dev regressions:
1. **Creating `src/env.ts` after start**: If it didn't exist at startup, `resolved_entry` is `null` and `deps` is empty (`load_explicit_env` returns an empty dep set when `file` is null). The `add` event doesn't reach `handleHotUpdate`, and even if it did the file isn't in `deps`, so the new env vars are never generated until a manual restart.
2. **Deleting an existing `src/env.ts`**: The entry file is in `deps` (the dependency-scanner `load` hook records it). On delete, `handleHotUpdate` fires with the deleted file in `deps`, calling `generate()` → `sync.env(config, resolved_entry, ...)` with the stale, now-nonexistent `resolved_entry`, so `load_explicit_env` does `runner.import(file)` on a missing module and throws (`ERR_MODULE_NOT_FOUND`). `resolved_entry` is never reset to `null`.
The dev watcher in `dev/index.js` handles app/error templates, service worker and hooks, but not `src/env`, so it doesn't compensate.
## Fix
Added a `configureServer` hook to the env-vars plugin that listens for `add`/`unlink` events, re-resolves the entry, updates `resolved_entry`, regenerates the modules, and sends a `full-reload` — mirroring the removed watcher. `change` events remain handled by `handleHotUpdate` (for the entry and its transitive deps), so there is no double processing.
```js
configureServer(server) {
const on_entry_add_unlink = async (file) => {
const resolved = resolve_entry(path.join(resolved_config.root, entry)) ?? null;
if (file === resolved_entry || file === resolved) {
resolved_entry = resolved;
await generate();
server.hot.send({ type: 'full-reload' });
}
};
server.watcher.on('add', on_entry_add_unlink);
server.watcher.on('unlink', on_entry_add_unlink);
},
```
This restores dev handling for both creating (case 1) and deleting (case 2, `resolved` becomes `null` so `generate()` runs with a null entry and produces empty env modules instead of throwing) the env entry. The `server.watcher`/`server.hot` usage matches existing patterns in `dev/index.js`.
Co-authored-by: Vercel <vercel[bot]@users.noreply.github.com>
Co-authored-by: Rich-Harris <hello@rich-harris.dev>
Co-authored-by: Nic Polumeyv <nicolas.polum@outlook.com>
…generated-env-modules
Co-authored-by: Nic Polumeyv <nicolas.polum@outlook.com>
…generated-env-modules
Co-authored-by: Nic Polumeyv <nicolas.polum@outlook.com>
…generated-env-modules
Co-authored-by: Nic Polumeyv <nicolas.polum@outlook.com>
…the config with the wrong mode
…nts aren't always called 'ssr'
Nic-Polumeyv
left a comment
Contributor
There was a problem hiding this comment.
What a roller coaster. LGTM!
Nic-Polumeyv
approved these changes
Aug 14, 2026
Member
Author
|
feels good! appreciate the quarterbacking on this, some of the Vite nuances can be pretty subtle |
6 tasks
Rich-Harris
added a commit
that referenced
this pull request
Aug 17, 2026
Follow-up to #16807. Instead of using a `resolveId` hook, we can use an alias for all the generated modules — every module ID like `<sveltekit:generated>/foo.js` corresponds to `.svelte-kit/generated/(build|dev)/foo.js`, making things a little easier to navigate, and reducing the cost of adding more generated modules relative to having to faff about with plugin hooks. The `<sveltekit:generated>` prefix is bikesheddable, but I figured it's worth being explicit about what this is, and using characters that are invalid in npm package names. Creating separate directories for dev and build means we don't need to be as careful about what goes where, and can freely use relative imports between generated modules. It means that building while also running a dev server won't result in clobbering. We can easily extend this to the other virtual modules. --- ### Please don't delete this checklist! Before submitting the PR, please make sure you do the following: - [x] It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs - [x] This message body should clearly illustrate what problems it solves. - [ ] Ideally, include a test that fails without this PR but passes with it. ### Tests - [x] Run the tests with `pnpm test` and lint the project with `pnpm lint` and `pnpm check` ### Changesets - [ ] If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running `pnpm changeset` and following the prompts. Changesets that add features should be `minor` and those that fix bugs should be `patch`. Please prefix changeset messages with `feat:`, `fix:`, or `chore:`. ### Edits - [x] Please ensure that 'Allow edits from maintainers' is checked. PRs without this option may be closed. --------- Co-authored-by: vercel[bot] <35613825+vercel[bot]@users.noreply.github.com> Co-authored-by: Vercel <vercel[bot]@users.noreply.github.com> Co-authored-by: Nic Polumeyv <nicolas.polum@outlook.com> Co-authored-by: Nic Polumeyv <162764842+Nic-Polumeyv@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ref #15212.
Something we've wanted to do for a while is get rid of the virtual modules, in favour of writing stuff to disk. The system becomes a lot easier to understand when it involves real artifacts instead of the crazy indirection we have going on at the moment.
I started with the
$app/env/*stuff because that's likely to be the most challenging, since it involves some mad science around starting up a mini Vite dev server to load thesrc/env.tsmodule so that we can analyse it so that we can create a generated module that also loads thesrc/env.tsmodule... anyway, it works, and so I assume we will be able to do the same for the other__sveltekit/*modules.(We could probably replace the
resolveIdlogic with an alias that just points to thegeneratedfolder, same as we have for$app/*currently. That can wait for a follow-up PR though.)Another thing I'm doing in this PR is creating the plugin in a separate module, rather than adding to the chaos in
vite/index.js. It involves a little bit of duplication, but it makes everything so much more self-contained, and makes the coupling between different plugins more explicit (e.g. thecallback).Please don't delete this checklist! Before submitting the PR, please make sure you do the following:
Tests
pnpm testand lint the project withpnpm lintandpnpm checkChangesets
pnpm changesetand following the prompts. Changesets that add features should beminorand those that fix bugs should bepatch. Please prefix changeset messages withfeat:,fix:, orchore:.Edits