From b570d3ce943efaeebf7e8cc219e27a4815e880a6 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 21 Nov 2022 16:42:11 -0500 Subject: [PATCH 01/20] add failing test for #7548 --- .../routes/routing/optional-then-rest/+layout.svelte | 4 ++++ .../routes/routing/optional-then-rest/+page.svelte | 0 .../[[x=numeric]]/[...y]/+page.svelte | 6 ++++++ packages/kit/test/apps/basics/test/test.js | 12 ++++++++++++ 4 files changed, 22 insertions(+) create mode 100644 packages/kit/test/apps/basics/src/routes/routing/optional-then-rest/+layout.svelte create mode 100644 packages/kit/test/apps/basics/src/routes/routing/optional-then-rest/+page.svelte create mode 100644 packages/kit/test/apps/basics/src/routes/routing/optional-then-rest/[[x=numeric]]/[...y]/+page.svelte diff --git a/packages/kit/test/apps/basics/src/routes/routing/optional-then-rest/+layout.svelte b/packages/kit/test/apps/basics/src/routes/routing/optional-then-rest/+layout.svelte new file mode 100644 index 000000000000..023662db9505 --- /dev/null +++ b/packages/kit/test/apps/basics/src/routes/routing/optional-then-rest/+layout.svelte @@ -0,0 +1,4 @@ +/routing/optional-then-rest/a/1 +/routing/optional-then-rest/1/2 + + diff --git a/packages/kit/test/apps/basics/src/routes/routing/optional-then-rest/+page.svelte b/packages/kit/test/apps/basics/src/routes/routing/optional-then-rest/+page.svelte new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/packages/kit/test/apps/basics/src/routes/routing/optional-then-rest/[[x=numeric]]/[...y]/+page.svelte b/packages/kit/test/apps/basics/src/routes/routing/optional-then-rest/[[x=numeric]]/[...y]/+page.svelte new file mode 100644 index 000000000000..3ceb365c4a89 --- /dev/null +++ b/packages/kit/test/apps/basics/src/routes/routing/optional-then-rest/[[x=numeric]]/[...y]/+page.svelte @@ -0,0 +1,6 @@ + + +

{$page.params.x}

+

{$page.params.y}

diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index be47fcb68a93..2c65e444b476 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -1686,6 +1686,18 @@ test.describe('Routing', () => { expect(await page.textContent('p')).toBe('This is your custom error page saying: "Not Found"'); }); + test('allows optional parameter before a rest parameter', async ({ page, clicknav }) => { + await page.goto('/routing/optional-then-rest'); + + await clicknav('[href="/routing/optional-then-rest/a/1"]'); + expect(await page.textContent('[data-testid="x"]')).toBe('undefined'); + expect(await page.textContent('[data-testid="y"]')).toBe('a/1'); + + await clicknav('[href="/routing/optional-then-rest/1/2"]'); + expect(await page.textContent('[data-testid="x"]')).toBe('1'); + expect(await page.textContent('[data-testid="y"]')).toBe('2'); + }); + if (process.platform !== 'win32') { test('Respects symlinks', async ({ page, clicknav }) => { await page.goto('/routing'); From b29fd0f84d48b3d2aaf6e33cd83fa5b3d9ab304c Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 21 Nov 2022 16:42:38 -0500 Subject: [PATCH 02/20] refactor --- .../kit/src/core/generate_manifest/index.js | 8 +-- .../core/sync/create_manifest_data/index.js | 10 +--- .../kit/src/core/sync/write_types/index.js | 8 +-- packages/kit/src/exports/vite/dev/index.js | 4 +- packages/kit/src/runtime/client/parse.js | 4 +- packages/kit/src/runtime/server/index.js | 2 +- packages/kit/src/utils/routing.js | 59 ++++++++----------- packages/kit/src/utils/routing.spec.js | 53 ++++++----------- packages/kit/types/internal.d.ts | 14 ++--- 9 files changed, 62 insertions(+), 100 deletions(-) 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..96f5ff503d84 100644 --- a/packages/kit/src/core/sync/create_manifest_data/index.js +++ b/packages/kit/src/core/sync/create_manifest_data/index.js @@ -153,7 +153,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 +162,7 @@ function create_routes_and_nodes(cwd, config, fallback) { segment, pattern, - names, - types, - optional, + params, layout: null, error: null, @@ -273,9 +271,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/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..eb119a97b94e 100644 --- a/packages/kit/src/utils/routing.js +++ b/packages/kit/src/utils/routing.js @@ -5,14 +5,8 @@ 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 = []; + /** @type {Array<{ name: string, matcher: string, optional: boolean }>} */ + const params = []; // `/foo` should get an optional trailing slash, `/foo.json` should not // const add_trailing_slash = !/\.[a-z]+$/.test(key); @@ -27,17 +21,17 @@ export function parse_route_id(id) { // 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 }); 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 + }); return '(?:/([^/]+))?'; } @@ -73,14 +67,12 @@ 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 }); return is_rest ? '(.*?)' : is_optional ? '([^/]*)?' : '([^/]+?)'; } @@ -95,7 +87,7 @@ export function parse_route_id(id) { .join('')}${add_trailing_slash ? '/?' : ''}$` ); - return { pattern, names, types, optional }; + return { pattern, params }; } /** @@ -119,35 +111,32 @@ export function get_route_segments(route) { /** * @param {RegExpMatchArray} match - * @param {{ - * names: string[]; - * types: string[]; - * optional: boolean[]; - * }} candidate + * @param {Array<{ name: string, matcher: string, optional: boolean }>} params * @param {Record} matchers */ -export function exec(match, { names, types, optional }, matchers) { +export function exec(match, params, matchers) { /** @type {Record} */ - const params = {}; + const result = {}; + + console.log(match, params); - for (let i = 0; i < names.length; i += 1) { - const name = names[i]; - const type = types[i]; + for (let i = 0; i < params.length; i += 1) { + const param = params[i]; let value = match[i + 1]; - 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? + if (value || !param.optional) { + if (param.matcher) { + const matcher = matchers[param.matcher]; + if (!matcher) throw new Error(`Missing "${param.matcher}" param matcher`); // TODO do this ahead of time? if (!matcher(value)) return; } - params[name] = value ?? ''; + result[param.name] = value ?? ''; } } - return params; + 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..8d69b0971a06 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: [] + params: [] }, '/blog/[slug]': { pattern: /^\/blog\/([^/]+?)\/?$/, - names: ['slug'], - types: [undefined] + params: [{ name: 'slug', matcher: undefined }] }, '/blog/[slug].json': { pattern: /^\/blog\/([^/]+?)\.json$/, - names: ['slug'], - types: [undefined] + params: [{ name: 'slug', matcher: undefined }] }, '/blog/[[slug]]': { pattern: /^\/blog(?:\/([^/]+))?\/?$/, - names: ['slug'], - types: [undefined] + params: [{ name: 'slug', matcher: undefined }] }, '/blog/[[slug=type]]/sub': { pattern: /^\/blog(?:\/([^/]+))?\/sub\/?$/, - names: ['slug'], - types: ['type'] + params: [{ name: 'slug', matcher: undefined }] }, '/blog/[[slug]].json': { pattern: /^\/blog\/([^/]*)?\.json$/, - names: ['slug'], - types: [undefined] + params: [{ name: 'slug', matcher: undefined }] }, '/[...catchall]': { pattern: /^(?:\/(.*))?\/?$/, - names: ['catchall'], - types: [undefined] + params: [{ name: 'catchall', matcher: undefined }] }, '/foo/[...catchall]/bar': { pattern: /^\/foo(?:\/(.*))?\/bar\/?$/, - names: ['catchall'], - types: [undefined] + params: [{ name: 'catchall', matcher: undefined }] }, '/matched/[id=uuid]': { pattern: /^\/matched\/([^/]+?)\/?$/, - names: ['id'], - types: ['uuid'] + params: [{ name: 'id', matcher: undefined }] }, '/@-symbol/[id]': { pattern: /^\/@-symbol\/([^/]+?)\/?$/, - names: ['id'], - types: [undefined] + params: [{ name: 'id', matcher: undefined }] } }; @@ -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); }); } @@ -165,17 +152,13 @@ const exec_tests = [ 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..bf8ee4ca802a 100644 --- a/packages/kit/types/internal.d.ts +++ b/packages/kit/types/internal.d.ts @@ -169,9 +169,11 @@ export interface RouteData { segment: string; pattern: RegExp; - names: string[]; - types: string[]; - optional: boolean[]; + params: Array<{ + name: string; + matcher: string; + optional: boolean; + }>; layout: PageNode | null; error: PageNode | null; @@ -337,12 +339,8 @@ export type SSREndpoint = Partial> & { export interface SSRRoute { id: string; pattern: RegExp; - names: string[]; - types: string[]; - optional: boolean[]; - + params: Array<{ name: string; matcher: string; optional: boolean }>; page: PageNodeIndexes | null; - endpoint: (() => Promise) | null; } From 50c4adcd66763be6a0d4d434c3c7f0aaf8082a54 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 21 Nov 2022 16:53:47 -0500 Subject: [PATCH 03/20] fix --- packages/kit/src/utils/routing.js | 2 -- packages/kit/src/utils/routing.spec.js | 18 +++++++++--------- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/packages/kit/src/utils/routing.js b/packages/kit/src/utils/routing.js index eb119a97b94e..564645c42141 100644 --- a/packages/kit/src/utils/routing.js +++ b/packages/kit/src/utils/routing.js @@ -118,8 +118,6 @@ export function exec(match, params, matchers) { /** @type {Record} */ const result = {}; - console.log(match, params); - for (let i = 0; i < params.length; i += 1) { const param = params[i]; let value = match[i + 1]; diff --git a/packages/kit/src/utils/routing.spec.js b/packages/kit/src/utils/routing.spec.js index 8d69b0971a06..5f838f2aace2 100644 --- a/packages/kit/src/utils/routing.spec.js +++ b/packages/kit/src/utils/routing.spec.js @@ -17,39 +17,39 @@ const tests = { }, '/blog/[slug]': { pattern: /^\/blog\/([^/]+?)\/?$/, - params: [{ name: 'slug', matcher: undefined }] + params: [{ name: 'slug', matcher: undefined, optional: false }] }, '/blog/[slug].json': { pattern: /^\/blog\/([^/]+?)\.json$/, - params: [{ name: 'slug', matcher: undefined }] + params: [{ name: 'slug', matcher: undefined, optional: false }] }, '/blog/[[slug]]': { pattern: /^\/blog(?:\/([^/]+))?\/?$/, - params: [{ name: 'slug', matcher: undefined }] + params: [{ name: 'slug', matcher: undefined, optional: true }] }, '/blog/[[slug=type]]/sub': { pattern: /^\/blog(?:\/([^/]+))?\/sub\/?$/, - params: [{ name: 'slug', matcher: undefined }] + params: [{ name: 'slug', matcher: 'type', optional: true }] }, '/blog/[[slug]].json': { pattern: /^\/blog\/([^/]*)?\.json$/, - params: [{ name: 'slug', matcher: undefined }] + params: [{ name: 'slug', matcher: undefined, optional: true }] }, '/[...catchall]': { pattern: /^(?:\/(.*))?\/?$/, - params: [{ name: 'catchall', matcher: undefined }] + params: [{ name: 'catchall', matcher: undefined, optional: false }] }, '/foo/[...catchall]/bar': { pattern: /^\/foo(?:\/(.*))?\/bar\/?$/, - params: [{ name: 'catchall', matcher: undefined }] + params: [{ name: 'catchall', matcher: undefined, optional: false }] }, '/matched/[id=uuid]': { pattern: /^\/matched\/([^/]+?)\/?$/, - params: [{ name: 'id', matcher: undefined }] + params: [{ name: 'id', matcher: 'uuid', optional: false }] }, '/@-symbol/[id]': { pattern: /^\/@-symbol\/([^/]+?)\/?$/, - params: [{ name: 'id', matcher: undefined }] + params: [{ name: 'id', matcher: undefined, optional: false }] } }; From e3458a4d1acb7dee760c26d7b807d5ce43c2efeb Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 21 Nov 2022 17:00:36 -0500 Subject: [PATCH 04/20] always add trailing slash, that logic is way out of date --- packages/kit/src/utils/routing.js | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/packages/kit/src/utils/routing.js b/packages/kit/src/utils/routing.js index 564645c42141..7a1ead754d48 100644 --- a/packages/kit/src/utils/routing.js +++ b/packages/kit/src/utils/routing.js @@ -8,16 +8,12 @@ export function parse_route_id(id) { /** @type {Array<{ name: string, matcher: string, optional: boolean }>} */ const params = []; - // `/foo` should get an optional trailing slash, `/foo.json` should not - // const add_trailing_slash = !/\.[a-z]+$/.test(key); - let add_trailing_slash = true; - 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) { @@ -35,8 +31,6 @@ export function parse_route_id(id) { return '(?:/([^/]+))?'; } - const is_last = i === segments.length - 1; - if (!segment) { return; } @@ -76,15 +70,13 @@ export function parse_route_id(id) { 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, params }; From 78ca9bdf91f9148ccbcf956c8438439732c0ff03 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 21 Nov 2022 17:32:52 -0500 Subject: [PATCH 05/20] fix tests --- .../src/core/sync/create_manifest_data/index.spec.js | 12 ++++++------ packages/kit/src/utils/routing.spec.js | 6 +++--- 2 files changed, 9 insertions(+), 9 deletions(-) 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/utils/routing.spec.js b/packages/kit/src/utils/routing.spec.js index 5f838f2aace2..163c3ad3da6d 100644 --- a/packages/kit/src/utils/routing.spec.js +++ b/packages/kit/src/utils/routing.spec.js @@ -12,7 +12,7 @@ const tests = { params: [] }, '/blog.json': { - pattern: /^\/blog\.json$/, + pattern: /^\/blog\.json\/?$/, params: [] }, '/blog/[slug]': { @@ -20,7 +20,7 @@ const tests = { params: [{ name: 'slug', matcher: undefined, optional: false }] }, '/blog/[slug].json': { - pattern: /^\/blog\/([^/]+?)\.json$/, + pattern: /^\/blog\/([^/]+?)\.json\/?$/, params: [{ name: 'slug', matcher: undefined, optional: false }] }, '/blog/[[slug]]': { @@ -32,7 +32,7 @@ const tests = { params: [{ name: 'slug', matcher: 'type', optional: true }] }, '/blog/[[slug]].json': { - pattern: /^\/blog\/([^/]*)?\.json$/, + pattern: /^\/blog\/([^/]*)?\.json\/?$/, params: [{ name: 'slug', matcher: undefined, optional: true }] }, '/[...catchall]': { From ed3045d10231c5272fb41b8f9628e7537c62a4ed Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 21 Nov 2022 17:37:41 -0500 Subject: [PATCH 06/20] DRY out --- packages/kit/src/utils/routing.js | 12 +++++++----- packages/kit/types/internal.d.ts | 14 ++++++++------ 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/packages/kit/src/utils/routing.js b/packages/kit/src/utils/routing.js index 7a1ead754d48..ae8ede07911b 100644 --- a/packages/kit/src/utils/routing.js +++ b/packages/kit/src/utils/routing.js @@ -5,7 +5,7 @@ const param_pattern = /^(\[)?(\.\.\.)?(\w+)(?:=(\w+))?(\])?$/; * @param {string} id */ export function parse_route_id(id) { - /** @type {Array<{ name: string, matcher: string, optional: boolean }>} */ + /** @type {import('types').RouteParam[]} */ const params = []; const pattern = @@ -103,16 +103,18 @@ export function get_route_segments(route) { /** * @param {RegExpMatchArray} match - * @param {Array<{ name: string, matcher: string, optional: boolean }>} params + * @param {import('types').RouteParam[]} params * @param {Record} matchers */ export function exec(match, params, matchers) { /** @type {Record} */ const result = {}; - for (let i = 0; i < params.length; i += 1) { - const param = params[i]; - let value = match[i + 1]; + let p = 0; + + for (let i = 1; i < match.length; i += 1) { + const param = params[p++]; + const value = match[i]; if (value || !param.optional) { if (param.matcher) { diff --git a/packages/kit/types/internal.d.ts b/packages/kit/types/internal.d.ts index bf8ee4ca802a..d543c250312d 100644 --- a/packages/kit/types/internal.d.ts +++ b/packages/kit/types/internal.d.ts @@ -158,6 +158,12 @@ export interface Respond { (request: Request, options: SSROptions, state: SSRState): Promise; } +export interface RouteParam { + name: string; + matcher: string; + optional: 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,11 +175,7 @@ export interface RouteData { segment: string; pattern: RegExp; - params: Array<{ - name: string; - matcher: string; - optional: boolean; - }>; + params: RouteParam[]; layout: PageNode | null; error: PageNode | null; @@ -339,7 +341,7 @@ export type SSREndpoint = Partial> & { export interface SSRRoute { id: string; pattern: RegExp; - params: Array<{ name: string; matcher: string; optional: boolean }>; + params: RouteParam[]; page: PageNodeIndexes | null; endpoint: (() => Promise) | null; } From 31b05a4f796b3ca3f68486f0876743e72fe02ab3 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 21 Nov 2022 17:42:20 -0500 Subject: [PATCH 07/20] track more param info --- packages/kit/src/utils/routing.js | 20 +++++++++++++++++--- packages/kit/types/internal.d.ts | 2 ++ 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/packages/kit/src/utils/routing.js b/packages/kit/src/utils/routing.js index ae8ede07911b..3de8eaf1c56b 100644 --- a/packages/kit/src/utils/routing.js +++ b/packages/kit/src/utils/routing.js @@ -17,7 +17,13 @@ export function parse_route_id(id) { // special case — /[...rest]/ could contain zero segments const rest_match = /^\[\.\.\.(\w+)(?:=(\w+))?\]$/.exec(segment); if (rest_match) { - params.push({ name: rest_match[1], matcher: rest_match[2], optional: false }); + params.push({ + name: rest_match[1], + matcher: rest_match[2], + optional: false, + rest: true, + standalone: true + }); return '(?:/(.*))?'; } // special case — /[[optional]]/ could contain zero segments @@ -26,7 +32,9 @@ export function parse_route_id(id) { params.push({ name: optional_match[1], matcher: optional_match[2], - optional: true + optional: true, + rest: false, + standalone: true }); return '(?:/([^/]+))?'; } @@ -66,7 +74,13 @@ export function parse_route_id(id) { // - unbalanced brackets // - optional param following rest param - params.push({ name, matcher, optional: !!is_optional }); + params.push({ + name, + matcher, + optional: !!is_optional, + rest: !!is_rest, + standalone: false + }); return is_rest ? '(.*?)' : is_optional ? '([^/]*)?' : '([^/]+?)'; } diff --git a/packages/kit/types/internal.d.ts b/packages/kit/types/internal.d.ts index d543c250312d..769a6e56a2ab 100644 --- a/packages/kit/types/internal.d.ts +++ b/packages/kit/types/internal.d.ts @@ -162,6 +162,8 @@ export interface RouteParam { name: string; matcher: string; optional: boolean; + rest: boolean; + standalone: boolean; } /** From 886303e76fa0814c45d88be8563e7cf78c46ffb5 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 21 Nov 2022 19:26:28 -0500 Subject: [PATCH 08/20] check for missing matcher at manifest creation time --- packages/kit/src/core/sync/create_manifest_data/index.js | 8 ++++++++ packages/kit/src/utils/routing.js | 2 -- 2 files changed, 8 insertions(+), 2 deletions(-) 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 96f5ff503d84..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, diff --git a/packages/kit/src/utils/routing.js b/packages/kit/src/utils/routing.js index 3de8eaf1c56b..d4a65740b972 100644 --- a/packages/kit/src/utils/routing.js +++ b/packages/kit/src/utils/routing.js @@ -133,8 +133,6 @@ export function exec(match, params, matchers) { if (value || !param.optional) { if (param.matcher) { const matcher = matchers[param.matcher]; - if (!matcher) throw new Error(`Missing "${param.matcher}" param matcher`); // TODO do this ahead of time? - if (!matcher(value)) return; } From d149a3855ecd8cdd4760e2e1703671f24d1c5d2a Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 21 Nov 2022 22:39:52 -0500 Subject: [PATCH 09/20] working, i think --- packages/kit/src/utils/routing.js | 40 +++++++++++++++++++++---------- packages/kit/types/internal.d.ts | 2 +- 2 files changed, 28 insertions(+), 14 deletions(-) diff --git a/packages/kit/src/utils/routing.js b/packages/kit/src/utils/routing.js index d4a65740b972..0ff76eea5afd 100644 --- a/packages/kit/src/utils/routing.js +++ b/packages/kit/src/utils/routing.js @@ -22,7 +22,7 @@ export function parse_route_id(id) { matcher: rest_match[2], optional: false, rest: true, - standalone: true + chained: true }); return '(?:/(.*))?'; } @@ -34,7 +34,7 @@ export function parse_route_id(id) { matcher: optional_match[2], optional: true, rest: false, - standalone: true + chained: true }); return '(?:/([^/]+))?'; } @@ -79,7 +79,7 @@ export function parse_route_id(id) { matcher, optional: !!is_optional, rest: !!is_rest, - standalone: false + chained: is_rest ? i === 1 && parts[0] === '' : false }); return is_rest ? '(.*?)' : is_optional ? '([^/]*)?' : '([^/]+?)'; } @@ -124,20 +124,34 @@ export function exec(match, params, matchers) { /** @type {Record} */ const result = {}; - let p = 0; + let m = 1; + let prev = ''; - for (let i = 1; i < match.length; i += 1) { - const param = params[p++]; - const value = match[i]; + for (const param of params) { + let value = (param.chained && prev) || match[m++]; - if (value || !param.optional) { - if (param.matcher) { - const matcher = matchers[param.matcher]; - if (!matcher(value)) return; - } + if (param.rest && param.chained && prev && match[m]) { + value += `/${match[m++]}`; + } + + if (!value) { + if (param.rest) result[param.name] = ''; + continue; + } - result[param.name] = value ?? ''; + if (param.matcher) { + const matcher = matchers[param.matcher]; + if (!matcher(value)) { + if (param.optional && param.chained) { + prev = value; + continue; + } + return; + } } + + result[param.name] = value; + prev = ''; } return result; diff --git a/packages/kit/types/internal.d.ts b/packages/kit/types/internal.d.ts index 769a6e56a2ab..a2ae2f371283 100644 --- a/packages/kit/types/internal.d.ts +++ b/packages/kit/types/internal.d.ts @@ -163,7 +163,7 @@ export interface RouteParam { matcher: string; optional: boolean; rest: boolean; - standalone: boolean; + chained: boolean; } /** From 6cd925d78fc33b08809fafb03c64aaf9b81dd3f0 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 21 Nov 2022 22:42:55 -0500 Subject: [PATCH 10/20] shuffle around --- packages/kit/src/utils/routing.js | 34 +++++++++++++++---------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/packages/kit/src/utils/routing.js b/packages/kit/src/utils/routing.js index 0ff76eea5afd..c92be7fa9695 100644 --- a/packages/kit/src/utils/routing.js +++ b/packages/kit/src/utils/routing.js @@ -125,33 +125,33 @@ export function exec(match, params, matchers) { const result = {}; let m = 1; - let prev = ''; + let buffered = ''; for (const param of params) { - let value = (param.chained && prev) || match[m++]; + let value = (param.chained && buffered) || match[m++]; - if (param.rest && param.chained && prev && match[m]) { + if (param.rest && param.chained && buffered && match[m]) { value += `/${match[m++]}`; } - if (!value) { - if (param.rest) result[param.name] = ''; - continue; - } - - if (param.matcher) { - const matcher = matchers[param.matcher]; - if (!matcher(value)) { - if (param.optional && param.chained) { - prev = value; - continue; + if (value) { + if (param.matcher) { + const matcher = matchers[param.matcher]; + if (!matcher(value)) { + if (param.optional && param.chained) { + buffered = value; + continue; + } + return; } - return; } + + result[param.name] = value; + } else { + if (param.rest) result[param.name] = ''; } - result[param.name] = value; - prev = ''; + buffered = ''; } return result; From e5405a7bed53d9fba17a77d298d95cc8cd5ce25c Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 21 Nov 2022 23:53:54 -0500 Subject: [PATCH 11/20] ok try this --- packages/kit/src/utils/routing.js | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/packages/kit/src/utils/routing.js b/packages/kit/src/utils/routing.js index c92be7fa9695..706324bd9ac0 100644 --- a/packages/kit/src/utils/routing.js +++ b/packages/kit/src/utils/routing.js @@ -124,14 +124,19 @@ export function exec(match, params, matchers) { /** @type {Record} */ const result = {}; - let m = 1; + const values = match.slice(1); + + let p = 0; + let v = 0; + let buffered = ''; - for (const param of params) { - let value = (param.chained && buffered) || match[m++]; + while (p < params.length) { + const param = params[p++]; + let value = values[v++]; - if (param.rest && param.chained && buffered && match[m]) { - value += `/${match[m++]}`; + if (param.chained && param.rest && buffered) { + value = value ? buffered + '/' + value : buffered; } if (value) { @@ -139,7 +144,17 @@ export function exec(match, params, matchers) { const matcher = matchers[param.matcher]; if (!matcher(value)) { if (param.optional && param.chained) { - buffered = value; + let i = values.indexOf(undefined, v); + + if (i === -1) { + buffered = value; + } + + while (i >= v) { + values[i] = values[i - 1]; + i -= 1; + } + continue; } return; From 64911dc1d0290c8ac1c0cb14eea541c8a975600c Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Mon, 21 Nov 2022 23:58:54 -0500 Subject: [PATCH 12/20] update tests --- packages/kit/src/utils/routing.spec.js | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/kit/src/utils/routing.spec.js b/packages/kit/src/utils/routing.spec.js index 163c3ad3da6d..27b6310a0330 100644 --- a/packages/kit/src/utils/routing.spec.js +++ b/packages/kit/src/utils/routing.spec.js @@ -17,39 +17,39 @@ const tests = { }, '/blog/[slug]': { pattern: /^\/blog\/([^/]+?)\/?$/, - params: [{ name: 'slug', matcher: undefined, optional: false }] + params: [{ name: 'slug', matcher: undefined, optional: false, rest: false, chained: false }] }, '/blog/[slug].json': { pattern: /^\/blog\/([^/]+?)\.json\/?$/, - params: [{ name: 'slug', matcher: undefined, optional: false }] + params: [{ name: 'slug', matcher: undefined, optional: false, rest: false, chained: false }] }, '/blog/[[slug]]': { pattern: /^\/blog(?:\/([^/]+))?\/?$/, - params: [{ name: 'slug', matcher: undefined, optional: true }] + params: [{ name: 'slug', matcher: undefined, optional: true, rest: false, chained: true }] }, '/blog/[[slug=type]]/sub': { pattern: /^\/blog(?:\/([^/]+))?\/sub\/?$/, - params: [{ name: 'slug', matcher: 'type', optional: true }] + params: [{ name: 'slug', matcher: 'type', optional: true, rest: false, chained: true }] }, '/blog/[[slug]].json': { pattern: /^\/blog\/([^/]*)?\.json\/?$/, - params: [{ name: 'slug', matcher: undefined, optional: true }] + params: [{ name: 'slug', matcher: undefined, optional: true, rest: false, chained: false }] }, '/[...catchall]': { pattern: /^(?:\/(.*))?\/?$/, - params: [{ name: 'catchall', matcher: undefined, optional: false }] + params: [{ name: 'catchall', matcher: undefined, optional: false, rest: true, chained: true }] }, '/foo/[...catchall]/bar': { pattern: /^\/foo(?:\/(.*))?\/bar\/?$/, - params: [{ name: 'catchall', matcher: undefined, optional: false }] + params: [{ name: 'catchall', matcher: undefined, optional: false, rest: true, chained: true }] }, '/matched/[id=uuid]': { pattern: /^\/matched\/([^/]+?)\/?$/, - params: [{ name: 'id', matcher: 'uuid', optional: false }] + params: [{ name: 'id', matcher: 'uuid', optional: false, rest: false, chained: false }] }, '/@-symbol/[id]': { pattern: /^\/@-symbol\/([^/]+?)\/?$/, - params: [{ name: 'id', matcher: undefined, optional: false }] + params: [{ name: 'id', matcher: undefined, optional: false, rest: false, chained: false }] } }; From 33ccb66bb29d2effc8f3fcd402fd645172d1bcab Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 22 Nov 2022 00:03:27 -0500 Subject: [PATCH 13/20] fix tests --- packages/kit/src/utils/routing.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/kit/src/utils/routing.js b/packages/kit/src/utils/routing.js index 706324bd9ac0..7761105714b4 100644 --- a/packages/kit/src/utils/routing.js +++ b/packages/kit/src/utils/routing.js @@ -139,7 +139,9 @@ export function exec(match, params, matchers) { value = value ? buffered + '/' + value : buffered; } - if (value) { + if (value === undefined) { + if (param.rest) result[param.name] = ''; + } else { if (param.matcher) { const matcher = matchers[param.matcher]; if (!matcher(value)) { @@ -162,13 +164,12 @@ export function exec(match, params, matchers) { } result[param.name] = value; - } else { - if (param.rest) result[param.name] = ''; } buffered = ''; } + if (buffered) return; return result; } From 4d3e6c7acd014aba3023f0d52a57ec604e5e5c08 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 22 Nov 2022 00:17:06 -0500 Subject: [PATCH 14/20] lint --- packages/kit/src/utils/routing.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/kit/src/utils/routing.js b/packages/kit/src/utils/routing.js index 7761105714b4..1ccb8117b076 100644 --- a/packages/kit/src/utils/routing.js +++ b/packages/kit/src/utils/routing.js @@ -146,6 +146,7 @@ export function exec(match, params, matchers) { const matcher = matchers[param.matcher]; if (!matcher(value)) { if (param.optional && param.chained) { + // @ts-expect-error TypeScript is... wrong let i = values.indexOf(undefined, v); if (i === -1) { From caa1ca3717c75210a1fed39c252ace50dc48948f Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 22 Nov 2022 00:20:23 -0500 Subject: [PATCH 15/20] fix --- sites/kit.svelte.dev/src/lib/docs/server/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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( From 6e3c7df678d02a2f0fb62d0e74b62f19ee3b3c8e Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 22 Nov 2022 00:21:33 -0500 Subject: [PATCH 16/20] changeset --- .changeset/forty-icons-switch.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/forty-icons-switch.md diff --git a/.changeset/forty-icons-switch.md b/.changeset/forty-icons-switch.md new file mode 100644 index 000000000000..42fc680358d9 --- /dev/null +++ b/.changeset/forty-icons-switch.md @@ -0,0 +1,5 @@ +--- +'@sveltejs/kit': patch +--- + +Allow chained optional parameters From 2cbf6043f81f2808a66901c17174c3a4ac72e4ac Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 22 Nov 2022 10:07:34 -0500 Subject: [PATCH 17/20] unit tests --- packages/kit/src/utils/routing.spec.js | 10 ++++++++++ .../routes/routing/optional-then-rest/+layout.svelte | 4 ---- .../routes/routing/optional-then-rest/+page.svelte | 0 .../[[x=numeric]]/[...y]/+page.svelte | 6 ------ packages/kit/test/apps/basics/test/test.js | 12 ------------ 5 files changed, 10 insertions(+), 22 deletions(-) delete mode 100644 packages/kit/test/apps/basics/src/routes/routing/optional-then-rest/+layout.svelte delete mode 100644 packages/kit/test/apps/basics/src/routes/routing/optional-then-rest/+page.svelte delete mode 100644 packages/kit/test/apps/basics/src/routes/routing/optional-then-rest/[[x=numeric]]/[...y]/+page.svelte diff --git a/packages/kit/src/utils/routing.spec.js b/packages/kit/src/utils/routing.spec.js index 27b6310a0330..d069e690da13 100644 --- a/packages/kit/src/utils/routing.spec.js +++ b/packages/kit/src/utils/routing.spec.js @@ -147,6 +147,16 @@ 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' } } ]; diff --git a/packages/kit/test/apps/basics/src/routes/routing/optional-then-rest/+layout.svelte b/packages/kit/test/apps/basics/src/routes/routing/optional-then-rest/+layout.svelte deleted file mode 100644 index 023662db9505..000000000000 --- a/packages/kit/test/apps/basics/src/routes/routing/optional-then-rest/+layout.svelte +++ /dev/null @@ -1,4 +0,0 @@ -/routing/optional-then-rest/a/1 -/routing/optional-then-rest/1/2 - - diff --git a/packages/kit/test/apps/basics/src/routes/routing/optional-then-rest/+page.svelte b/packages/kit/test/apps/basics/src/routes/routing/optional-then-rest/+page.svelte deleted file mode 100644 index e69de29bb2d1..000000000000 diff --git a/packages/kit/test/apps/basics/src/routes/routing/optional-then-rest/[[x=numeric]]/[...y]/+page.svelte b/packages/kit/test/apps/basics/src/routes/routing/optional-then-rest/[[x=numeric]]/[...y]/+page.svelte deleted file mode 100644 index 3ceb365c4a89..000000000000 --- a/packages/kit/test/apps/basics/src/routes/routing/optional-then-rest/[[x=numeric]]/[...y]/+page.svelte +++ /dev/null @@ -1,6 +0,0 @@ - - -

{$page.params.x}

-

{$page.params.y}

diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index 2c65e444b476..be47fcb68a93 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -1686,18 +1686,6 @@ test.describe('Routing', () => { expect(await page.textContent('p')).toBe('This is your custom error page saying: "Not Found"'); }); - test('allows optional parameter before a rest parameter', async ({ page, clicknav }) => { - await page.goto('/routing/optional-then-rest'); - - await clicknav('[href="/routing/optional-then-rest/a/1"]'); - expect(await page.textContent('[data-testid="x"]')).toBe('undefined'); - expect(await page.textContent('[data-testid="y"]')).toBe('a/1'); - - await clicknav('[href="/routing/optional-then-rest/1/2"]'); - expect(await page.textContent('[data-testid="x"]')).toBe('1'); - expect(await page.textContent('[data-testid="y"]')).toBe('2'); - }); - if (process.platform !== 'win32') { test('Respects symlinks', async ({ page, clicknav }) => { await page.goto('/routing'); From d3c3959d418a9724c2a01cf974489564466e743b Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 22 Nov 2022 10:17:48 -0500 Subject: [PATCH 18/20] make the code a bit more comprehensible --- packages/kit/src/utils/routing.js | 56 +++++++++++++++++-------------- 1 file changed, 30 insertions(+), 26 deletions(-) diff --git a/packages/kit/src/utils/routing.js b/packages/kit/src/utils/routing.js index 1ccb8117b076..30610e068b27 100644 --- a/packages/kit/src/utils/routing.js +++ b/packages/kit/src/utils/routing.js @@ -126,48 +126,52 @@ export function exec(match, params, matchers) { const values = match.slice(1); - let p = 0; - let v = 0; - let buffered = ''; - while (p < params.length) { - const param = params[p++]; - let value = values[v++]; + 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; } + 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) { - const matcher = matchers[param.matcher]; - if (!matcher(value)) { - if (param.optional && param.chained) { - // @ts-expect-error TypeScript is... wrong - let i = values.indexOf(undefined, v); - - if (i === -1) { - buffered = value; - } - - while (i >= v) { - values[i] = values[i - 1]; - i -= 1; - } - - continue; + 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 + buffered = value; + } + + while (j >= i) { + values[j] = values[j - 1]; + j -= 1; } - return; + + continue; } + + // otherwise, if the matcher returns `false`, the route did not match + return; } result[param.name] = value; } - - buffered = ''; } if (buffered) return; From eceb4d18b95f1017bf800994b7529b91d049536e Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 22 Nov 2022 10:18:51 -0500 Subject: [PATCH 19/20] Update .changeset/forty-icons-switch.md --- .changeset/forty-icons-switch.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/forty-icons-switch.md b/.changeset/forty-icons-switch.md index 42fc680358d9..51ef8c4659fa 100644 --- a/.changeset/forty-icons-switch.md +++ b/.changeset/forty-icons-switch.md @@ -2,4 +2,4 @@ '@sveltejs/kit': patch --- -Allow chained optional parameters +Roll over non-matching optional parameters instead of 404ing From 7c91626705c8a4d644b0fd2bdcd787451874e6d6 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Tue, 22 Nov 2022 14:07:12 -0500 Subject: [PATCH 20/20] fix another case --- packages/kit/src/utils/routing.js | 7 ++++++- packages/kit/src/utils/routing.spec.js | 5 +++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/packages/kit/src/utils/routing.js b/packages/kit/src/utils/routing.js index 30610e068b27..6c71be0ef343 100644 --- a/packages/kit/src/utils/routing.js +++ b/packages/kit/src/utils/routing.js @@ -155,7 +155,12 @@ export function exec(match, params, matchers) { 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 - buffered = value; + const next = params[i + 1]; + if (next?.rest && next.chained) { + buffered = value; + } else { + return; + } } while (j >= i) { diff --git a/packages/kit/src/utils/routing.spec.js b/packages/kit/src/utils/routing.spec.js index d069e690da13..18d144ce3c7c 100644 --- a/packages/kit/src/utils/routing.spec.js +++ b/packages/kit/src/utils/routing.spec.js @@ -157,6 +157,11 @@ const exec_tests = [ 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 } ];