From abbb294dfdedae4b05fe85eeb2e2897313d89cda Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 20 Oct 2022 21:02:36 -0400 Subject: [PATCH 1/2] prefix all route IDs with / - closes #7332 --- .changeset/strange-crabs-shop.md | 5 + packages/kit/src/core/prerender/prerender.js | 2 +- .../core/sync/create_manifest_data/index.js | 4 +- .../sync/create_manifest_data/index.spec.js | 110 +++++++++--------- .../kit/src/runtime/server/page/render.js | 2 +- packages/kit/src/runtime/server/utils.js | 2 +- packages/kit/src/utils/routing.js | 3 +- packages/kit/src/utils/routing.spec.js | 48 ++++---- .../kit/test/apps/basics/test/server.test.js | 2 +- packages/kit/test/apps/basics/test/test.js | 4 +- 10 files changed, 94 insertions(+), 88 deletions(-) create mode 100644 .changeset/strange-crabs-shop.md diff --git a/.changeset/strange-crabs-shop.md b/.changeset/strange-crabs-shop.md new file mode 100644 index 000000000000..224b8db97f7f --- /dev/null +++ b/.changeset/strange-crabs-shop.md @@ -0,0 +1,5 @@ +--- +'@sveltejs/kit': patch +--- + +[breaking] Prefix all route IDs with / diff --git a/packages/kit/src/core/prerender/prerender.js b/packages/kit/src/core/prerender/prerender.js index ea711cc504cf..2fddd504bd25 100644 --- a/packages/kit/src/core/prerender/prerender.js +++ b/packages/kit/src/core/prerender/prerender.js @@ -370,7 +370,7 @@ export async function prerender() { for (const [id, prerender] of prerender_map) { if (prerender) { if (id.includes('[')) continue; - const path = `/${id.split('/').filter(affects_path).join('/')}`; + const path = `${id.split('/').filter(affects_path).join('/')}`; enqueue(null, config.paths.base + path); } } 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 ad30f046f8a0..b38ce5a4599e 100644 --- a/packages/kit/src/core/sync/create_manifest_data/index.js +++ b/packages/kit/src/core/sync/create_manifest_data/index.js @@ -207,7 +207,7 @@ function create_routes_and_nodes(cwd, config, fallback) { } }; - walk(0, '', '', null); + walk(0, '/', '', null); if (routes.length === 1) { const root = routes[0]; @@ -223,7 +223,7 @@ function create_routes_and_nodes(cwd, config, fallback) { // If there's no routes directory, we'll just create a single empty route. This ensures the root layout and // error components are included in the manifest, which is needed for subsequent build/dev commands to work routes.push({ - id: '', + id: '/', segment: '', pattern: /^$/, names: [], 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 3c5ca665c3cd..6488d13731e2 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 @@ -76,34 +76,34 @@ test('creates routes', () => { assert.equal(routes.map(simplify_route), [ { - id: '', + id: '/', pattern: '/^/$/', page: { layouts: [0], errors: [1], leaf: 2 } }, { - id: 'about', + id: '/about', pattern: '/^/about/?$/', page: { layouts: [0], errors: [1], leaf: 3 } }, { - id: 'blog.json', + id: '/blog.json', pattern: '/^/blog.json$/', endpoint: { file: 'samples/basic/blog.json/+server.js' } }, { - id: 'blog', + id: '/blog', pattern: '/^/blog/?$/', page: { layouts: [0], errors: [1], leaf: 4 } }, { - id: 'blog/[slug].json', + id: '/blog/[slug].json', pattern: '/^/blog/([^/]+?).json$/', endpoint: { file: 'samples/basic/blog/[slug].json/+server.ts' } }, { - id: 'blog/[slug]', + id: '/blog/[slug]', pattern: '/^/blog/([^/]+?)/?$/', page: { layouts: [0], errors: [1], leaf: 5 } } @@ -128,13 +128,13 @@ test_symlinks('creates symlinked routes', () => { assert.equal(routes, [ { - id: '', + id: '/', pattern: '/^/$/', page: { layouts: [0], errors: [1], leaf: 1 } }, { - id: 'foo', + id: '/foo', pattern: '/^/foo/?$/', page: { layouts: [0], errors: [1], leaf: 2 } } @@ -154,13 +154,13 @@ test('creates routes with layout', () => { assert.equal(routes.map(simplify_route), [ { - id: '', + id: '/', pattern: '/^/$/', page: { layouts: [0], errors: [1], leaf: 3 } }, { - id: 'foo', + id: '/foo', pattern: '/^/foo/?$/', page: { layouts: [0, 2], errors: [1, undefined], leaf: 4 } } @@ -175,7 +175,7 @@ test('succeeds when routes does not exist', () => { ]); assert.equal(routes.map(simplify_route), [ { - id: '', + id: '/', pattern: '/^$/' } ]); @@ -269,24 +269,24 @@ test('sorts routes with rest correctly', () => { assert.equal(routes.map(simplify_route), [ { - id: '', + id: '/', pattern: '/^/$/' }, { - id: 'a', + id: '/a', pattern: '/^/a/?$/' }, { - id: 'a/[...rest]', + id: '/a/[...rest]', pattern: '/^/a(?:/(.*))?/?$/', page: { layouts: [0], errors: [1], leaf: 2 } }, { - id: 'b', + id: '/b', pattern: '/^/b/?$/' }, { - id: 'b/[...rest]', + id: '/b/[...rest]', pattern: '/^/b(?:/(.*))?/?$/', page: { layouts: [0], errors: [1], leaf: 3 } } @@ -306,16 +306,16 @@ test('allows rest parameters inside segments', () => { assert.equal(routes.map(simplify_route), [ { - id: '', + id: '/', pattern: '/^/$/' }, { - id: 'prefix-[...rest]', + id: '/prefix-[...rest]', pattern: '/^/prefix-(.*?)/?$/', page: { layouts: [0], errors: [1], leaf: 2 } }, { - id: '[...rest].json', + id: '/[...rest].json', pattern: '/^/(.*?).json$/', endpoint: { file: 'samples/rest-prefix-suffix/[...rest].json/+server.js' @@ -351,17 +351,17 @@ test('optional parameters', () => { assert.equal(routes.map(simplify_route), [ { - id: '', + id: '/', pattern: '/^/$/' }, { - id: '[[foo]]bar', + id: '/[[foo]]bar', pattern: '/^/([^/]*)?bar/?$/', endpoint: { file: 'samples/optional/[[foo]]bar/+server.js' } }, - { id: 'nested', pattern: '/^/nested/?$/' }, + { id: '/nested', pattern: '/^/nested/?$/' }, { - id: 'nested/[[optional]]/sub', + id: '/nested/[[optional]]/sub', pattern: '/^/nested(?:/([^/]+))?/sub/?$/', page: { layouts: [0], @@ -370,9 +370,9 @@ test('optional parameters', () => { leaf: nodes.findIndex((node) => node.component?.includes('nested/[[optional]]')) } }, - { id: 'nested/[[optional]]', pattern: '/^/nested(?:/([^/]+))?/?$/' }, + { id: '/nested/[[optional]]', pattern: '/^/nested(?:/([^/]+))?/?$/' }, { - id: 'prefix[[suffix]]', + id: '/prefix[[suffix]]', pattern: '/^/prefix([^/]*)?/?$/', page: { layouts: [0], @@ -382,7 +382,7 @@ test('optional parameters', () => { } }, { - id: '[[optional]]', + id: '/[[optional]]', pattern: '/^(?:/([^/]+))?/?$/', page: { layouts: [0], @@ -415,7 +415,7 @@ test('allows multiple slugs', () => { assert.equal(routes.filter((route) => route.endpoint).map(simplify_route), [ { - id: '[file].[ext]', + id: '/[file].[ext]', pattern: '/^/([^/]+?).([^/]+?)$/', endpoint: { file: 'samples/multiple-slugs/[file].[ext]/+server.js' @@ -427,7 +427,7 @@ test('allows multiple slugs', () => { test('fails if dynamic params are not separated', () => { assert.throws(() => { create('samples/invalid-params'); - }, /Invalid route \[foo\]\[bar\] — parameters must be separated/); + }, /Invalid route \/\[foo\]\[bar\] — parameters must be separated/); }); test('ignores things that look like lockfiles', () => { @@ -435,11 +435,11 @@ test('ignores things that look like lockfiles', () => { assert.equal(routes.map(simplify_route), [ { - id: '', + id: '/', pattern: '/^/$/' }, { - id: 'foo', + id: '/foo', pattern: '/^/foo/?$/', endpoint: { file: 'samples/lockfiles/foo/+server.js' @@ -464,36 +464,36 @@ test('works with custom extensions', () => { assert.equal(routes.map(simplify_route), [ { - id: '', + id: '/', pattern: '/^/$/', page: { layouts: [0], errors: [1], leaf: 2 } }, { - id: 'about', + id: '/about', pattern: '/^/about/?$/', page: { layouts: [0], errors: [1], leaf: 3 } }, { - id: 'blog.json', + id: '/blog.json', pattern: '/^/blog.json$/', endpoint: { file: 'samples/custom-extension/blog.json/+server.js' } }, { - id: 'blog', + id: '/blog', pattern: '/^/blog/?$/', page: { layouts: [0], errors: [1], leaf: 4 } }, { - id: 'blog/[slug].json', + id: '/blog/[slug].json', pattern: '/^/blog/([^/]+?).json$/', endpoint: { file: 'samples/custom-extension/blog/[slug].json/+server.js' } }, { - id: 'blog/[slug]', + id: '/blog/[slug]', pattern: '/^/blog/([^/]+?)/?$/', page: { layouts: [0], errors: [1], leaf: 5 } } @@ -532,19 +532,19 @@ test('includes nested error components', () => { assert.equal(routes.map(simplify_route), [ { - id: '', + id: '/', pattern: '/^/$/' }, { - id: 'foo', + id: '/foo', pattern: '/^/foo/?$/' }, { - id: 'foo/bar', + id: '/foo/bar', pattern: '/^/foo/bar/?$/' }, { - id: 'foo/bar/baz', + id: '/foo/bar/baz', pattern: '/^/foo/bar/baz/?$/', page: { layouts: [0, 2, undefined, 4], errors: [1, undefined, 3, 5], leaf: 6 } } @@ -585,42 +585,42 @@ test('creates routes with named layouts', () => { assert.equal(routes.filter((route) => route.page).map(simplify_route), [ { - id: 'a/a1', + id: '/a/a1', pattern: '/^/a/a1/?$/', page: { layouts: [0, 4], errors: [1, undefined], leaf: 10 } }, { - id: '(special)/a/a2', + id: '/(special)/a/a2', pattern: '/^/a/a2/?$/', page: { layouts: [0, 2], errors: [1, undefined], leaf: 9 } }, { - id: '(special)/(alsospecial)/b/c/c1', + id: '/(special)/(alsospecial)/b/c/c1', pattern: '/^/b/c/c1/?$/', page: { layouts: [0, 2, 3], errors: [1, undefined, undefined], leaf: 8 } }, { - id: 'b/c/c2', + id: '/b/c/c2', pattern: '/^/b/c/c2/?$/', page: { layouts: [0], errors: [1], leaf: 11 } }, { - id: 'b/d/(special)', + id: '/b/d/(special)', pattern: '/^/b/d/?$/', page: { layouts: [0, 6], errors: [1, undefined], leaf: 12 } }, { - id: 'b/d/d1', + id: '/b/d/d1', pattern: '/^/b/d/d1/?$/', page: { layouts: [0], errors: [1], leaf: 15 } }, { - id: 'b/d/(special)/(extraspecial)/d2', + id: '/b/d/(special)/(extraspecial)/d2', pattern: '/^/b/d/d2/?$/', page: { layouts: [0, 6, 7], errors: [1, undefined, undefined], leaf: 13 } }, { - id: 'b/d/(special)/(extraspecial)/d3', + id: '/b/d/(special)/(extraspecial)/d3', pattern: '/^/b/d/d3/?$/', page: { layouts: [0, 6], errors: [1, undefined], leaf: 14 } } @@ -643,30 +643,30 @@ test('handles pages without .svelte file', () => { assert.equal(routes.map(simplify_route), [ { - id: '', + id: '/', pattern: '/^/$/', page: { layouts: [0], errors: [1], leaf: 4 } }, { - id: 'error', + id: '/error', pattern: '/^/error/?$/' }, { - id: 'error/[...path]', + id: '/error/[...path]', pattern: '/^/error(?:/(.*))?/?$/', page: { layouts: [0, undefined], errors: [1, 2], leaf: 5 } }, { - id: 'layout', + id: '/layout', pattern: '/^/layout/?$/' }, { - id: 'layout/exists', + id: '/layout/exists', pattern: '/^/layout/exists/?$/', page: { layouts: [0, 3], errors: [1, undefined], leaf: 6 } }, { - id: 'layout/redirect', + id: '/layout/redirect', pattern: '/^/layout/redirect/?$/', page: { layouts: [0, 3], errors: [1, undefined], leaf: 7 } } @@ -725,7 +725,7 @@ test('errors on duplicate matchers', () => { test('prevents route conflicts between groups', () => { assert.throws( () => create('samples/conflicting-groups'), - /The "\(x\)\/a" and "\(y\)\/a" routes conflict with each other/ + /The "\/\(x\)\/a" and "\/\(y\)\/a" routes conflict with each other/ ); }); diff --git a/packages/kit/src/runtime/server/page/render.js b/packages/kit/src/runtime/server/page/render.js index 7953d33016c3..c0650e00f455 100644 --- a/packages/kit/src/runtime/server/page/render.js +++ b/packages/kit/src/runtime/server/page/render.js @@ -179,7 +179,7 @@ export async function render_response({ const match = /\[(\d+)\]\.data\.(.+)/.exec(error.path); if (match) { throw new Error( - `Data returned from \`load\` while rendering /${event.routeId} is not serializable: ${error.message} (data.${match[2]})` + `Data returned from \`load\` while rendering ${event.routeId} is not serializable: ${error.message} (data.${match[2]})` ); } throw error; diff --git a/packages/kit/src/runtime/server/utils.js b/packages/kit/src/runtime/server/utils.js index 33ef9b7c06ba..e2559e1a2e45 100644 --- a/packages/kit/src/runtime/server/utils.js +++ b/packages/kit/src/runtime/server/utils.js @@ -83,7 +83,7 @@ export function data_response(data, event) { const error = /** @type {any} */ (e); const match = /\[(\d+)\]\.data\.(.+)/.exec(error.path); const message = match - ? `Data returned from \`load\` while rendering /${event.routeId} is not serializable: ${error.message} (data.${match[2]})` + ? `Data returned from \`load\` while rendering ${event.routeId} is not serializable: ${error.message} (data.${match[2]})` : error.message; return new Response(JSON.stringify(message), { headers, status: 500 }); } diff --git a/packages/kit/src/utils/routing.js b/packages/kit/src/utils/routing.js index 921c62d706c5..85b9acdbb86b 100644 --- a/packages/kit/src/utils/routing.js +++ b/packages/kit/src/utils/routing.js @@ -13,11 +13,12 @@ export function parse_route_id(id) { let add_trailing_slash = true; const pattern = - id === '' + id === '/' ? /^\/$/ : new RegExp( `^${id .split(/(?:\/|$)/) + .slice(1) .filter(affects_path) .map((segment, i, segments) => { const decoded_segment = decodeURIComponent(segment); diff --git a/packages/kit/src/utils/routing.spec.js b/packages/kit/src/utils/routing.spec.js index 1ee6553127ea..ac356049ac76 100644 --- a/packages/kit/src/utils/routing.spec.js +++ b/packages/kit/src/utils/routing.spec.js @@ -3,67 +3,67 @@ import * as assert from 'uvu/assert'; import { exec, parse_route_id } from './routing.js'; const tests = { - '': { + '/': { pattern: /^\/$/, names: [], types: [] }, - blog: { + '/blog': { pattern: /^\/blog\/?$/, names: [], types: [] }, - 'blog.json': { + '/blog.json': { pattern: /^\/blog\.json$/, names: [], types: [] }, - 'blog/[slug]': { + '/blog/[slug]': { pattern: /^\/blog\/([^/]+?)\/?$/, names: ['slug'], types: [undefined] }, - 'blog/[slug].json': { + '/blog/[slug].json': { pattern: /^\/blog\/([^/]+?)\.json$/, names: ['slug'], types: [undefined] }, - 'blog/[[slug]]': { + '/blog/[[slug]]': { pattern: /^\/blog(?:\/([^/]+))?\/?$/, names: ['slug'], types: [undefined] }, - 'blog/[[slug=type]]/sub': { + '/blog/[[slug=type]]/sub': { pattern: /^\/blog(?:\/([^/]+))?\/sub\/?$/, names: ['slug'], types: ['type'] }, - 'blog/[[slug]].json': { + '/blog/[[slug]].json': { pattern: /^\/blog\/([^/]*)?\.json$/, names: ['slug'], types: [undefined] }, - '[...catchall]': { + '/[...catchall]': { pattern: /^(?:\/(.*))?\/?$/, names: ['catchall'], types: [undefined] }, - 'foo/[...catchall]/bar': { + '/foo/[...catchall]/bar': { pattern: /^\/foo(?:\/(.*))?\/bar\/?$/, names: ['catchall'], types: [undefined] }, - 'matched/[id=uuid]': { + '/matched/[id=uuid]': { pattern: /^\/matched\/([^/]+?)\/?$/, names: ['id'], types: ['uuid'] }, - '%23hash-encoded': { + '/%23hash-encoded': { pattern: /^\/%23hash-encoded\/?$/, names: [], types: [] }, - '%40at-encoded/[id]': { + '/%40at-encoded/[id]': { pattern: /^\/@at-encoded\/([^/]+?)\/?$/, names: ['id'], types: [undefined] @@ -82,57 +82,57 @@ for (const [key, expected] of Object.entries(tests)) { const exec_tests = [ { - route: 'blog/[[slug]]/sub[[param]]', + route: '/blog/[[slug]]/sub[[param]]', path: '/blog/sub', expected: { slug: '', param: '' } }, { - route: 'blog/[[slug]]/sub[[param]]', + route: '/blog/[[slug]]/sub[[param]]', path: '/blog/slug/sub', expected: { slug: 'slug', param: '' } }, { - route: 'blog/[[slug]]/sub[[param]]', + route: '/blog/[[slug]]/sub[[param]]', path: '/blog/slug/subparam', expected: { slug: 'slug', param: 'param' } }, { - route: 'blog/[[slug]]/sub[[param]]', + route: '/blog/[[slug]]/sub[[param]]', path: '/blog/subparam', expected: { slug: '', param: 'param' } }, { - route: '[[slug]]/[...rest]', + route: '/[[slug]]/[...rest]', path: '/slug/rest/sub', expected: { slug: 'slug', rest: 'rest/sub' } }, { - route: '[[slug]]/[...rest]', + route: '/[[slug]]/[...rest]', path: '/slug/rest', expected: { slug: 'slug', rest: 'rest' } }, { - route: '[[slug]]/[...rest]', + route: '/[[slug]]/[...rest]', path: '/slug', expected: { slug: 'slug', rest: '' } }, { - route: '[[slug]]/[...rest]', + route: '/[[slug]]/[...rest]', path: '/', expected: { slug: '', rest: '' } }, { - route: '[...rest]/path', + route: '/[...rest]/path', path: '/rest/path', expected: { rest: 'rest' } }, { - route: '[[slug1]]/[[slug2]]', + route: '/[[slug1]]/[[slug2]]', path: '/slug1/slug2', expected: { slug1: 'slug1', slug2: 'slug2' } }, { - route: '[[slug1]]/[[slug2]]', + route: '/[[slug1]]/[[slug2]]', path: '/slug1', expected: { slug1: 'slug1', slug2: '' } } diff --git a/packages/kit/test/apps/basics/test/server.test.js b/packages/kit/test/apps/basics/test/server.test.js index 2ea3737df9d7..2891ac916629 100644 --- a/packages/kit/test/apps/basics/test/server.test.js +++ b/packages/kit/test/apps/basics/test/server.test.js @@ -304,7 +304,7 @@ test.describe('Routing', () => { test('event.params are available in handle', async ({ request }) => { const response = await request.get('/routing/params-in-handle/banana'); expect(await response.json()).toStrictEqual({ - key: 'routing/params-in-handle/[x]', + key: '/routing/params-in-handle/[x]', params: { x: 'banana' } }); }); diff --git a/packages/kit/test/apps/basics/test/test.js b/packages/kit/test/apps/basics/test/test.js index c0f2325d8507..b352c5b404a3 100644 --- a/packages/kit/test/apps/basics/test/test.js +++ b/packages/kit/test/apps/basics/test/test.js @@ -1665,8 +1665,8 @@ test.describe('Routing', () => { await page.goto('/routing/route-id'); await clicknav('[href="/routing/route-id/foo"]'); - expect(await page.textContent('h1')).toBe('routeId in load: routing/route-id/[x]'); - expect(await page.textContent('h2')).toBe('routeId in store: routing/route-id/[x]'); + expect(await page.textContent('h1')).toBe('routeId in load: /routing/route-id/[x]'); + expect(await page.textContent('h2')).toBe('routeId in store: /routing/route-id/[x]'); }); test('serves a page that clashes with a root directory', async ({ page }) => { From c7a9c4e15a465bd2e8999613edfae627fc8b3139 Mon Sep 17 00:00:00 2001 From: Ignatius Bagus Date: Fri, 21 Oct 2022 11:20:02 +0700 Subject: [PATCH 2/2] expected prefix --- packages/kit/test/build-errors/prerender.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/kit/test/build-errors/prerender.spec.js b/packages/kit/test/build-errors/prerender.spec.js index 209a3dd2b6c7..d646b9990699 100644 --- a/packages/kit/test/build-errors/prerender.spec.js +++ b/packages/kit/test/build-errors/prerender.spec.js @@ -11,7 +11,7 @@ test('prerenderable routes must be prerendered', () => { stdio: 'pipe', timeout: 15000 }), - /The following routes were marked as prerenderable, but were not prerendered because they were not found while crawling your app:\r?\n - \[x\]/gs + /The following routes were marked as prerenderable, but were not prerendered because they were not found while crawling your app:\r?\n - \/\[x\]/gs ); });