diff --git a/.changeset/brown-ads-allow.md b/.changeset/brown-ads-allow.md new file mode 100644 index 000000000000..c386b82d4d65 --- /dev/null +++ b/.changeset/brown-ads-allow.md @@ -0,0 +1,5 @@ +--- +'@sveltejs/kit': patch +--- + +fix: precedence of `entries` should be `+page => +page.server => +server` diff --git a/.changeset/polite-months-scream.md b/.changeset/polite-months-scream.md new file mode 100644 index 000000000000..accb35751666 --- /dev/null +++ b/.changeset/polite-months-scream.md @@ -0,0 +1,5 @@ +--- +'@sveltejs/kit': patch +--- + +fix: prerendered routes will throw when exposing both a GET handler and a page, preventing impossible content negotiation diff --git a/packages/kit/src/core/postbuild/analyse.js b/packages/kit/src/core/postbuild/analyse.js index c311dfe43337..c867841447f3 100644 --- a/packages/kit/src/core/postbuild/analyse.js +++ b/packages/kit/src/core/postbuild/analyse.js @@ -21,10 +21,11 @@ export default forked(import.meta.url, analyse); /** * @param {{ * manifest_path: string; + * manifest_data: import('types').ManifestData; * env: Record * }} opts */ -async function analyse({ manifest_path, env }) { +async function analyse({ manifest_path, manifest_data, env }) { /** @type {import('@sveltejs/kit').SSRManifest} */ const manifest = (await import(pathToFileURL(manifest_path).href)).manifest; @@ -66,31 +67,38 @@ async function analyse({ manifest_path, env }) { // analyse routes for (const route of manifest._.routes) { + const route_data = /** @type {import('types').RouteData} */ ( + manifest_data.routes.find((r) => r.id === route.id) + ); + /** @type {Array<'GET' | 'POST'>} */ const page_methods = []; /** @type {(import('types').HttpMethod | '*')[]} */ const api_methods = []; - /** @type {import('types').PrerenderOption | undefined} */ - let prerender = undefined; - /** @type {any} */ - let config = undefined; - /** @type {import('types').PrerenderEntryGenerator | undefined} */ - let entries = undefined; + /** @type {{ endpoint?: import('types').PrerenderOption, page?: import('types').PrerenderOption }} */ + const prerender_option = {}; + /** @type {{ endpoint?: unknown, page?: unknown }} */ + const config_option = {}; + /** @type {{ endpoint?: import('types').PrerenderEntryGenerator | undefined, page?: import('types').PrerenderEntryGenerator | undefined }} */ + const entries_option = {}; if (route.endpoint) { const mod = await route.endpoint(); if (mod.prerender !== undefined) { validate_server_exports(mod, route.id); - if (mod.prerender && (mod.POST || mod.PATCH || mod.PUT || mod.DELETE)) { + if ( + mod.prerender && + (mod.POST || mod.PATCH || mod.PUT || mod.DELETE || mod.HEAD || mod.OPTIONS) + ) { throw new Error( - `Cannot prerender a +server file with POST, PATCH, PUT, or DELETE (${route.id})` + `Cannot prerender ${route_data.endpoint?.file} as it exposes POST, PATCH, PUT, DELETE, HEAD, or OPTIONS handlers` ); } - prerender = mod.prerender; + prerender_option.endpoint = mod.prerender; } Object.values(mod).forEach((/** @type {import('types').HttpMethod} */ method) => { @@ -101,8 +109,8 @@ async function analyse({ manifest_path, env }) { } }); - config = mod.config; - entries = mod.entries; + config_option.endpoint = mod.config; + entries_option.endpoint = mod.entries; } if (route.page) { @@ -130,10 +138,25 @@ async function analyse({ manifest_path, env }) { validate_page_exports(page.universal, page.universal_id); } - prerender = get_option(nodes, 'prerender') ?? false; + prerender_option.page = get_option(nodes, 'prerender'); + + config_option.page = get_config(nodes); + entries_option.page = get_option(nodes, 'entries'); + } + + const prerender = prerender_option.page ?? prerender_option.endpoint ?? false; + const config = config_option.page ?? entries_option.endpoint; + const entries = entries_option.page ?? entries_option.endpoint; - config = get_config(nodes); - entries ??= get_option(nodes, 'entries'); + if (prerender === true && route.endpoint && route.page) { + const page = /** @type {string} */ ( + route_data.leaf?.component ?? route_data.leaf?.universal ?? route_data.leaf?.server + ); + const endpoint = /** @type {string} */ (route_data.endpoint?.file); + + throw new Error( + `Cannot prerender both ${page} and ${endpoint}. Disable prerendering for this route, or delete one of the files.` + ); } metadata.routes.set(route.id, { diff --git a/packages/kit/src/exports/vite/dev/index.js b/packages/kit/src/exports/vite/dev/index.js index a70be05c9cc8..037d8312a613 100644 --- a/packages/kit/src/exports/vite/dev/index.js +++ b/packages/kit/src/exports/vite/dev/index.js @@ -150,6 +150,8 @@ export async function dev(vite, vite_config, svelte_config) { return module.default; }; + + result.component_id = node.component; } if (node.universal) { diff --git a/packages/kit/src/exports/vite/index.js b/packages/kit/src/exports/vite/index.js index 426dd36d265d..5b368281eb56 100644 --- a/packages/kit/src/exports/vite/index.js +++ b/packages/kit/src/exports/vite/index.js @@ -687,6 +687,7 @@ function kit({ svelte_config }) { const metadata = await analyse({ manifest_path, + manifest_data, env: { ...env.private, ...env.public } }); diff --git a/packages/kit/src/runtime/server/respond.js b/packages/kit/src/runtime/server/respond.js index 7967658c77f1..49b84d98b48b 100644 --- a/packages/kit/src/runtime/server/respond.js +++ b/packages/kit/src/runtime/server/respond.js @@ -390,6 +390,25 @@ export async function respond(request, options, manifest, state) { /** @type {Response} */ let response; + if (DEV && route.page && route.endpoint) { + const nodes = await Promise.all( + [...route.page.layouts, route.page.leaf].map((n) => { + if (n !== undefined) return manifest._.nodes[n](); + }) + ); + + const mod = await route.endpoint(); + + if (get_option(nodes, 'prerender') ?? mod.prerender) { + const page = nodes.at(-1)?.component_id; + const endpoint = route.endpoint_id; + + throw new Error( + `Cannot prerender both ${page} and ${endpoint}. Disable prerendering for this route, or delete one of the files.` + ); + } + } + if (is_data_request) { response = await render_data( event, diff --git a/packages/kit/src/types/internal.d.ts b/packages/kit/src/types/internal.d.ts index 84ffcd4bc4ef..6c7a3c45e18c 100644 --- a/packages/kit/src/types/internal.d.ts +++ b/packages/kit/src/types/internal.d.ts @@ -323,6 +323,7 @@ export interface SSRNode { entries?: PrerenderEntryGenerator; }; + component_id: string; universal_id: string; server_id: string; } diff --git a/packages/kit/test/build-errors/apps/prerender-entry-generator-mismatch/package.json b/packages/kit/test/build-errors/apps/prerender-entry-generator-mismatch/package.json index ca443111f8ee..99eca99774da 100644 --- a/packages/kit/test/build-errors/apps/prerender-entry-generator-mismatch/package.json +++ b/packages/kit/test/build-errors/apps/prerender-entry-generator-mismatch/package.json @@ -1,5 +1,5 @@ { - "name": "prerenderable-incorrect-fragment", + "name": "prerenderable-entry-generator-mismatch", "private": true, "version": "0.0.1", "scripts": { diff --git a/packages/kit/test/build-errors/apps/prerender-impossible-content-negotiation/package.json b/packages/kit/test/build-errors/apps/prerender-impossible-content-negotiation/package.json new file mode 100644 index 000000000000..b168ca71f9a0 --- /dev/null +++ b/packages/kit/test/build-errors/apps/prerender-impossible-content-negotiation/package.json @@ -0,0 +1,20 @@ +{ + "name": "prerender-impossible-content-negotiation", + "private": true, + "version": "0.0.1", + "scripts": { + "dev": "vite dev", + "build": "vite build", + "preview": "vite preview", + "check": "svelte-kit sync && tsc && svelte-check" + }, + "devDependencies": { + "@sveltejs/adapter-auto": "workspace:^", + "@sveltejs/kit": "workspace:^", + "svelte": "^3.56.0", + "svelte-check": "^3.0.2", + "typescript": "^4.9.4", + "vite": "^4.3.6" + }, + "type": "module" +} diff --git a/packages/kit/test/build-errors/apps/prerender-impossible-content-negotiation/src/app.html b/packages/kit/test/build-errors/apps/prerender-impossible-content-negotiation/src/app.html new file mode 100644 index 000000000000..5b53ef7e3ae7 --- /dev/null +++ b/packages/kit/test/build-errors/apps/prerender-impossible-content-negotiation/src/app.html @@ -0,0 +1,12 @@ + + + + + + + %sveltekit.head% + + +
%sveltekit.body%
+ + diff --git a/packages/kit/test/build-errors/apps/prerender-impossible-content-negotiation/src/routes/+page.svelte b/packages/kit/test/build-errors/apps/prerender-impossible-content-negotiation/src/routes/+page.svelte new file mode 100644 index 000000000000..159202e879ac --- /dev/null +++ b/packages/kit/test/build-errors/apps/prerender-impossible-content-negotiation/src/routes/+page.svelte @@ -0,0 +1 @@ +

Hello world

diff --git a/packages/kit/test/build-errors/apps/prerender-impossible-content-negotiation/src/routes/+page.ts b/packages/kit/test/build-errors/apps/prerender-impossible-content-negotiation/src/routes/+page.ts new file mode 100644 index 000000000000..189f71e2e1b3 --- /dev/null +++ b/packages/kit/test/build-errors/apps/prerender-impossible-content-negotiation/src/routes/+page.ts @@ -0,0 +1 @@ +export const prerender = true; diff --git a/packages/kit/test/build-errors/apps/prerender-impossible-content-negotiation/src/routes/+server.ts b/packages/kit/test/build-errors/apps/prerender-impossible-content-negotiation/src/routes/+server.ts new file mode 100644 index 000000000000..a394a9f1528a --- /dev/null +++ b/packages/kit/test/build-errors/apps/prerender-impossible-content-negotiation/src/routes/+server.ts @@ -0,0 +1 @@ +export function GET() {} diff --git a/packages/kit/test/build-errors/apps/prerender-impossible-content-negotiation/static/favicon.png b/packages/kit/test/build-errors/apps/prerender-impossible-content-negotiation/static/favicon.png new file mode 100644 index 000000000000..825b9e65af7c Binary files /dev/null and b/packages/kit/test/build-errors/apps/prerender-impossible-content-negotiation/static/favicon.png differ diff --git a/packages/kit/test/build-errors/apps/prerender-impossible-content-negotiation/svelte.config.js b/packages/kit/test/build-errors/apps/prerender-impossible-content-negotiation/svelte.config.js new file mode 100644 index 000000000000..6e60dbbbdd9c --- /dev/null +++ b/packages/kit/test/build-errors/apps/prerender-impossible-content-negotiation/svelte.config.js @@ -0,0 +1,10 @@ +import adapter from '../../../../../adapter-auto/index.js'; + +/** @type {import('@sveltejs/kit').Config} */ +const config = { + kit: { + adapter: adapter() + } +}; + +export default config; diff --git a/packages/kit/test/build-errors/apps/prerender-impossible-content-negotiation/tsconfig.json b/packages/kit/test/build-errors/apps/prerender-impossible-content-negotiation/tsconfig.json new file mode 100644 index 000000000000..95d9c037df6c --- /dev/null +++ b/packages/kit/test/build-errors/apps/prerender-impossible-content-negotiation/tsconfig.json @@ -0,0 +1,15 @@ +{ + "compilerOptions": { + "allowJs": true, + "checkJs": true, + "noEmit": true, + "module": "esnext", + "moduleResolution": "node", + "paths": { + "@sveltejs/kit": ["../../../../types"], + "$lib": ["./src/lib"], + "$lib/*": ["./src/lib/*"] + } + }, + "extends": "./.svelte-kit/tsconfig.json" +} diff --git a/packages/kit/test/build-errors/apps/prerender-impossible-content-negotiation/vite.config.js b/packages/kit/test/build-errors/apps/prerender-impossible-content-negotiation/vite.config.js new file mode 100644 index 000000000000..3f12d3677ea6 --- /dev/null +++ b/packages/kit/test/build-errors/apps/prerender-impossible-content-negotiation/vite.config.js @@ -0,0 +1,23 @@ +import * as path from 'node:path'; +import { sveltekit } from '@sveltejs/kit/vite'; + +/** @type {import('vite').UserConfig} */ +const config = { + build: { + minify: false + }, + + clearScreen: false, + + logLevel: 'silent', + + plugins: [sveltekit()], + + server: { + fs: { + allow: [path.resolve('../../../../src')] + } + } +}; + +export default config; diff --git a/packages/kit/test/build-errors/prerender.spec.js b/packages/kit/test/build-errors/prerender.spec.js index 848de79f8492..684bb72dce4f 100644 --- a/packages/kit/test/build-errors/prerender.spec.js +++ b/packages/kit/test/build-errors/prerender.spec.js @@ -26,3 +26,15 @@ test('entry generators should match their own route', () => { `Error: The entries export from /[slug]/[notSpecific] generated entry /whatever/specific, which was matched by /[slug]/specific - see the \`handleEntryGeneratorMismatch\` option in https://kit.svelte.dev/docs/configuration#prerender for more info.${EOL}To suppress or handle this error, implement \`handleEntryGeneratorMismatch\` in https://kit.svelte.dev/docs/configuration#prerender` ); }); + +test('prerendering a route with +page and +server files should fail due to impossible content negotiation', () => { + assert.throws( + () => + execSync('pnpm build', { + cwd: path.join(process.cwd(), 'apps/prerender-impossible-content-negotiation'), + stdio: 'pipe', + timeout: 60000 + }), + 'Detected a prerendered route with both a +server and +page file (route: /). Because content negotiation for static files is impossible, this is not allowed.' + ); +}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 007ebb344126..f25b6f342d2a 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -630,6 +630,27 @@ importers: specifier: ^4.4.9 version: 4.4.9(@types/node@16.18.6)(lightningcss@1.21.8) + packages/kit/test/build-errors/apps/prerender-impossible-content-negotiation: + devDependencies: + '@sveltejs/adapter-auto': + specifier: workspace:^ + version: link:../../../../../adapter-auto + '@sveltejs/kit': + specifier: workspace:^ + version: link:../../../.. + svelte: + specifier: ^3.56.0 + version: 3.56.0 + svelte-check: + specifier: ^3.0.2 + version: 3.4.4(postcss@8.4.31)(svelte@3.56.0) + typescript: + specifier: ^4.9.4 + version: 4.9.4 + vite: + specifier: ^4.3.6 + version: 4.4.9(@types/node@16.18.6)(lightningcss@1.21.8) + packages/kit/test/build-errors/apps/prerenderable-incorrect-fragment: devDependencies: '@sveltejs/adapter-auto': @@ -5738,6 +5759,33 @@ packages: resolution: {integrity: sha512-ot0WnXS9fgdkgIcePe6RHNk1WA8+muPa6cSjeR3V8K27q9BB1rTE3R1p7Hv0z1ZyAc8s6Vvv8DIyWf681MAt0w==} engines: {node: '>= 0.4'} + /svelte-check@3.4.4(postcss@8.4.31)(svelte@3.56.0): + resolution: {integrity: sha512-Uys9+R65cj8TmP8f5UpS7B2xKpNLYNxEWJsA5ZoKcWq/uwvABFF7xS6iPQGLoa7hxz0DS6xU60YFpmq06E4JxA==} + hasBin: true + peerDependencies: + svelte: ^3.55.0 || ^4.0.0-next.0 || ^4.0.0 + dependencies: + '@jridgewell/trace-mapping': 0.3.19 + chokidar: 3.5.3 + fast-glob: 3.3.1 + import-fresh: 3.3.0 + picocolors: 1.0.0 + sade: 1.8.1 + svelte: 3.56.0 + svelte-preprocess: 5.0.4(postcss@8.4.31)(svelte@3.56.0)(typescript@5.0.4) + typescript: 5.0.4 + transitivePeerDependencies: + - '@babel/core' + - coffeescript + - less + - postcss + - postcss-load-config + - pug + - sass + - stylus + - sugarss + dev: true + /svelte-check@3.4.4(postcss@8.4.31)(svelte@4.1.2): resolution: {integrity: sha512-Uys9+R65cj8TmP8f5UpS7B2xKpNLYNxEWJsA5ZoKcWq/uwvABFF7xS6iPQGLoa7hxz0DS6xU60YFpmq06E4JxA==} hasBin: true @@ -5751,8 +5799,8 @@ packages: picocolors: 1.0.0 sade: 1.8.1 svelte: 4.1.2 - svelte-preprocess: 5.0.4(postcss@8.4.31)(svelte@4.1.2)(typescript@5.2.2) - typescript: 5.2.2 + svelte-preprocess: 5.0.4(postcss@8.4.31)(svelte@4.1.2)(typescript@5.0.4) + typescript: 5.0.4 transitivePeerDependencies: - '@babel/core' - coffeescript @@ -5800,6 +5848,54 @@ packages: svelte: 4.2.0 dev: true + /svelte-preprocess@5.0.4(postcss@8.4.31)(svelte@3.56.0)(typescript@5.0.4): + resolution: {integrity: sha512-ABia2QegosxOGsVlsSBJvoWeXy1wUKSfF7SWJdTjLAbx/Y3SrVevvvbFNQqrSJw89+lNSsM58SipmZJ5SRi5iw==} + engines: {node: '>= 14.10.0'} + requiresBuild: true + peerDependencies: + '@babel/core': ^7.10.2 + coffeescript: ^2.5.1 + less: ^3.11.3 || ^4.0.0 + postcss: ^7 || ^8 + postcss-load-config: ^2.1.0 || ^3.0.0 || ^4.0.0 + pug: ^3.0.0 + sass: ^1.26.8 + stylus: ^0.55.0 + sugarss: ^2.0.0 || ^3.0.0 || ^4.0.0 + svelte: ^3.23.0 || ^4.0.0-next.0 || ^4.0.0 + typescript: '>=3.9.5 || ^4.0.0 || ^5.0.0' + peerDependenciesMeta: + '@babel/core': + optional: true + coffeescript: + optional: true + less: + optional: true + postcss: + optional: true + postcss-load-config: + optional: true + pug: + optional: true + sass: + optional: true + stylus: + optional: true + sugarss: + optional: true + typescript: + optional: true + dependencies: + '@types/pug': 2.0.6 + detect-indent: 6.1.0 + magic-string: 0.27.0 + postcss: 8.4.31 + sorcery: 0.11.0 + strip-indent: 3.0.0 + svelte: 3.56.0 + typescript: 5.0.4 + dev: true + /svelte-preprocess@5.0.4(postcss@8.4.31)(svelte@4.1.2)(typescript@4.9.4): resolution: {integrity: sha512-ABia2QegosxOGsVlsSBJvoWeXy1wUKSfF7SWJdTjLAbx/Y3SrVevvvbFNQqrSJw89+lNSsM58SipmZJ5SRi5iw==} engines: {node: '>= 14.10.0'} @@ -5848,7 +5944,7 @@ packages: typescript: 4.9.4 dev: true - /svelte-preprocess@5.0.4(postcss@8.4.31)(svelte@4.1.2)(typescript@5.2.2): + /svelte-preprocess@5.0.4(postcss@8.4.31)(svelte@4.1.2)(typescript@5.0.4): resolution: {integrity: sha512-ABia2QegosxOGsVlsSBJvoWeXy1wUKSfF7SWJdTjLAbx/Y3SrVevvvbFNQqrSJw89+lNSsM58SipmZJ5SRi5iw==} engines: {node: '>= 14.10.0'} requiresBuild: true @@ -5893,7 +5989,7 @@ packages: sorcery: 0.11.0 strip-indent: 3.0.0 svelte: 4.1.2 - typescript: 5.2.2 + typescript: 5.0.4 dev: true /svelte2tsx@0.6.19(svelte@4.1.2)(typescript@4.9.4): @@ -5908,6 +6004,11 @@ packages: typescript: 4.9.4 dev: false + /svelte@3.56.0: + resolution: {integrity: sha512-LvXiJbjdvJKwB/0CQyYpDX0q+hFqCyWmybzC2G6eK1tJJA/RSRCytTfNmjHv+RHlLuA70vWG7nXp6gbeErYvRA==} + engines: {node: '>= 8'} + dev: true + /svelte@4.1.2: resolution: {integrity: sha512-/evA8U6CgOHe5ZD1C1W3va9iJG7mWflcCdghBORJaAhD2JzrVERJty/2gl0pIPrJYBGZwZycH6onYf+64XXF9g==} engines: {node: '>=16'} @@ -6230,12 +6331,6 @@ packages: engines: {node: '>=12.20'} hasBin: true - /typescript@5.2.2: - resolution: {integrity: sha512-mI4WrpHsbCIcwT9cF4FZvr80QUeKvsUsUvKDoR+X/7XHQH98xYD8YHZg7ANtz2GtZt/CBq2QJ0thkGJMHfqc1w==} - engines: {node: '>=14.17'} - hasBin: true - dev: true - /ufo@1.3.0: resolution: {integrity: sha512-bRn3CsoojyNStCZe0BG0Mt4Nr/4KF+rhFlnNXybgqt5pXHNFRlqinSoQaTrGyzE4X8aHplSb+TorH+COin9Yxw==} dev: true