test(adapter-node): boot the built output when instrumentation.server.js is present#16305
Open
SobolievOleksii wants to merge 7 commits into
Open
Conversation
….js is present Adds a fixture app + boot test that builds with adapter-node and starts build/index.js, asserting that (1) the server boots at all and (2) a module-scope $app/env/private read observes the runtime value. Currently red on version-3: with an instrumentation file present, the adapter's env.js is emitted outside the ENV_PREFIX replace filter and the server crashes at startup with 'ReferenceError: ENV_PREFIX is not defined'. Also covers the startup-crash and env-ordering regressions discussed in sveltejs#16288 / sveltejs#16302 / sveltejs#16303. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Install the latest version of pnpm add https://pkg.svelte.dev/@sveltejs/kit/c/38778c5bf48f2d86f2c09fa24e7bcc567d26df70Open in Note This PR is from a fork. A maintainer must approve approve each commit before it can be built and installed. |
|
6 tasks
Rich-Harris
pushed a commit
that referenced
this pull request
Jul 20, 2026
) Fixes #13718. Supersedes #13719, credit to @HoldYourWaffle for the diagnosis and approach. The trailing slash redirects use absolute pathnames, so any prefix a parent server strips before the request reaches kit (an Express mount, #13702) is dropped and the browser lands outside the app. The prefix never reaches Polka's parser, so it can't be reconstructed server side. A relative Location resolves against the full browser URL and is prefix-agnostic. Per the review on #13719, the helper is implemented in kit rather than adding `get-relative-path`, stays internal instead of being exported from `@sveltejs/kit/node`, and adapter-node gets a local copy since its handler is bundled standalone. Both call sites only produce pathnames differing by a trailing slash, so it handles exactly that case. The #2515 guard becomes unnecessary, a relative reference never starts with `/`. The options app trailingSlash tests now assert the Location values and fail on the old code. The prerender crawler resolves locations with `new URL()`, so it follows the relative form unchanged. Verified end to end with an adapter-node build mounted under a stripped prefix, both the SSR and the prerendered redirect. No adapter-node test app here since #16305 is establishing that infrastructure. --- ### 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. - [x] 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 - [x] 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: HoldYourWaffle <holdyourwaffle@gmail.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
1 participant
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.
What this adds
A boot test for adapter-node with a server instrumentation file present — the coverage gap behind #16288 and the review findings on #16302/#16303. The fixture app (under
packages/adapter-node/test/apps/instrumentation, next to the existingbasicapp) builds with the real adapter and thentest/boot.jsstartsbuild/index.jsand asserts:$app/env/privateread observes the runtime value (the "capturedundefined" class from instrumentation.server.js can evaluate app modules before Server.init() when the bundler colocates a shared dependency ($env/dynamic/private silently undefined) #16288).Wired into
pnpm testof the adapter (test:unit+test:apps), so the existingtest-othersCI job picks it up. The existingbasicapp covers hooks/streaming behaviour but nothing boots the output withsrc/instrumentation.server.jspresent — which is exactly the path #16303 changes.Results
version-3, built on ubuntu (this PR's CI)captured === liveversion-3, built on WindowsReferenceError: ENV_PREFIX is not defined— the adapter'senv.jsmodule lands outside theENV_PREFIXmagic-string replace filter (filter: { id: RegExp(entries) }), so the token survives intobuild/server/chunks/env-*.js. Chunk layout is platform-dependent, so this only bites Windows builds — andtest-othersonly runs on ubuntu, so CI can't see itSyntaxError: The requested module './env.js' does not provide an export named 'set_env'— the generated__sveltekit_env_init.jsfacade resolves the adapter's ownenv.jsinstead of kit's env module. This is a name collision in the emitted output, so it is platform-independent — this test would catch it in CI once the branch includes itSo on CI this PR is green today, guards the instrumentation boot path going forward, and would have flagged #16303. The Windows
ENV_PREFIXfailure is a separate latent bug — happy to file it as its own issue (kit's adapter tests never run on Windows, so a CI matrix entry would be needed to gate it).Refs #16288.