diff --git a/.changeset/forty-icons-switch.md b/.changeset/forty-icons-switch.md new file mode 100644 index 000000000000..51ef8c4659fa --- /dev/null +++ b/.changeset/forty-icons-switch.md @@ -0,0 +1,5 @@ +--- +'@sveltejs/kit': patch +--- + +Roll over non-matching optional parameters instead of 404ing diff --git a/packages/kit/src/core/generate_manifest/index.js b/packages/kit/src/core/generate_manifest/index.js index 429fd27cca1a..a77685b964ca 100644 --- a/packages/kit/src/core/generate_manifest/index.js +++ b/packages/kit/src/core/generate_manifest/index.js @@ -99,8 +99,8 @@ export function generate_manifest({ build_data, relative_path, routes, format = ], routes: [ ${routes.map(route => { - route.types.forEach(type => { - if (type) matchers.add(type); + route.params.forEach(param => { + if (param.matcher) matchers.add(param.matcher); }); if (!route.page && !route.endpoint) return; @@ -108,9 +108,7 @@ export function generate_manifest({ build_data, relative_path, routes, format = return `{ id: ${s(route.id)}, pattern: ${route.pattern}, - names: ${s(route.names)}, - types: ${s(route.types)}, - optional: ${s(route.optional)}, + params: ${s(route.params)}, page: ${route.page ? `{ layouts: ${get_nodes(route.page.layouts)}, errors: ${get_nodes(route.page.errors)}, leaf: ${reindexed.get(route.page.leaf)} }` : 'null'}, endpoint: ${route.endpoint ? loader(join_relative(relative_path, resolve_symlinks(build_data.server.vite_manifest, route.endpoint.file).chunk.file)) : 'null'} }`; diff --git a/packages/kit/src/core/sync/create_manifest_data/index.js b/packages/kit/src/core/sync/create_manifest_data/index.js index bd797a9aea32..4fc738f92ea3 100644 --- a/packages/kit/src/core/sync/create_manifest_data/index.js +++ b/packages/kit/src/core/sync/create_manifest_data/index.js @@ -24,6 +24,14 @@ export default function create_manifest_data({ const matchers = create_matchers(config, cwd); const { nodes, routes } = create_routes_and_nodes(cwd, config, fallback); + for (const route of routes) { + for (const param of route.params) { + if (param.matcher && !matchers[param.matcher]) { + throw new Error(`No matcher found for parameter '${param.matcher}' in route ${route.id}`); + } + } + } + return { assets, matchers, @@ -153,7 +161,7 @@ function create_routes_and_nodes(cwd, config, fallback) { ); } - const { pattern, names, types, optional } = parse_route_id(id); + const { pattern, params } = parse_route_id(id); /** @type {import('types').RouteData} */ const route = { @@ -162,9 +170,7 @@ function create_routes_and_nodes(cwd, config, fallback) { segment, pattern, - names, - types, - optional, + params, layout: null, error: null, @@ -273,9 +279,7 @@ function create_routes_and_nodes(cwd, config, fallback) { id: '/', segment: '', pattern: /^$/, - names: [], - types: [], - optional: [], + params: [], parent: null, layout: null, error: null, diff --git a/packages/kit/src/core/sync/create_manifest_data/index.spec.js b/packages/kit/src/core/sync/create_manifest_data/index.spec.js index e036d3f748e1..c386d8ac0614 100644 --- a/packages/kit/src/core/sync/create_manifest_data/index.spec.js +++ b/packages/kit/src/core/sync/create_manifest_data/index.spec.js @@ -87,7 +87,7 @@ test('creates routes', () => { }, { id: '/blog.json', - pattern: '/^/blog.json$/', + pattern: '/^/blog.json/?$/', endpoint: { file: 'samples/basic/blog.json/+server.js' } }, { @@ -97,7 +97,7 @@ test('creates routes', () => { }, { id: '/blog/[slug].json', - pattern: '/^/blog/([^/]+?).json$/', + pattern: '/^/blog/([^/]+?).json/?$/', endpoint: { file: 'samples/basic/blog/[slug].json/+server.ts' } @@ -308,7 +308,7 @@ test('allows rest parameters inside segments', () => { }, { id: '/[...rest].json', - pattern: '/^/(.*?).json$/', + pattern: '/^/(.*?).json/?$/', endpoint: { file: 'samples/rest-prefix-suffix/[...rest].json/+server.js' } @@ -408,7 +408,7 @@ test('allows multiple slugs', () => { assert.equal(routes.filter((route) => route.endpoint).map(simplify_route), [ { id: '/[file].[ext]', - pattern: '/^/([^/]+?).([^/]+?)$/', + pattern: '/^/([^/]+?).([^/]+?)/?$/', endpoint: { file: 'samples/multiple-slugs/[file].[ext]/+server.js' } @@ -467,7 +467,7 @@ test('works with custom extensions', () => { }, { id: '/blog.json', - pattern: '/^/blog.json$/', + pattern: '/^/blog.json/?$/', endpoint: { file: 'samples/custom-extension/blog.json/+server.js' } @@ -479,7 +479,7 @@ test('works with custom extensions', () => { }, { id: '/blog/[slug].json', - pattern: '/^/blog/([^/]+?).json$/', + pattern: '/^/blog/([^/]+?).json/?$/', endpoint: { file: 'samples/custom-extension/blog/[slug].json/+server.js' } diff --git a/packages/kit/src/core/sync/write_types/index.js b/packages/kit/src/core/sync/write_types/index.js index 17cb181256ae..a10a654b5cc4 100644 --- a/packages/kit/src/core/sync/write_types/index.js +++ b/packages/kit/src/core/sync/write_types/index.js @@ -195,8 +195,8 @@ function update_types(config, routes, route, to_delete = new Set()) { // Makes sure a type is "repackaged" and therefore more readable declarations.push('type Expand = T extends infer O ? { [K in keyof O]: O[K] } : never;'); declarations.push( - `type RouteParams = { ${route.names - .map((param, idx) => `${param}${route.optional[idx] ? '?' : ''}: string`) + `type RouteParams = { ${route.params + .map((param) => `${param.name}${param.optional ? '?' : ''}: string`) .join('; ')} }` ); @@ -270,8 +270,8 @@ function update_types(config, routes, route, to_delete = new Set()) { if (leaf) { if (leaf.route.page) ids.push(`"${leaf.route.id}"`); - for (const name of leaf.route.names) { - layout_params.add(name); + for (const param of leaf.route.params) { + layout_params.add(param.name); } ensureProxies(page, leaf.proxies); diff --git a/packages/kit/src/exports/vite/dev/index.js b/packages/kit/src/exports/vite/dev/index.js index 6684163a5df5..7feaa7cfec3a 100644 --- a/packages/kit/src/exports/vite/dev/index.js +++ b/packages/kit/src/exports/vite/dev/index.js @@ -156,9 +156,7 @@ export async function dev(vite, vite_config, svelte_config) { return { id: route.id, pattern: route.pattern, - names: route.names, - types: route.types, - optional: route.optional, + params: route.params, page: route.page, endpoint: endpoint ? async () => { diff --git a/packages/kit/src/runtime/client/parse.js b/packages/kit/src/runtime/client/parse.js index 63909d0fcf8f..b452e3b87604 100644 --- a/packages/kit/src/runtime/client/parse.js +++ b/packages/kit/src/runtime/client/parse.js @@ -11,14 +11,14 @@ export function parse(nodes, server_loads, dictionary, matchers) { const layouts_with_server_load = new Set(server_loads); return Object.entries(dictionary).map(([id, [leaf, layouts, errors]]) => { - const { pattern, names, types, optional } = parse_route_id(id); + const { pattern, params } = parse_route_id(id); const route = { id, /** @param {string} path */ exec: (path) => { const match = pattern.exec(path); - if (match) return exec(match, { names, types, optional }, matchers); + if (match) return exec(match, params, matchers); }, errors: [1, ...(errors || [])].map((n) => nodes[n]), layouts: [0, ...(layouts || [])].map(create_layout_loader), diff --git a/packages/kit/src/runtime/server/index.js b/packages/kit/src/runtime/server/index.js index 18a9fe003395..f5554c4c64cf 100644 --- a/packages/kit/src/runtime/server/index.js +++ b/packages/kit/src/runtime/server/index.js @@ -77,7 +77,7 @@ export async function respond(request, options, state) { const match = candidate.pattern.exec(decoded); if (!match) continue; - const matched = exec(match, candidate, matchers); + const matched = exec(match, candidate.params, matchers); if (matched) { route = candidate; params = decode_params(matched); diff --git a/packages/kit/src/utils/routing.js b/packages/kit/src/utils/routing.js index 7991499df119..6c71be0ef343 100644 --- a/packages/kit/src/utils/routing.js +++ b/packages/kit/src/utils/routing.js @@ -5,44 +5,40 @@ const param_pattern = /^(\[)?(\.\.\.)?(\w+)(?:=(\w+))?(\])?$/; * @param {string} id */ export function parse_route_id(id) { - /** @type {string[]} */ - const names = []; - - /** @type {string[]} */ - const types = []; - - /** @type {boolean[]} */ - const optional = []; - - // `/foo` should get an optional trailing slash, `/foo.json` should not - // const add_trailing_slash = !/\.[a-z]+$/.test(key); - let add_trailing_slash = true; + /** @type {import('types').RouteParam[]} */ + const params = []; const pattern = id === '/' ? /^\/$/ : new RegExp( `^${get_route_segments(id) - .map((segment, i, segments) => { + .map((segment) => { // special case — /[...rest]/ could contain zero segments const rest_match = /^\[\.\.\.(\w+)(?:=(\w+))?\]$/.exec(segment); if (rest_match) { - names.push(rest_match[1]); - types.push(rest_match[2]); - optional.push(false); + params.push({ + name: rest_match[1], + matcher: rest_match[2], + optional: false, + rest: true, + chained: true + }); return '(?:/(.*))?'; } // special case — /[[optional]]/ could contain zero segments const optional_match = /^\[\[(\w+)(?:=(\w+))?\]\]$/.exec(segment); if (optional_match) { - names.push(optional_match[1]); - types.push(optional_match[2]); - optional.push(true); + params.push({ + name: optional_match[1], + matcher: optional_match[2], + optional: true, + rest: false, + chained: true + }); return '(?:/([^/]+))?'; } - const is_last = i === segments.length - 1; - if (!segment) { return; } @@ -73,29 +69,31 @@ export function parse_route_id(id) { ); } - const [, is_optional, is_rest, name, type] = match; + const [, is_optional, is_rest, name, matcher] = match; // It's assumed that the following invalid route id cases are already checked // - unbalanced brackets // - optional param following rest param - names.push(name); - types.push(type); - optional.push(!!is_optional); + params.push({ + name, + matcher, + optional: !!is_optional, + rest: !!is_rest, + chained: is_rest ? i === 1 && parts[0] === '' : false + }); return is_rest ? '(.*?)' : is_optional ? '([^/]*)?' : '([^/]+?)'; } - if (is_last && content.includes('.')) add_trailing_slash = false; - return escape(content); }) .join(''); return '/' + result; }) - .join('')}${add_trailing_slash ? '/?' : ''}$` + .join('')}/?$` ); - return { pattern, names, types, optional }; + return { pattern, params }; } /** @@ -119,35 +117,70 @@ export function get_route_segments(route) { /** * @param {RegExpMatchArray} match - * @param {{ - * names: string[]; - * types: string[]; - * optional: boolean[]; - * }} candidate + * @param {import('types').RouteParam[]} params * @param {Record} matchers */ -export function exec(match, { names, types, optional }, matchers) { +export function exec(match, params, matchers) { /** @type {Record} */ - const params = {}; + const result = {}; + + const values = match.slice(1); - for (let i = 0; i < names.length; i += 1) { - const name = names[i]; - const type = types[i]; - let value = match[i + 1]; + let buffered = ''; - if (value || !optional[i]) { - if (type) { - const matcher = matchers[type]; - if (!matcher) throw new Error(`Missing "${type}" param matcher`); // TODO do this ahead of time? + for (let i = 0; i < params.length; i += 1) { + const param = params[i]; + let value = values[i]; + + if (param.chained && param.rest && buffered) { + // in the `[[lang=lang]]/[...rest]` case, if `lang` didn't + // match, we roll it over into the rest value + value = value ? buffered + '/' + value : buffered; + } - if (!matcher(value)) return; + buffered = ''; + + if (value === undefined) { + // if `value` is undefined, it means this is + // an optional or rest parameter + if (param.rest) result[param.name] = ''; + } else { + if (param.matcher && !matchers[param.matcher](value)) { + // in the `/[[a=b]]/[[c=d]]` case, if the value didn't satisfy the `b` matcher, + // try again with the next segment by shifting values rightwards + if (param.optional && param.chained) { + // @ts-expect-error TypeScript is... wrong + let j = values.indexOf(undefined, i); + + if (j === -1) { + // we can't shift values any further, so hang on to this value + // so it can be rolled into a subsequent `[...rest]` param + const next = params[i + 1]; + if (next?.rest && next.chained) { + buffered = value; + } else { + return; + } + } + + while (j >= i) { + values[j] = values[j - 1]; + j -= 1; + } + + continue; + } + + // otherwise, if the matcher returns `false`, the route did not match + return; } - params[name] = value ?? ''; + result[param.name] = value; } } - return params; + if (buffered) return; + return result; } /** @param {string} str */ diff --git a/packages/kit/src/utils/routing.spec.js b/packages/kit/src/utils/routing.spec.js index c4b585ac5653..18d144ce3c7c 100644 --- a/packages/kit/src/utils/routing.spec.js +++ b/packages/kit/src/utils/routing.spec.js @@ -5,63 +5,51 @@ import { exec, parse_route_id } from './routing.js'; const tests = { '/': { pattern: /^\/$/, - names: [], - types: [] + params: [] }, '/blog': { pattern: /^\/blog\/?$/, - names: [], - types: [] + params: [] }, '/blog.json': { - pattern: /^\/blog\.json$/, - names: [], - types: [] + pattern: /^\/blog\.json\/?$/, + params: [] }, '/blog/[slug]': { pattern: /^\/blog\/([^/]+?)\/?$/, - names: ['slug'], - types: [undefined] + params: [{ name: 'slug', matcher: undefined, optional: false, rest: false, chained: false }] }, '/blog/[slug].json': { - pattern: /^\/blog\/([^/]+?)\.json$/, - names: ['slug'], - types: [undefined] + pattern: /^\/blog\/([^/]+?)\.json\/?$/, + params: [{ name: 'slug', matcher: undefined, optional: false, rest: false, chained: false }] }, '/blog/[[slug]]': { pattern: /^\/blog(?:\/([^/]+))?\/?$/, - names: ['slug'], - types: [undefined] + params: [{ name: 'slug', matcher: undefined, optional: true, rest: false, chained: true }] }, '/blog/[[slug=type]]/sub': { pattern: /^\/blog(?:\/([^/]+))?\/sub\/?$/, - names: ['slug'], - types: ['type'] + params: [{ name: 'slug', matcher: 'type', optional: true, rest: false, chained: true }] }, '/blog/[[slug]].json': { - pattern: /^\/blog\/([^/]*)?\.json$/, - names: ['slug'], - types: [undefined] + pattern: /^\/blog\/([^/]*)?\.json\/?$/, + params: [{ name: 'slug', matcher: undefined, optional: true, rest: false, chained: false }] }, '/[...catchall]': { pattern: /^(?:\/(.*))?\/?$/, - names: ['catchall'], - types: [undefined] + params: [{ name: 'catchall', matcher: undefined, optional: false, rest: true, chained: true }] }, '/foo/[...catchall]/bar': { pattern: /^\/foo(?:\/(.*))?\/bar\/?$/, - names: ['catchall'], - types: [undefined] + params: [{ name: 'catchall', matcher: undefined, optional: false, rest: true, chained: true }] }, '/matched/[id=uuid]': { pattern: /^\/matched\/([^/]+?)\/?$/, - names: ['id'], - types: ['uuid'] + params: [{ name: 'id', matcher: 'uuid', optional: false, rest: false, chained: false }] }, '/@-symbol/[id]': { pattern: /^\/@-symbol\/([^/]+?)\/?$/, - names: ['id'], - types: [undefined] + params: [{ name: 'id', matcher: undefined, optional: false, rest: false, chained: false }] } }; @@ -70,8 +58,7 @@ for (const [key, expected] of Object.entries(tests)) { const actual = parse_route_id(key); assert.equal(actual.pattern.toString(), expected.pattern.toString()); - assert.equal(actual.names, expected.names); - assert.equal(actual.types, expected.types); + assert.equal(actual.params, expected.params); }); } @@ -160,22 +147,33 @@ const exec_tests = [ route: '/[...slug1=doesntmatch]', path: '/', expected: undefined + }, + { + route: '/[[slug=doesntmatch]]/[...rest]', + path: '/foo', + expected: { rest: 'foo' } + }, + { + route: '/[[slug1=doesntmatch]]/[[slug2=matches]]/[[slug3=doesntmatch]]/[...rest].json', + path: '/foo/bar/baz.json', + expected: { slug2: 'foo', rest: 'bar/baz' } + }, + { + route: '/[[a=doesntmatch]]/[[b=matches]]/c', + path: '/a/b/c', + expected: undefined } ]; for (const { path, route, expected } of exec_tests) { test(`exec extracts params correctly for ${path} from ${route}`, () => { - const { pattern, names, types, optional } = parse_route_id(route); + const { pattern, params } = parse_route_id(route); const match = pattern.exec(path); if (!match) throw new Error(`Failed to match ${path}`); - const actual = exec( - match, - { names, types, optional }, - { - matches: () => true, - doesntmatch: () => false - } - ); + const actual = exec(match, params, { + matches: () => true, + doesntmatch: () => false + }); assert.equal(actual, expected); }); } diff --git a/packages/kit/types/internal.d.ts b/packages/kit/types/internal.d.ts index 33e084bc3315..a2ae2f371283 100644 --- a/packages/kit/types/internal.d.ts +++ b/packages/kit/types/internal.d.ts @@ -158,6 +158,14 @@ export interface Respond { (request: Request, options: SSROptions, state: SSRState): Promise; } +export interface RouteParam { + name: string; + matcher: string; + optional: boolean; + rest: boolean; + chained: boolean; +} + /** * Represents a route segment in the app. It can either be an intermediate node * with only layout/error pages, or a leaf, at which point either `page` and `leaf` @@ -169,9 +177,7 @@ export interface RouteData { segment: string; pattern: RegExp; - names: string[]; - types: string[]; - optional: boolean[]; + params: RouteParam[]; layout: PageNode | null; error: PageNode | null; @@ -337,12 +343,8 @@ export type SSREndpoint = Partial> & { export interface SSRRoute { id: string; pattern: RegExp; - names: string[]; - types: string[]; - optional: boolean[]; - + params: RouteParam[]; page: PageNodeIndexes | null; - endpoint: (() => Promise) | null; } diff --git a/sites/kit.svelte.dev/src/lib/docs/server/index.js b/sites/kit.svelte.dev/src/lib/docs/server/index.js index 878c2be6be21..c7465541da4f 100644 --- a/sites/kit.svelte.dev/src/lib/docs/server/index.js +++ b/sites/kit.svelte.dev/src/lib/docs/server/index.js @@ -124,7 +124,7 @@ export async function read_file(file) { } if (source.includes('./$types') && !source.includes('@filename: $types.d.ts')) { const params = parse_route_id(options.file || `+page.${language}`) - .names.map((name) => `${name}: string`) + .params.map((param) => `${param.name}: string`) .join(', '); injected.push(