Skip to content

chore: bundle adapter-node's server source directly instead of prebuilding it with rolldown - #17109

Merged
teemingc merged 20 commits into
version-3from
adapter-node-drop-prebuild
Sep 23, 2026
Merged

teemingc merged 20 commits into
version-3from
adapter-node-drop-prebuild

Conversation

@Nic-Polumeyv

@Nic-Polumeyv Nic-Polumeyv commented Sep 14, 2026 •

Copy link
Copy Markdown
Member

adapter-node prebuilds its server source with rolldown into files/ and points the app's Vite build at that output. The only reason was to vendor devDependencies like sirv into the adapter, since a consumer's node_modules cannot resolve them. With #16908 the source imports nothing but Node builtins, @sveltejs/kit/node and the hand-off module from #17103, all of which the app's own build resolves.

This points the Vite inputs at src/ and deletes the prebuild: the rolldown config and dependency, the build scripts, the CI step that ran them, and the two test fixtures that guarded chunk patching and specifier interception, neither of which exists now.

Stacked on #16908.

@pkg-svelte-dev

pkg-svelte-dev Bot commented Sep 14, 2026 •

Copy link
Copy Markdown

Install the latest version of @sveltejs/kit from fe20a80:

pnpm add https://pkg.svelte.dev/@sveltejs/kit/c/fe20a80ab18d363c00e7ab4308400778cf02dd68

Open in pkg.svelte.dev: https://pkg.svelte.dev/repos/kit/pr/17109

@changeset-bot

changeset-bot Bot commented Sep 14, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: fe20a80

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@sveltejs/adapter-node Patch

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

@Nic-Polumeyv
Nic-Polumeyv force-pushed the adapter-node-drop-prebuild branch 2 times, most recently from e308b55 to ccfa2ce Compare September 14, 2026 23:38
@Nic-Polumeyv
Nic-Polumeyv force-pushed the manifest-static-assets branch 2 times, most recently from 6a1bf95 to a025f33 Compare September 14, 2026 23:51
@Nic-Polumeyv
Nic-Polumeyv force-pushed the adapter-node-drop-prebuild branch from ccfa2ce to 348f74f Compare September 14, 2026 23:51
@Nic-Polumeyv
Nic-Polumeyv force-pushed the adapter-node-drop-prebuild branch from 348f74f to c6538b5 Compare September 14, 2026 23:59
@Nic-Polumeyv
Nic-Polumeyv added this pull request to stack #17110 September 15, 2026 00:09
@Nic-Polumeyv
Nic-Polumeyv marked this pull request as ready for review September 15, 2026 00:14
@Rich-Harris Rich-Harris added the needs-platform-tests This PR needs to run platform tests in order to merge. label Sep 15, 2026
@Nic-Polumeyv
Nic-Polumeyv removed this pull request from stack #17110 September 17, 2026 19:30
@Nic-Polumeyv
Nic-Polumeyv force-pushed the adapter-node-drop-prebuild branch from 23fd551 to e4d272b Compare September 17, 2026 19:32
@Nic-Polumeyv
Nic-Polumeyv added this pull request to stack #17126 September 17, 2026 19:32
Base automatically changed from manifest-static-assets to version-3 September 23, 2026 14:35

@vercel vercel Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additional Suggestion:

serve_static returns HTTP 405 for every non-GET/HEAD request before the asset lookup, blocking all POST/PUT/DELETE/PATCH/OPTIONS requests to any path and preventing SvelteKit SSR (form actions, +server.js handlers, remote functions, CORS preflight) from ever running.

Fix on Vercel

…efore the asset lookup, blocking all POST/PUT/DELETE/PATCH/OPTIONS requests to any path and preventing SvelteKit SSR (form actions, `+server.js` handlers, remote functions, CORS preflight) from ever running.

This commit fixes the issue reported at packages/adapter-node/src/static.js:172

## Bug

In `packages/adapter-node/src/static.js`, `serve_static` returned the middleware:

```js
return (req, res, next) => {
	if (req.method !== 'GET' && req.method !== 'HEAD') {
		res.writeHead(405, { allow: 'GET, HEAD' });
		res.end();
		return;
	}

	const { pathname, search } = split_url(req);
	const asset = files.get(pathname);
	if (!asset) return next();
	...
```

The method check runs **before** the asset lookup and **before** `next()`. Because the handler is wired as `sequence([serve_static(...), ssr])` (see `packages/adapter-node/src/handler.js`), `serve_static` runs first for **every** request.

### Failure mode / trigger

Any non-GET/HEAD request to a non-asset path (e.g. `POST /login`, `PUT /api/item`, `DELETE /api/x`, `PATCH ...`, or a CORS `OPTIONS` preflight) is short-circuited with `405 Allow: GET, HEAD` and never reaches the SSR handler. This breaks:

*   Form actions (`POST` to `+page.server.js` actions)
*   API routes (`+server.js` `POST`/`PUT`/`DELETE`/`PATCH` handlers)
*   Remote functions
*   CORS preflight (`OPTIONS`)

This is a regression from the previous `sirv`-based behavior, which forwarded non-GET/HEAD methods to `next()`.

## Fix

Move the 405 check to **after** the asset lookup and the redirect (`location`) handling, so the method restriction only applies when the path actually maps to a real static asset:

```js
const { pathname, search } = split_url(req);
const asset = files.get(pathname);
if (!asset) return next();

if ('location' in asset) {
	res.writeHead(308, { location: asset.location + search }).end();
	return;
}

if (req.method !== 'GET' && req.method !== 'HEAD') {
	res.writeHead(405, { allow: 'GET, HEAD' });
	res.end();
	return;
}
```

Now non-asset paths fall through to `next()` regardless of method, while a disallowed method on an actual static asset still correctly returns 405.

## Test

The existing test `disallows non-GET/HEAD methods` POSTs to `/plain.txt` (a real asset) and still passes. I added a regression test `passes non-GET/HEAD requests to non-asset paths to the next handler` that POSTs to `/missing.txt` and asserts it reaches the next handler (status 404, body `next`), which fails on the old code and passes on the fixed code.


Co-authored-by: Vercel <vercel[bot]@users.noreply.github.com>
Co-authored-by: teemingc <chewteeming01@gmail.com>
@teemingc
teemingc merged commit 5db47d3 into version-3 Sep 23, 2026
38 of 41 checks passed
@teemingc
teemingc deleted the adapter-node-drop-prebuild branch September 23, 2026 16:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-platform-tests This PR needs to run platform tests in order to merge. pkg:adapter-node

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants