Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/kit-17442-compiled-params.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@sveltejs/kit': patch
---

fix: validate compiled parameter matchers during builds
5 changes: 2 additions & 3 deletions packages/kit/src/core/params.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
/** @import { ParamMatcher } from '@sveltejs/kit/params' */
import path from 'node:path';
import { pathToFileURL } from 'node:url';
import * as e from '../messages/build-errors.js';

/**
Expand Down Expand Up @@ -38,7 +37,7 @@ export function validate_param_matchers(params, names, file) {
* routes: import('types').RouteData[];
* params_path: string | null;
* root: string;
* load?: (file: string) => Promise<Record<string, unknown>>;
* load: (file: string) => Promise<Record<string, unknown>>;
* }} opts
* @returns {Promise<Record<string, ParamMatcher> | null>}
*/
Expand All @@ -52,7 +51,7 @@ export async function load_and_validate_params({ routes, params_path, root, load
}

const file = path.resolve(root, params_path);
const module = load ? await load(file) : await import(pathToFileURL(file).href);
const module = await load(file);

if (!module.params || typeof module.params !== 'object') {
e.params_export_missing({ file: params_path });
Expand Down
16 changes: 15 additions & 1 deletion packages/kit/src/core/params.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ test('load_and_validate_params requires a params file for matchers', async () =>
})
],
params_path: null,
root: import.meta.dirname
root: import.meta.dirname,
load: () => Promise.resolve({})
})
).rejects.toThrowKitError('param_matcher_missing', { contains: ['`number`'] });
});
Expand Down Expand Up @@ -71,3 +72,16 @@ test('load_and_validate_params loads and validates params', async () => {

expect(params).toEqual({ number: expect.any(Function) });
});

test('load_and_validate_params does not load unused matchers', async () => {
const params = await load_and_validate_params({
routes: [],
params_path: 'params.ts',
root: import.meta.dirname,
load: () => {
throw new Error('Unused matchers should not be loaded');
}
});

expect(params).toBeNull();
});
10 changes: 10 additions & 0 deletions packages/kit/src/core/postbuild/analyse.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
/** @import { RemoteChunk } from 'types' */
import { join } from 'node:path';
import process from 'node:process';
import { pathToFileURL } from 'node:url';
import { validate_server_exports } from '../../utils/exports.js';
import { extract_svelte_config, load_vite_config } from '../config/index.js';
import { forked } from '../../utils/fork.js';
import { BODY_DEPENDENT_METHODS, ENDPOINT_METHODS } from '../../constants.js';
import { has_server_load, resolve_route } from '../../utils/routing.js';
import { check_feature } from '../features.js';
import { load_and_validate_params } from '../params.js';
import { createReadableStream } from '@sveltejs/kit/node';
import { PageNodes } from '../../utils/page_nodes.js';
import { enable_verbose_errors } from '../../messages/internal/shared.js';
Expand Down Expand Up @@ -59,6 +61,14 @@ async function analyse({
read: (file) => createReadableStream(`${server_root}/server/${file}`)
});

// Validate the compiled matchers after initialising their build-time state.
await load_and_validate_params({
routes: manifest_data.routes,
params_path: manifest_data.params,
root: vite_config.root ?? process.cwd(),
load: async () => ({ params: await manifest.matchers() })
});

/** @type {import('types').ServerMetadata} */
const metadata = {
nodes: [],
Expand Down
15 changes: 1 addition & 14 deletions packages/kit/src/exports/vite/build/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import { write_client_manifest } from '../../../core/sync/write_client_manifest.
import { logger, runtime_directory } from '../../../core/utils.js';
import { compact } from '../../../utils/array.js';
import { copy, read, resolve_entry, walk } from '../../../utils/filesystem.js';
import { load_and_validate_params } from '../../../core/params.js';
import { posixify } from '../../../utils/os.js';
import { stackless } from '../../../utils/error.js';
import * as e from '../../../messages/build-errors.js';
Expand Down Expand Up @@ -928,19 +927,13 @@ export function plugin_compile(
// `vite build`
ssr_build = Array.isArray(ssr_build) ? ssr_build[0] : ssr_build;
if ('output' in ssr_build) {
await load_and_validate_params({
routes: manifest_data.routes,
params_path: manifest_data.params,
root
});

return await process_ssr_build(ssr_build.output);
}

// `vite build --watch`
let rebuild = false;

const before_ssr_build_rerun = async () => {
const before_ssr_build_rerun = () => {
rebuild = true;

// these are set once per plugin initialisation or when the config hook
Expand All @@ -955,12 +948,6 @@ export function plugin_compile(
finalise = null;

fs.mkdirSync(out, { recursive: true });

await load_and_validate_params({
routes: manifest_data.routes,
params_path: manifest_data.params,
root
});
};

ssr_build.on('change', before_ssr_build_rerun);
Expand Down
1 change: 1 addition & 0 deletions packages/kit/test/apps/basics/src/lib/fruit-values.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const fruits: string[] = ['apple', 'orange'];
1 change: 1 addition & 0 deletions packages/kit/test/apps/basics/src/lib/fruits.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { fruits } from './fruit-values.js';
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { defineParams } from '@sveltejs/kit/params';
import { fruits } from '#lib/fruits.js';

export const params = defineParams({
lowercase: (param) => {
Expand All @@ -14,5 +15,8 @@ export const params = defineParams({
if (isNaN(value)) return;
return value;
},
bigint: (param) => BigInt(param)
bigint: (param) => BigInt(param),
fruit: (param) => {
if (fruits.includes(param)) return param;
}
});
4 changes: 3 additions & 1 deletion packages/kit/test/apps/basics/src/routes/match/const.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,7 @@ export const testPaths = [
'/match/slug/test-slug',
'/match/slug/test-slug?from=query',
'/match/not-a-real-route-that-exists',
'/reroute/basic/a'
'/reroute/basic/a',
'/routing/matched/fruits/apple',
'/routing/matched/fruits/rocket'
];
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<script>
import { page } from '$app/state';
</script>

<h1>fruit: {page.params.name}</h1>
<a href="/routing/matched/fruits/orange">orange</a>
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,16 @@ test.skip(({ javaScriptEnabled }) => javaScriptEnabled);

test.describe.configure({ mode: 'parallel' });

test('Server-side match() uses TypeScript params and their imports', async ({ page }) => {
await page.goto('/match');

const results = page.locator('#server-results');
await expect(results.locator('[data-path="/routing/matched/fruits/apple"]')).toHaveText(
JSON.stringify({ id: '/routing/matched/fruits/[name=fruit]', params: { name: 'apple' } })
);
await expect(results.locator('[data-path="/routing/matched/fruits/rocket"]')).toHaveText('null');
});

test.describe('Static files', () => {
test('does not serve files outside the prerendered directory', async ({ request }) => {
test.skip(!!process.env.DEV);
Expand Down
15 changes: 15 additions & 0 deletions packages/kit/test/apps/basics/test/playwright/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1093,6 +1093,21 @@ test.describe('searchParams', () => {
});

test.describe('Matchers', () => {
test('Loads TypeScript params and their imports with .js specifiers', async ({
page,
clicknav
}) => {
const response = await page.goto('/routing/matched/fruits/apple');
expect(response?.status()).toBe(200);
expect(await page.textContent('h1')).toBe('fruit: apple');

await clicknav('[href="/routing/matched/fruits/orange"]');
expect(await page.textContent('h1')).toBe('fruit: orange');

const invalid = await page.goto('/routing/matched/fruits/rocket');
expect(invalid?.status()).toBe(404);
});

test('Matches parameters', async ({ page, clicknav }) => {
await page.goto('/routing/matched');

Expand Down
Loading