feat(hosting): add SvelteKit framework adapter#202
Conversation
Add a first-class SvelteKit adapter to @aws-blocks/hosting. SvelteKit apps are auto-detected (@sveltejs/kit) and deployed via @sveltejs/adapter-node running on Lambda behind the Lambda Web Adapter (existing http-server compute path), the same proven model as the Astro (@astrojs/node) and Nitro node-server adapters. - packages/hosting/src/adapters/sveltekit.ts: version guard (>=2.0), transparent svelte.config bridge that force-selects @sveltejs/adapter-node when unwired (auto-install + package-manager detection, restored in finally), build runner, and a DeployManifest builder (http-server compute with HOST_HEADER/ PROTOCOL_HEADER/BODY_SIZE_LIMIT, _app/immutable/* immutable assets, prerendered->S3 routing, error pages, basePath). Normalizes SvelteKit's flat prerendered HTML (about.html) into directory-index form (about/index.html) so the L3 router's extensionless lookup resolves on S3. - Register the adapter, add @sveltejs/kit framework detection, FrameworkType union, and the 'build' default output dir. - Unit tests (sveltekit.test.ts), detectFramework cases, version-pin gate. - test-apps/hosting-ssr-sveltekit: sample app exercising SSR, prerender/SSG, +server.js (all verbs), form actions, streaming, headers, cookies, redirect, error(), hashed assets, and the single-origin API proxy. Deployed to a sandbox and verified end to end; e2e suite green (see osama-rizk/ssr-hosting-e2e sveltekit project).
… load The /about build timestamp changed on every reload even though S3 served frozen HTML: `new Date()` in the component <script> (and later a universal +page.ts load) re-ran during client hydration, overwriting the DOM with a fresh time — making a genuinely-frozen prerendered page LOOK dynamic. Move the timestamp into a +page.server.ts server load: it runs ONLY on the server (at build time for a prerendered page) and its result is serialized into the built HTML, never recomputed in the browser. Confirmed frozen across reloads in a real browser.
soberm
left a comment
soberm
left a comment
There was a problem hiding this comment.
Solid implementation overall — the transparent bridge design, the flat→directory-index normalization for prerendered pages, and the LWA env wiring are all done thoughtfully. The test app is comprehensive and the VERIFIED_SVELTEKIT_RANGE export + cross-adapter version-pin test is a nice pattern.
Two issues need addressing before merge: the version guard only enforces the floor of the verified range (SvelteKit 3.x will silently pass), and the bridge will fire — and silently replace — any non-adapter-node adapter, not just cases where no adapter is wired. See inline comments.
| const assertSvelteKitVersion = (projectDir: string): void => { | ||
| const info = getPackageInfoSync('@sveltejs/kit', { paths: [projectDir] }); | ||
| const version = info?.version; | ||
| if (!version || !semver.gte(version, '2.0.0')) { |
There was a problem hiding this comment.
CRITICAL: This only enforces the floor of VERIFIED_SVELTEKIT_RANGE. A SvelteKit 3.x project will pass this check silently even though the verified range caps at <3.0.0 and the adapter's build-layout assumptions haven't been validated against a new major.
Should be:
if (!version || !semver.satisfies(version, VERIFIED_SVELTEKIT_RANGE)) {
throw new HostingError('UnsupportedSvelteKitVersionError', {
message: `SvelteKit version ${version ?? '(not installed)'} is outside the verified range ${VERIFIED_SVELTEKIT_RANGE}.`,
...
});
}The version_pins.test.ts test asserts VERIFIED_SVELTEKIT_RANGE contains a < bound — the runtime guard should honor it too.
There was a problem hiding this comment.
Fixed in c57cf6c. assertSvelteKitVersion now uses semver.satisfies(version, VERIFIED_SVELTEKIT_RANGE) instead of the floor-only gte("2.0.0"), so a 3.x install is rejected until the range is deliberately bumped after re-verification. Added a unit test asserting an above-range major (3.0.0) throws UnsupportedSvelteKitVersionError.
| // node_modules presence is unreliable — adapter-node can land there | ||
| // transitively — so we key off the config text, the same signal Astro's | ||
| // adapter uses for its bridge decision. | ||
| const useBridge = !svelteConfigUsesAdapterNode(projectDir); |
There was a problem hiding this comment.
CRITICAL: svelteConfigUsesAdapterNode only scans for the string @sveltejs/adapter-node. If the user has a different adapter (@sveltejs/adapter-cloudflare, adapter-auto, a monorepo-local adapter), the scan returns false, useBridge = true, and the bridge silently installs adapter-node and replaces their configured adapter. The only signal is a process.stderr.write that fires inside the generated config during the build subprocess — after the user has already lost their original adapter choice.
Suggest checking whether the config references any @sveltejs/adapter-* import and erroring when a non-adapter-node adapter is detected rather than silently overwriting it:
const usesAnyAdapter = /@sveltejs\/adapter-/.test(configSrc);
if (usesAnyAdapter && !svelteConfigUsesAdapterNode(projectDir)) {
throw new HostingError('SvelteKitIncompatibleAdapterError', {
message: 'Your svelte.config uses an adapter incompatible with Lambda deployment.',
resolution: 'Switch to @sveltejs/adapter-node, or remove the adapter and let the hosting adapter install it.',
});
}There was a problem hiding this comment.
Fixed in c57cf6c. Added assertNoIncompatibleAdapter, called before the bridge decision: if the config references any @sveltejs/adapter-* that is not adapter-node, it throws SvelteKitIncompatibleAdapterError (switch to adapter-node, or remove the adapter and let hosting install it) rather than silently overwriting. adapter-node or no-adapter still flow through to the normal bridge path. Added a unit test with @sveltejs/adapter-cloudflare.
|
|
||
| const buildBridgeConfigSource = ( | ||
| originalConfigFile: string, | ||
| bodySizeLimit: number, |
There was a problem hiding this comment.
SUGGESTION: bodySizeLimit is accepted here but never referenced in the template string — it's a dead parameter. The value is correctly wired as a BODY_SIZE_LIMIT env var in buildManifest (the right place for a runtime env var), so the behavior is fine, but this signature implies the value affects the generated config source when it doesn't. Just drop the parameter.
There was a problem hiding this comment.
Fixed in c57cf6c. Dropped bodySizeLimit from buildBridgeConfigSource (and from installSvelteKitBridge, which only forwarded it). The value stays wired as the BODY_SIZE_LIMIT env var in buildManifest.
| if (has('yarn.lock')) { | ||
| return { command: 'yarn', args: ['add', '--silent', packageSpec] }; | ||
| } | ||
| if (has('bun.lockb') || has('bun.lock')) { |
There was a problem hiding this comment.
SUGGESTION: bun add <pkg> adds to dependencies, not devDependencies. @sveltejs/adapter-node is a dev dependency in every SvelteKit project. The npm path correctly uses --save-dev; Bun users would end up with it in dependencies.
return { command: 'bun', args: ['add', '-d', packageSpec] };There was a problem hiding this comment.
Fixed in c57cf6c. Bun now uses bun add -d. I applied the same reasoning to pnpm (-D) and yarn (--dev) since adapter-node is a devDependency in every SvelteKit project and those two would otherwise also land it in dependencies.
|
|
||
| // Park the original config under a real `.js` name (so the bridge can import | ||
| // it), then write the bridge into the original config's slot. | ||
| fs.renameSync(userConfigPath, backupPath); |
There was a problem hiding this comment.
SUGGESTION: BRIDGE_BACKUP_FILE is always svelte.config.blocks-original.js regardless of the original file's extension. When the original is svelte.config.mjs or svelte.config.ts, you rename a .mjs/.ts file to a .js path and the bridge imports it as ./svelte.config.blocks-original.js. This works today because Vite transpiles TypeScript content even from a .js path, but it's fragile — strict loader configs or future tooling changes could break it.
Consider preserving the original extension:
const backupExt = path.extname(userConfigPath) || '.js';
const backupPath = path.join(projectDir, `svelte.config.blocks-original${backupExt}`);and updating the import in buildBridgeConfigSource accordingly.
There was a problem hiding this comment.
Fixed in c57cf6c. Replaced the hardcoded BRIDGE_BACKUP_FILE with BRIDGE_BACKUP_BASENAME and now preserve the original extension: const backupExt = path.extname(userConfigPath) || ".js". So an .mjs/.ts source is parked as svelte.config.blocks-original.mjs/.ts, and the bridge imports that exact filename.
| for (const htmlFile of walkHtmlFiles(prerenderedDir)) { | ||
| const rel = path.relative(prerenderedDir, htmlFile).replace(/\\/g, '/'); | ||
| const urlPath = htmlFileToUrlPath(rel); | ||
| if (urlPath === '/') continue; // root is served by the catch-all / SSR |
There was a problem hiding this comment.
SUGGESTION: When / is prerendered it still gets merged into client/ (so the file lands in S3), but no static route is emitted for it — every homepage request hits Lambda. If this is intentional (to allow SSR to serve root even when a prerendered copy exists), worth a comment explaining why. If not, emitting { pattern: '/', target: 'static' } when build/prerendered/index.html exists would save a Lambda invocation on every homepage hit.
There was a problem hiding this comment.
Fixed in c57cf6c. It was not intentional — now emitting { pattern: "/", target: "static" } when the root is prerendered (it is already merged into client/ as index.html), so the homepage serves from S3 instead of the SSR catch-all. The bare / static route precedes the /* catch-all; deeper paths still reach SSR. Added a unit test for both the prerendered and non-prerendered cases.
| ); | ||
| }); | ||
| }); | ||
|
|
There was a problem hiding this comment.
SUGGESTION: The SvelteKitBridgeCollisionError path — thrown when svelte.config.blocks-original.js already exists on entry — has no unit test coverage. Given that the wrong behavior here would be silently overwriting the user's real config, a targeted test is worth adding: scaffold a project with a pre-existing backup file and assert the error is thrown before any file mutation occurs.
There was a problem hiding this comment.
Added in c57cf6c. New test scaffolds a project with a pre-existing svelte.config.blocks-original.js and asserts SvelteKitBridgeCollisionError is thrown with the active config and backup left byte-identical. I also moved the collision guard ahead of installAdapterNode so it truly fires before any mutation (the install writes package.json/node_modules).
| // from proxy headers so redirects / form-action URLs / cookies point | ||
| // at the viewer domain, not the raw Lambda host. The KVS router copies | ||
| // Host → x-forwarded-host on compute-bound requests. | ||
| // eslint-disable-next-line @typescript-eslint/naming-convention |
There was a problem hiding this comment.
NITPICK: Three consecutive eslint-disable-next-line comments for the same rule is noisy. A single block disable around the whole environment object reads cleaner:
/* eslint-disable @typescript-eslint/naming-convention */
environment: {
HOST_HEADER: 'x-forwarded-host',
PROTOCOL_HEADER: 'x-forwarded-proto',
BODY_SIZE_LIMIT: String(bodySizeLimit),
},
/* eslint-enable @typescript-eslint/naming-convention */There was a problem hiding this comment.
Fixed in c57cf6c. Collapsed the three eslint-disable-next-line comments into a single /* eslint-disable @typescript-eslint/naming-convention */ ... /* eslint-enable */ block around the environment object.
| "telemetry": { | ||
| "projectId": "1d97c564-fa80-410a-9dd0-59feaa841900" | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
NITPICK: Missing newline at end of file.
There was a problem hiding this comment.
Fixed in c57cf6c. Added the trailing newline.
- Enforce full VERIFIED_SVELTEKIT_RANGE in the version guard (reject 3.x, not just < 2.0) via semver.satisfies - Throw SvelteKitIncompatibleAdapterError when the config wires a non adapter-node adapter, instead of silently overwriting it - Check the bridge-backup collision guard before any mutation (install/ rename), so a collision aborts cleanly - Preserve the original svelte.config extension for the bridge backup (.mjs/.ts) rather than forcing .js - Pass each package manager's dev flag (npm --save-dev, pnpm -D, yarn --dev, bun -d) so adapter-node installs as a devDependency - Drop the dead bodySizeLimit param from buildBridgeConfigSource - Emit a static / route when the root page is prerendered, saving a Lambda invocation on every homepage request - Collapse the per-line eslint-disable comments into a single block - Add tests for the collision guard, incompatible-adapter rejection, above-range version rejection, and prerendered-root routing - Add trailing newline to the sveltekit test app config.json
🦋 Changeset detectedLatest commit: 9d9e4f5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 |
|
Thanks for the review! All feedback addressed in c57cf6c — replied inline on each thread. Summary: Critical
Suggestions/nitpicks
Validation
|
ahmedhamouda78
left a comment
ahmedhamouda78
left a comment
There was a problem hiding this comment.
Re-reviewed against the current head (post-c57cf6c). The c57cf6c round addressed the earlier feedback well — version ceiling, the incompatible-adapter guard, collision-guard ordering, extension preservation, per-PM dev flags, and the prerendered / route are all solid. Three issues remain that each have a realistic path to data loss or silent misbehaviour, plus a few minors/nits. All are targeted, low-risk fixes. Note: these paths are currently untested (unit tests use skipBuild: true), which is why CI is green despite them. Severity tags: [major] blocking, [minor] should-fix, [nit] optional.
|
|
||
| // Park the original config under its preserved name (so the bridge can import | ||
| // it), then write the bridge into the original config's slot. | ||
| fs.renameSync(userConfigPath, backupPath); |
There was a problem hiding this comment.
[major] Config left broken if the bridge write fails. installSvelteKitBridge does renameSync(userConfig → backup) then writeFileSync(userConfig, bridge). If the write throws (disk full, EROFS, permissions), the exception propagates before the cleanup closure is returned, so the caller’s finally { cleanupBridge?.() } never runs — the user’s config is stranded at the backup path with no svelte.config.* in place (recovery needs a manual rename). Restore before rethrowing:
fs.renameSync(userConfigPath, backupPath);
try {
fs.writeFileSync(userConfigPath, buildBridgeConfigSource(backupFile), 'utf-8');
} catch (writeErr) {
try { fs.renameSync(backupPath, userConfigPath); } catch { /* best-effort */ }
throw writeErr;
}This failure path is untested — all bridge tests use skipBuild: true.
There was a problem hiding this comment.
Fixed in 9d9e4f5. The writeFileSync is now wrapped in a try/catch that renames the backup back to the original path before rethrowing, exactly as suggested — so a failed write no longer strands the config at the backup path. (The restore is best-effort; if even that fails, the collision guard surfaces the leftover loudly on the next run.)
| */ | ||
| const prunePreCompressedAssets = (dir: string): void => { | ||
| if (!fs.existsSync(dir)) return; | ||
| const compressed = fg.sync('**/*.{gz,br,zst}', { |
There was a problem hiding this comment.
[major] prunePreCompressedAssets silently deletes user static files. The glob **/*.{gz,br,zst} over build/client/ matches everything, not just adapter-node’s precompress siblings. A user’s static/downloads/dataset.tar.gz or static/fonts/custom.woff2.br gets merged into client/ and then rmSync’d — the S3 upload is missing the file with no warning. Only delete when the uncompressed sibling exists:
for (const f of compressed) {
const base = f.replace(/\.(gz|br|zst)$/i, '');
if (fs.existsSync(base)) fs.rmSync(f);
}There was a problem hiding this comment.
Fixed in 9d9e4f5. prunePreCompressedAssets now only deletes a .gz/.br/.zst when its uncompressed sibling exists, so a user's standalone static/dataset.tar.gz or custom.woff2.br survives to S3. Added a unit test covering both the precompress-sibling (pruned) and standalone-archive (kept) cases.
| if (!configPath) return false; | ||
| try { | ||
| const src = fs.readFileSync(configPath, 'utf-8'); | ||
| return /@sveltejs\/adapter-node/.test(src); |
There was a problem hiding this comment.
[major] Adapter detection is defeated by commented-out imports (follow-up to the earlier adapter-detection thread). svelteConfigUsesAdapterNode (and assertNoIncompatibleAdapter at L293) run the regex over raw source including comments, so the assertNoIncompatibleAdapter guard added in c57cf6c is still bypassed. A commented-out adapter-node import with an active incompatible adapter:
// import node from '@sveltejs/adapter-node'; // old — switched to cloudflare
import cloudflare from '@sveltejs/adapter-cloudflare';makes svelteConfigUsesAdapterNode return true → bridge skipped AND the incompatible-adapter check short-circuits to false → build proceeds with the wrong adapter, producing an unrunnable Lambda bundle. The inverse (commented adapter, none active) throws a spurious error. Strip comments first:
const stripped = src.replace(/\/\/[^\n]*/g, '').replace(/\/\*[\s\S]*?\*\//g, '');Worth a test for 'stale adapter-node comment + active incompatible adapter'.
There was a problem hiding this comment.
Fixed in 9d9e4f5. Added a shared stripComments helper (+ readSvelteConfigSource) applied in svelteConfigUsesAdapterNode, assertNoIncompatibleAdapter, and textScanSvelteConfig. A commented-out adapter-node import no longer counts as "wired", so the incompatible-adapter guard fires correctly; the inverse (commented adapter, none active) no longer throws spuriously. Added tests for both directions using your exact stale-comment example.
| for (const entry of fs.readdirSync(clientDir, { withFileTypes: true })) { | ||
| if (entry.name === appDir) continue; // handled above | ||
| // Skip prerendered HTML index files — handled as page routes below. | ||
| if (entry.isFile() && entry.name.endsWith('.html')) continue; |
There was a problem hiding this comment.
[minor] Generic static .html still routed through Lambda (follow-up to the prerendered-/ fix). The prerendered-root fix is correct, but this loop skips ALL flat .html, so a user’s static/terms.html → client/terms.html gets no S3 route and falls through to the SSR Lambda (served, but wasted compute per hit). Narrow the skip to known files and route the rest to S3 — and fix the comment, which says 'prerendered HTML index files' but applies to everything:
const SKIP = new Set(['index.html', '404.html', '500.html']);
if (entry.isFile() && entry.name.endsWith('.html') && !SKIP.has(entry.name)) {
add({ pattern: `/${entry.name}`, target: 'static' });
continue;
}
if (entry.isFile() && entry.name.endsWith('.html')) continue;There was a problem hiding this comment.
Fixed in 9d9e4f5. The .html skip is narrowed to index.html/404.html/500.html (via a ROOT_AND_ERROR_HTML set); any other top-level flat .html (e.g. static/terms.html) now gets a { pattern: "/<name>", target: "static" } S3 route, and I corrected the misleading comment. Added a test asserting terms.html routes to S3 while the root/error pages do not get bare routes.
| buildCommand && buildCommand.length > 0 | ||
| ? buildCommand | ||
| : projectHasBuildScript(projectDir) | ||
| ? ['npm', 'run', 'build'] |
There was a problem hiding this comment.
[minor] Build step still hardcodes npm (follow-up to the install-step PM fix). c57cf6c made the install PM-aware via detectPackageManagerInstall, but runSvelteKitBuild still defaults to ['npm','run','build']. So a pnpm/yarn/bun project installs with the right manager then builds with npm — inconsistent, and it fails outright on Yarn PnP, strict pnpm (node-linker=isolated), or Bun-only CI without npm on PATH. Extract a detectPackageManagerRun(projectDir) twin and use it here.
There was a problem hiding this comment.
Fixed in 9d9e4f5. Extracted detectPackageManager and added a detectPackageManagerRun twin; runSvelteKitBuild now runs pnpm run / yarn <script> / bun run / npm run to match the detected manager, so install and build stay consistent (no npm on a Yarn PnP / strict-pnpm / Bun-only CI path).
| const out: SvelteConfigShape = { kit: {} }; | ||
| const appDir = src.match(/\bappDir\s*:\s*['"`]([^'"`]+)['"`]/); | ||
| if (appDir) out.kit!.appDir = appDir[1]; | ||
| const base = src.match(/\bbase\s*:\s*['"`]([^'"`]*)['"`]/); |
There was a problem hiding this comment.
[nit] textScanSvelteConfig base: regex matches comments/strings — it would extract /old from // base: '/old'. Only the jiti-failure fallback, but a wrong basePath → wrong CloudFront prefix → broken routing. Strip comments first (same helper as above).
There was a problem hiding this comment.
Fixed in 9d9e4f5 by the same stripComments helper — textScanSvelteConfig now scans comment-stripped source, so a commented-out base: '/old' no longer leaks into the resolved basePath.
|
|
||
| const installAdapterNode = (projectDir: string): void => { | ||
| // Idempotent: skip if adapter-node is already resolvable from the project. | ||
| if (getPackageInfoSync('@sveltejs/adapter-node', { paths: [projectDir] })) { |
There was a problem hiding this comment.
[nit] installAdapterNode skips install for ANY resolvable version, including a transitive ^4, whose build/ layout can differ from the pinned ^5. The @sveltejs/kit guard uses semver.satisfies; do the same here:
const info = getPackageInfoSync('@sveltejs/adapter-node', { paths: [projectDir] });
if (info?.version && semver.satisfies(info.version, '^5')) return;There was a problem hiding this comment.
Fixed in 9d9e4f5. installAdapterNode now skips only when the resolvable @sveltejs/adapter-node satisfies the pinned major (semver.satisfies(version, ADAPTER_NODE_RANGE) where ADAPTER_NODE_RANGE = "^5"), matching the @sveltejs/kit guard — a transitive ^4 is treated as not-installed and the pin is installed.
| // Preserve the original config's extension so the parked module resolves the | ||
| // same way the original did (an `.mjs`/`.ts` source keeps its loader | ||
| // semantics rather than being renamed to `.js`). | ||
| const backupExt = path.extname(userConfigPath) || '.js'; |
There was a problem hiding this comment.
[nit] Collision guard misses cross-extension stale backups. It keys backupPath off the current config’s extension, so a stale svelte.config.blocks-original.js from a prior crash is invisible once the user migrates to .mjs/.ts — the run proceeds and leaves an orphan backup. Check all backup-basename variants (over SVELTE_CONFIG_FILES extensions) instead.
There was a problem hiding this comment.
Fixed in 9d9e4f5. Added findStaleBridgeBackup, which checks every backup-basename variant across BRIDGE_BACKUP_EXTENSIONS (.js/.mjs/.ts) rather than only the current config's extension, so a stale .js backup is caught after the user migrates to .mjs/.ts. Added a test for the .js-backup + .mjs-config case.
- Restore the original svelte.config if the bridge write fails, so a disk-full/EROFS/permission error can't strand the user's config at the backup path (cleanup closure isn't returned yet at that point) - prunePreCompressedAssets only deletes a .gz/.br/.zst when its uncompressed sibling exists, so a user's standalone static archive (dataset.tar.gz, font.woff2.br) is no longer silently dropped - Strip comments before all svelte.config text scans, via a shared helper — a commented-out adapter-node import no longer defeats the incompatible-adapter guard, and a commented base:'/old' no longer leaks into the resolved basePath - Route generic top-level static .html (e.g. static/terms.html) to S3 instead of the SSR Lambda; skip only index/404/500 - Make the build command package-manager-aware (detectPackageManagerRun twin of the install detector), so pnpm/yarn/bun projects don't build with npm - installAdapterNode skips the install only when the resolvable adapter-node satisfies the pinned major (^5), matching the kit guard - Collision guard checks every backup extension variant, catching a stale .js backup after the user migrates the config to .mjs/.ts - Tests: comment-stripping (both directions), cross-extension collision, static-html routing, precompress-sibling prune
|
Thanks @ahmedhamouda78 — all 8 follow-up comments addressed in 9d9e4f5 (replied inline on each thread). Major
Minor
Nits
Validation
Rebased onto the latest branch (post main-merge) before pushing. |
ahmedhamouda78
left a comment
ahmedhamouda78
left a comment
There was a problem hiding this comment.
Re-reviewed after 9d9e4f5 — all 8 items from my previous review are resolved. Verified against the diff: the prunePreCompressedAssets sibling guard, the shared stripComments helper applied across svelteConfigUsesAdapterNode / assertNoIncompatibleAdapter / textScanSvelteConfig, the narrowed .html routing with static-HTML→S3, the cross-extension stale-backup scan, the semver-gated adapter-node install, the writeFileSync restore-on-failure, and detectPackageManagerRun for the build step are all correctly implemented, with new tests covering the sibling-prune, both comment-detection directions, terms.html→S3, and the cross-extension backup cases. Nice work. Approving. 🚢
| @@ -0,0 +1,5 @@ | |||
| { | |||
| "telemetry": { | |||
| "projectId": "1d97c564-fa80-410a-9dd0-59feaa841900" | |||
There was a problem hiding this comment.
[LOW] This projectId looks like a real UUID (1d97c564-fa80-410a-9dd0-59feaa841900). The sibling test apps (hosting-ssr-astro, hosting-ssr-nuxt) both commit the shared placeholder 00000000-0000-0000-0000-000000006e2e for exactly this reason — a realistic-looking ID risks sending test-app telemetry to a live project. Should swap it for the placeholder.
| @@ -0,0 +1,18 @@ | |||
| <script lang="ts"> | |||
| import type { PageData } from './$types'; | |||
| // Frozen at build time via the prerendered load (see +page.ts). A fresh | |||
There was a problem hiding this comment.
[NITPICK] Comment says "(see +page.ts)" but there's no +page.ts in this directory — the load lives in +page.server.ts. Should be "(see +page.server.ts)".
|
[LOW] The Overview still reads "(Next.js, Nuxt, Astro, SPA)" and the sub-path import example on line 54 only pulls in Fix: add SvelteKit to the prose list → "Next.js, Nuxt, Astro, SvelteKit, SPA" and add (Note: posted here because the README wasn't modified in this PR and GitHub doesn't allow inline comments on files outside the diff.) |
|
@osama-rizk nice work on the follow-ups 👍 The two hard blockers from my earlier pass are gone: precompressed pruning now checks for an uncompressed sibling, and adapter rejection fails loudly instead of silently swapping. I re-checked the full diff after 9d9e4f5; here's what still stands. Nothing left is data-loss or silent-failure class, so treat these as comment-level feedback, not blockers 🙂 Design questions
Smaller stuff
Repo hygiene
None of these block the merge from my side. Nice adapter overall, the flat→directory-index normalization and the version-pin gate are 💯 |
4 participants
What
Adds a first-class SvelteKit adapter to
@aws-blocks/hosting. SvelteKit apps are now auto-detected and deployed with the one-command flow — no manual configuration — joining the existing SPA, Next.js, Nuxt/Nitro, and Astro adapters.Approach
SvelteKit's official
@sveltejs/adapter-nodeemits a standardnode buildHTTP server, which we run on Lambda via the existinghttp-servercompute type + Lambda Web Adapter — the same proven path as the Astro (@astrojs/node) and Nitronode-serveradapters. This gives response streaming, tracks SvelteKit 2.x, and keeps custom runtime code minimal.Because SvelteKit has no
--configbuild flag, a transparent bridge temporarily rewritessvelte.config.jsto force@sveltejs/adapter-nodewhen the app hasn't wired it (auto-installing the dep via the detected package manager), and restores the original in afinally. The sample app configures adapter-node directly, so the happy path never mutates source.Changes
packages/hosting/src/adapters/sveltekit.ts— version guard (≥2.0), transparent bridge, build runner, andDeployManifestbuilder:http-servercompute withHOST_HEADER/PROTOCOL_HEADER/BODY_SIZE_LIMITLWA env (correct absolute URLs/redirects/cookies behind CloudFront; 20 MB body limit vs adapter-node's 512 KB default)_app/immutable/*immutable asset caching, prerendered→S3 routing, error pages,paths.baseabout.html) into directory-index form (about/index.html) so the L3 router's extensionless lookup (/about→about/index.html) resolves on S3@sveltejs/enhanced-imgis build-time only)@sveltejs/kitframework detection,FrameworkTypeunion member, andbuilddefault output dir (packages/core/src/hosting.ts)sveltekit.test.ts,detectFrameworkcases, cross-adapter version-pin gatetest-apps/hosting-ssr-sveltekit/— sample app exercising SSR, prerender/SSG,+server.js(GET/POST/PUT/DELETE), form actions, streaming, custom headers, cookies viahooks.server, server redirect,error(), hashed assets, and the single-origin/aws-blocks/*API proxypatchper the pre-1.0 caret convention — additive/backward-compatible)Validation
CREATE/UPDATE_COMPLETE); every feature confirmed live via curl on the CloudFront URLsveltekitproject inosama-rizk/ssr-hosting-e2e; suite + config changes are in that repo, not this PR)Test matrix
A SvelteKit per-framework matrix section is included at
test-apps/hosting-ssr-sveltekit/TEST-MATRIX.mdfor appending tossr-hosting-foundations/docs/01-test-matrix.md.Notes
sveltekit.spec.ts, anapp-basediscovery marker, and a fair fix to the shared M21.3 Range assertion — a 206 with validContent-Rangeis the real signal; S3 omitsaccept-rangeson the partial response) live in the separatessr-hosting-e2erepo.build/,.svelte-kit/) are excluded via a per-app.gitignoremirroring the Nuxt/Astro apps.