fix: ensure version is defined when using $app/env with explicit env vars#15971
Conversation
🦋 Changeset detectedLatest commit: 27717f0 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 |
|
The two failing jobs look unrelated to this change. test-kit (20) crashed at the Node level (Assertion failed: (req_wrap_async) != nullptr, exit 129) and test-kit (22) timed out on a remote-functions e2e test, while 18, 24 and all cross-browser runs pass. This PR only adds import.meta.hot to $app/env, so I don't think it's related, but happy to rebase or re-run if that helps. |
|
Added a regression test in |
064bf30 to
1ed409d
Compare
|
I wonder if a fix such as https://github.com/sveltejs/kit/pull/15574/changes#diff-a7244013c14e3925b24a2faf96dc33f01fae957ae2fa1a89a96fba3ee90be886R646 would be more fool proof. This isn’t the first time we’ve run into this issue and it could happen again depending on the order or presence of imported modules. |
When
experimental.explicitEnvironmentVariablesis enabled,$app/environmentthrows and apps import from$app/envinstead.$app/envre-exportsversionfrominternal.js(export const version = __SVELTEKIT_APP_VERSION__), but unlike$app/environmentit never referencesimport.meta.hot, so Vite doesn't force its client to load. If$app/envis imported before the Vite client has loaded,versionevaluates while the define is missing and the browser throws:This happens when
$app/envis imported early, e.g. fromhooks.client.js. Importing it from a route component is too late to hit it, since the client is already up by then, which is why it's intermittent. I ran into it in a real app after enablingexperimental.explicitEnvironmentVariables.It came in with #15934, which moved
versiononto the__SVELTEKIT_APP_VERSION__define and addedimport.meta.hotto$app/environment, but not to the newly split$app/env.The fix mirrors
$app/environment. I left that file'simport.meta.hotin place to keep this minimal, though it's now redundant with this one.Added a regression test in
options-2: an early$app/envimport inhooks.client.jsplus a load of the page. It fails onmain(the app never finishes hydrating) and passes with this change.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