diff --git a/.changeset/eighty-scissors-kick.md b/.changeset/eighty-scissors-kick.md new file mode 100644 index 000000000000..4f82777eddd2 --- /dev/null +++ b/.changeset/eighty-scissors-kick.md @@ -0,0 +1,5 @@ +--- +'@sveltejs/kit': major +--- + +breaking: remove `resolvePath` in favour of `resolveRoute` from `$app/paths` diff --git a/documentation/docs/60-appendix/30-migrating-to-sveltekit-2.md b/documentation/docs/60-appendix/30-migrating-to-sveltekit-2.md index ad7132df60d3..7dabe7ec4666 100644 --- a/documentation/docs/60-appendix/30-migrating-to-sveltekit-2.md +++ b/documentation/docs/60-appendix/30-migrating-to-sveltekit-2.md @@ -92,6 +92,23 @@ This inconsistency is fixed in version 2. Paths are either always relative or al Previously it was possible to track URLs from `fetch`es on the server in order to rerun load functions. This poses a possible security risk (private URLs leaking), and as such it was behind the `dangerZone.trackServerFetches` setting, which is now removed. +## `resolvePath` has been removed + +SvelteKit 1 included a function called `resolvePath` which allows you to resolve a route ID (like `/blog/[slug]`) and a set of parameters (like `{ slug: 'hello' }`) to a pathname. Unfortunately the return value didn't include the `base` path, limiting its usefulness in cases where `base` was set. + +As such, SvelteKit 2 replaces `resolvePath` with a (slightly better named) function called `resolveRoute`, which is imported from `$app/paths` and which takes `base` into account. + +```diff +-import { resolvePath } from '@sveltejs/kit'; +-import { base } from '$app/paths'; ++import { resolveRoute } from '$app/paths'; + +-const path = base + resolvePath('/blog/[slug]', { slug }); ++const path = resolveRoute('/blog/[slug]', { slug }); +``` + +`svelte-migrate` will do the method replacement for you, though if you later prepend the result with `base`, you need to remove that yourself. + ## Updated dependency requirements SvelteKit requires Node `18.13` or higher, Vite `^5.0`, vite-plugin-svelte `^3.0`, TypeScript `^5.0` and Svelte version 4 or higher. `svelte-migrate` will do the `package.json` bumps for you. diff --git a/packages/kit/src/core/postbuild/analyse.js b/packages/kit/src/core/postbuild/analyse.js index 891767c49b37..73bd7faa40b6 100644 --- a/packages/kit/src/core/postbuild/analyse.js +++ b/packages/kit/src/core/postbuild/analyse.js @@ -11,9 +11,9 @@ import { import { load_config } from '../config/index.js'; import { forked } from '../../utils/fork.js'; import { installPolyfills } from '../../exports/node/polyfills.js'; -import { resolvePath } from '../../exports/index.js'; import { ENDPOINT_METHODS } from '../../constants.js'; import { filter_private_env, filter_public_env } from '../../utils/env.js'; +import { resolve_route } from '../../utils/routing.js'; export default forked(import.meta.url, analyse); @@ -101,7 +101,7 @@ async function analyse({ manifest_path, env }) { }, prerender: page?.prerender ?? endpoint?.prerender, entries: - entries && (await entries()).map((entry_object) => resolvePath(route.id, entry_object)) + entries && (await entries()).map((entry_object) => resolve_route(route.id, entry_object)) }); } diff --git a/packages/kit/src/exports/index.js b/packages/kit/src/exports/index.js index f50bca003a52..283ba840c2ef 100644 --- a/packages/kit/src/exports/index.js +++ b/packages/kit/src/exports/index.js @@ -1,6 +1,5 @@ import { HttpError, Redirect, ActionFailure } from '../runtime/control.js'; import { BROWSER, DEV } from 'esm-env'; -import { get_route_segments } from '../utils/routing.js'; export { VERSION } from '../version.js'; @@ -183,58 +182,3 @@ export function fail(status, data) { // @ts-expect-error unique symbol missing return new ActionFailure(status, data); } - -const basic_param_pattern = /\[(\[)?(\.\.\.)?(\w+?)(?:=(\w+))?\]\]?/g; - -let warned = false; - -/** - * @deprecated Use `resolveRoute` from `$app/paths` instead. - * - * Populate a route ID with params to resolve a pathname. - * @example - * ```js - * resolvePath( - * `/blog/[slug]/[...somethingElse]`, - * { - * slug: 'hello-world', - * somethingElse: 'something/else' - * } - * ); // `/blog/hello-world/something/else` - * ``` - * @param {string} id - * @param {Record} params - * @returns {string} - */ -export function resolvePath(id, params) { - if (!warned) { - console.warn('`resolvePath` is deprecated. Use `resolveRoute` from `$app/paths` instead.'); - warned = true; - } - - const segments = get_route_segments(id); - return ( - '/' + - segments - .map((segment) => - segment.replace(basic_param_pattern, (_, optional, rest, name) => { - const param_value = params[name]; - - // This is nested so TS correctly narrows the type - if (!param_value) { - if (optional) return ''; - if (rest && param_value !== undefined) return ''; - throw new Error(`Missing parameter '${name}' in route ${id}`); - } - - if (param_value.startsWith('/') || param_value.endsWith('/')) - throw new Error( - `Parameter '${name}' in route ${id} cannot start or end with a slash -- this would cause an invalid route like foo//bar` - ); - return param_value; - }) - ) - .filter(Boolean) - .join('/') - ); -} diff --git a/packages/kit/src/exports/index.spec.js b/packages/kit/src/exports/index.spec.js deleted file mode 100644 index 22fbf9784cef..000000000000 --- a/packages/kit/src/exports/index.spec.js +++ /dev/null @@ -1,59 +0,0 @@ -import { assert, expect, test } from 'vitest'; -import { resolvePath } from './index.js'; - -const from_params_tests = [ - { - route: '/blog/[one]/[two]', - params: { one: 'one', two: 'two' }, - expected: '/blog/one/two' - }, - { - route: '/blog/[one=matcher]/[...two]', - params: { one: 'one', two: 'two/three' }, - expected: '/blog/one/two/three' - }, - { - route: '/blog/[one=matcher]/[[two]]', - params: { one: 'one' }, - expected: '/blog/one' - }, - { - route: '/blog/[one]/[two]-and-[three]', - params: { one: 'one', two: '2', three: '3' }, - expected: '/blog/one/2-and-3' - }, - { - route: '/blog/[...one]', - params: { one: '' }, - expected: '/blog' - }, - { - route: '/blog/[one]/[...two]-not-three', - params: { one: 'one', two: 'two/2' }, - expected: '/blog/one/two/2-not-three' - } -]; - -for (const { route, params, expected } of from_params_tests) { - test(`resolvePath generates correct path for ${route}`, () => { - const result = resolvePath(route, params); - assert.equal(result, expected); - }); -} - -test('resolvePath errors on missing params for required param', () => { - expect(() => resolvePath('/blog/[one]/[two]', { one: 'one' })).toThrow( - "Missing parameter 'two' in route /blog/[one]/[two]" - ); -}); - -test('resolvePath errors on params values starting or ending with slashes', () => { - assert.throws( - () => resolvePath('/blog/[one]/[two]', { one: 'one', two: '/two' }), - "Parameter 'two' in route /blog/[one]/[two] cannot start or end with a slash -- this would cause an invalid route like foo//bar" - ); - assert.throws( - () => resolvePath('/blog/[one]/[two]', { one: 'one', two: 'two/' }), - "Parameter 'two' in route /blog/[one]/[two] cannot start or end with a slash -- this would cause an invalid route like foo//bar" - ); -}); diff --git a/packages/kit/src/runtime/app/paths.js b/packages/kit/src/runtime/app/paths.js index 0a55bcfc33c5..32df2e1b3eda 100644 --- a/packages/kit/src/runtime/app/paths.js +++ b/packages/kit/src/runtime/app/paths.js @@ -1,8 +1,6 @@ export { base, assets } from '__sveltekit/paths'; import { base } from '__sveltekit/paths'; -import { get_route_segments } from '../../utils/routing.js'; - -const basic_param_pattern = /\[(\[)?(\.\.\.)?(\w+?)(?:=(\w+))?\]\]?/g; +import { resolve_route } from '../../utils/routing.js'; /** * Populate a route ID with params to resolve a pathname. @@ -21,30 +19,5 @@ const basic_param_pattern = /\[(\[)?(\.\.\.)?(\w+?)(?:=(\w+))?\]\]?/g; * @returns {string} */ export function resolveRoute(id, params) { - const segments = get_route_segments(id); - return ( - base + - '/' + - segments - .map((segment) => - segment.replace(basic_param_pattern, (_, optional, rest, name) => { - const param_value = params[name]; - - // This is nested so TS correctly narrows the type - if (!param_value) { - if (optional) return ''; - if (rest && param_value !== undefined) return ''; - throw new Error(`Missing parameter '${name}' in route ${id}`); - } - - if (param_value.startsWith('/') || param_value.endsWith('/')) - throw new Error( - `Parameter '${name}' in route ${id} cannot start or end with a slash -- this would cause an invalid route like foo//bar` - ); - return param_value; - }) - ) - .filter(Boolean) - .join('/') - ); + return base + resolve_route(id, params); } diff --git a/packages/kit/src/runtime/server/page/actions.js b/packages/kit/src/runtime/server/page/actions.js index 7fcaef78fe23..3bf369716776 100644 --- a/packages/kit/src/runtime/server/page/actions.js +++ b/packages/kit/src/runtime/server/page/actions.js @@ -1,5 +1,5 @@ import * as devalue from 'devalue'; -import { json } from '../../../exports/index.js'; +import { error, json } from '../../../exports/index.js'; import { normalize_error } from '../../../utils/error.js'; import { is_form_content_type, negotiate } from '../../../utils/http.js'; import { HttpError, Redirect, ActionFailure } from '../../control.js'; diff --git a/packages/kit/src/utils/routing.js b/packages/kit/src/utils/routing.js index f55c0762b10c..32a821a38c4f 100644 --- a/packages/kit/src/utils/routing.js +++ b/packages/kit/src/utils/routing.js @@ -214,3 +214,49 @@ function escape(str) { .replace(/[.*+?^${}()|\\]/g, '\\$&') ); } + +const basic_param_pattern = /\[(\[)?(\.\.\.)?(\w+?)(?:=(\w+))?\]\]?/g; + +/** + * Populate a route ID with params to resolve a pathname. + * @example + * ```js + * resolveRoute( + * `/blog/[slug]/[...somethingElse]`, + * { + * slug: 'hello-world', + * somethingElse: 'something/else' + * } + * ); // `/blog/hello-world/something/else` + * ``` + * @param {string} id + * @param {Record} params + * @returns {string} + */ +export function resolve_route(id, params) { + const segments = get_route_segments(id); + return ( + '/' + + segments + .map((segment) => + segment.replace(basic_param_pattern, (_, optional, rest, name) => { + const param_value = params[name]; + + // This is nested so TS correctly narrows the type + if (!param_value) { + if (optional) return ''; + if (rest && param_value !== undefined) return ''; + throw new Error(`Missing parameter '${name}' in route ${id}`); + } + + if (param_value.startsWith('/') || param_value.endsWith('/')) + throw new Error( + `Parameter '${name}' in route ${id} cannot start or end with a slash -- this would cause an invalid route like foo//bar` + ); + return param_value; + }) + ) + .filter(Boolean) + .join('/') + ); +} diff --git a/packages/kit/src/utils/routing.spec.js b/packages/kit/src/utils/routing.spec.js index 4b8dc6fcda47..e5595a28a4fb 100644 --- a/packages/kit/src/utils/routing.spec.js +++ b/packages/kit/src/utils/routing.spec.js @@ -1,263 +1,326 @@ -import { assert, expect, test } from 'vitest'; -import { exec, parse_route_id } from './routing.js'; +import { assert, expect, test, describe } from 'vitest'; +import { exec, parse_route_id, resolve_route } from './routing.js'; -const tests = { - '/': { - pattern: /^\/$/, - params: [] - }, - '/blog': { - pattern: /^\/blog\/?$/, - params: [] - }, - '/blog.json': { - pattern: /^\/blog\.json\/?$/, - params: [] - }, - '/blog/[slug]': { - pattern: /^\/blog\/([^/]+?)\/?$/, - params: [{ name: 'slug', matcher: undefined, optional: false, rest: false, chained: false }] - }, - '/blog/[slug].json': { - pattern: /^\/blog\/([^/]+?)\.json\/?$/, - params: [{ name: 'slug', matcher: undefined, optional: false, rest: false, chained: false }] - }, - '/blog/[[slug]]': { - pattern: /^\/blog(?:\/([^/]+))?\/?$/, - 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, rest: false, chained: true }] - }, - '/blog/[[slug]].json': { - pattern: /^\/blog\/([^/]*)?\.json\/?$/, - params: [{ name: 'slug', matcher: undefined, optional: true, rest: false, chained: false }] - }, - '/[...catchall]': { - pattern: /^(?:\/(.*))?\/?$/, - params: [{ name: 'catchall', matcher: undefined, optional: false, rest: true, chained: true }] - }, - '/foo/[...catchall]/bar': { - pattern: /^\/foo(?:\/(.*))?\/bar\/?$/, - params: [{ name: 'catchall', matcher: undefined, optional: false, rest: true, chained: true }] - }, - '/matched/[id=uuid]': { - pattern: /^\/matched\/([^/]+?)\/?$/, - params: [{ name: 'id', matcher: 'uuid', optional: false, rest: false, chained: false }] - }, - '/@-symbol/[id]': { - pattern: /^\/@-symbol\/([^/]+?)\/?$/, - params: [{ name: 'id', matcher: undefined, optional: false, rest: false, chained: false }] - } -}; +describe('parse_route_id', () => { + const tests = { + '/': { + pattern: /^\/$/, + params: [] + }, + '/blog': { + pattern: /^\/blog\/?$/, + params: [] + }, + '/blog.json': { + pattern: /^\/blog\.json\/?$/, + params: [] + }, + '/blog/[slug]': { + pattern: /^\/blog\/([^/]+?)\/?$/, + params: [{ name: 'slug', matcher: undefined, optional: false, rest: false, chained: false }] + }, + '/blog/[slug].json': { + pattern: /^\/blog\/([^/]+?)\.json\/?$/, + params: [{ name: 'slug', matcher: undefined, optional: false, rest: false, chained: false }] + }, + '/blog/[[slug]]': { + pattern: /^\/blog(?:\/([^/]+))?\/?$/, + 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, rest: false, chained: true }] + }, + '/blog/[[slug]].json': { + pattern: /^\/blog\/([^/]*)?\.json\/?$/, + params: [{ name: 'slug', matcher: undefined, optional: true, rest: false, chained: false }] + }, + '/[...catchall]': { + pattern: /^(?:\/(.*))?\/?$/, + params: [{ name: 'catchall', matcher: undefined, optional: false, rest: true, chained: true }] + }, + '/foo/[...catchall]/bar': { + pattern: /^\/foo(?:\/(.*))?\/bar\/?$/, + params: [{ name: 'catchall', matcher: undefined, optional: false, rest: true, chained: true }] + }, + '/matched/[id=uuid]': { + pattern: /^\/matched\/([^/]+?)\/?$/, + params: [{ name: 'id', matcher: 'uuid', optional: false, rest: false, chained: false }] + }, + '/@-symbol/[id]': { + pattern: /^\/@-symbol\/([^/]+?)\/?$/, + params: [{ name: 'id', matcher: undefined, optional: false, rest: false, chained: false }] + } + }; + + for (const [key, expected] of Object.entries(tests)) { + test(key, () => { + const actual = parse_route_id(key); -for (const [key, expected] of Object.entries(tests)) { - test(`parse_route_id: "${key}"`, () => { - const actual = parse_route_id(key); + expect(actual.pattern.toString()).toEqual(expected.pattern.toString()); + expect(actual.params).toEqual(expected.params); + }); + } - expect(actual.pattern.toString()).toEqual(expected.pattern.toString()); - expect(actual.params).toEqual(expected.params); + test('errors on bad param name', () => { + assert.throws(() => parse_route_id('abc/[b-c]'), /Invalid param: b-c/); + assert.throws(() => parse_route_id('abc/[bc=d-e]'), /Invalid param: bc=d-e/); }); -} +}); + +describe('exec', () => { + const tests = [ + { + route: '/blog/[[slug]]/sub[[param]]', + path: '/blog/sub', + expected: {} + }, + { + route: '/blog/[[slug]]/sub[[param]]', + path: '/blog/slug/sub', + expected: { slug: 'slug' } + }, + { + route: '/blog/[[slug]]/sub[[param]]', + path: '/blog/slug/subparam', + expected: { slug: 'slug', param: 'param' } + }, + { + route: '/blog/[[slug]]/sub[[param]]', + path: '/blog/subparam', + expected: { param: 'param' } + }, + { + route: '/[[slug]]/[...rest]', + path: '/slug/rest/sub', + expected: { slug: 'slug', rest: 'rest/sub' } + }, + { + route: '/[[slug]]/[...rest]', + path: '/slug/rest', + expected: { slug: 'slug', rest: 'rest' } + }, + { + route: '/[[slug]]/[...rest]', + path: '/slug', + expected: { slug: 'slug', rest: '' } + }, + { + route: '/[[slug]]/[...rest]', + path: '/', + expected: { rest: '' } + }, + { + route: '/[...rest]/path', + path: '/rest/path', + expected: { rest: 'rest' } + }, + { + route: '/[[slug1]]/[[slug2]]', + path: '/slug1/slug2', + expected: { slug1: 'slug1', slug2: 'slug2' } + }, + { + route: '/[[slug1]]/[[slug2]]', + path: '/slug1', + expected: { slug1: 'slug1' } + }, + { + route: '/[[slug1=matches]]', + path: '/', + expected: {} + }, + { + route: '/[[slug1=doesntmatch]]', + path: '/', + expected: {} + }, + { + route: '/[[slug1=matches]]/[[slug2=doesntmatch]]', + path: '/foo', + expected: { slug1: 'foo' } + }, + { + route: '/[[slug1=doesntmatch]]/[[slug2=doesntmatch]]', + path: '/foo', + expected: undefined + }, + { + route: '/[...slug1=matches]', + path: '/', + expected: { slug1: '' } + }, + { + route: '/[...slug1=doesntmatch]', + path: '/', + expected: undefined + }, + { + route: '/[[slug=doesntmatch]]/[...rest]', + path: '/foo', + expected: { rest: 'foo' } + }, + { + route: '/[[slug1=doesntmatch]]/[slug2]/[...rest]', + path: '/foo/bar/baz', + expected: { slug2: 'foo', rest: 'bar/baz' } + }, + { + route: '/[[slug1=doesntmatch]]/[slug2]/[...rest]/baz', + path: '/foo/bar/baz', + expected: { slug2: 'foo', rest: 'bar' } + }, + { + route: '/[[a=doesntmatch]]/[[b=doesntmatch]]/[[c=doesntmatch]]/[...d]', + path: '/a/b/c/d', + expected: { d: 'a/b/c/d' } + }, + { + route: '/[[a=doesntmatch]]/[b]/[...c]/[d]/e', + path: '/foo/bar/baz/qux/e', + expected: { b: 'foo', c: 'bar/baz', d: 'qux' } + }, + { + route: '/[[slug1=doesntmatch]]/[[slug2=doesntmatch]]/[...rest]', + path: '/foo/bar/baz', + expected: { rest: 'foo/bar/baz' } + }, + { + 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 + }, + { + route: '/[[slug1=matches]]/[[slug2=matches]]/constant/[[slug3=matches]]', + path: '/a/b/constant/c', + expected: { slug1: 'a', slug2: 'b', slug3: 'c' } + }, + { + route: '/[[slug1=doesntmatch]]/[[slug2=matches]]/constant/[[slug3=matches]]', + path: '/b/constant/c', + expected: { slug2: 'b', slug3: 'c' } + }, + { + route: '/[[slug1=doesntmatch]]/[[slug2=matches]]/[[slug3=matches]]', + path: '/b/c', + expected: { slug2: 'b', slug3: 'c' } + }, + { + route: '/[slug1]/[[lang=doesntmatch]]/[[page=matches]]', + path: '/a/2', + expected: { slug1: 'a', lang: undefined, page: '2' } + }, + { + route: '/[[slug1=doesntmatch]]/[slug2=matches]/[slug3]', + path: '/a/b/c', + expected: undefined + }, + { + route: '/[[lang=doesntmatch]]/[asset=matches]/[[categoryType]]/[...categories]', + path: '/music', + expected: { asset: 'music', categories: '' } + }, + { + route: '/[[lang=doesntmatch]]/[asset=matches]/[[categoryType]]/[...categories]', + path: '/music/genre', + expected: { asset: 'music', categoryType: 'genre', categories: '' } + }, + { + route: '/[[lang=doesntmatch]]/[asset=matches]/[[categoryType]]/[...categories]', + path: '/music/genre/rock', + expected: { asset: 'music', categoryType: 'genre', categories: 'rock' } + }, + { + route: '/[[lang=doesntmatch]]/[asset=matches]/[[categoryType]]/[...categories]', + path: '/sfx/category/car/crash', + expected: { asset: 'sfx', categoryType: 'category', categories: 'car/crash' } + }, + { + route: '/[[lang=matches]]/[asset=matches]/[[categoryType]]/[...categories]', + path: '/es/sfx/category/car/crash', + expected: { lang: 'es', asset: 'sfx', categoryType: 'category', categories: 'car/crash' } + }, + { + route: '/[[slug1=doesntmatch]]/[...slug2=doesntmatch]', + path: '/a/b/c', + expected: undefined + } + ]; -const exec_tests = [ - { - route: '/blog/[[slug]]/sub[[param]]', - path: '/blog/sub', - expected: {} - }, - { - route: '/blog/[[slug]]/sub[[param]]', - path: '/blog/slug/sub', - expected: { slug: 'slug' } - }, - { - route: '/blog/[[slug]]/sub[[param]]', - path: '/blog/slug/subparam', - expected: { slug: 'slug', param: 'param' } - }, - { - route: '/blog/[[slug]]/sub[[param]]', - path: '/blog/subparam', - expected: { param: 'param' } - }, - { - route: '/[[slug]]/[...rest]', - path: '/slug/rest/sub', - expected: { slug: 'slug', rest: 'rest/sub' } - }, - { - route: '/[[slug]]/[...rest]', - path: '/slug/rest', - expected: { slug: 'slug', rest: 'rest' } - }, - { - route: '/[[slug]]/[...rest]', - path: '/slug', - expected: { slug: 'slug', rest: '' } - }, - { - route: '/[[slug]]/[...rest]', - path: '/', - expected: { rest: '' } - }, - { - route: '/[...rest]/path', - path: '/rest/path', - expected: { rest: 'rest' } - }, - { - route: '/[[slug1]]/[[slug2]]', - path: '/slug1/slug2', - expected: { slug1: 'slug1', slug2: 'slug2' } - }, - { - route: '/[[slug1]]/[[slug2]]', - path: '/slug1', - expected: { slug1: 'slug1' } - }, - { - route: '/[[slug1=matches]]', - path: '/', - expected: {} - }, - { - route: '/[[slug1=doesntmatch]]', - path: '/', - expected: {} - }, - { - route: '/[[slug1=matches]]/[[slug2=doesntmatch]]', - path: '/foo', - expected: { slug1: 'foo' } - }, - { - route: '/[[slug1=doesntmatch]]/[[slug2=doesntmatch]]', - path: '/foo', - expected: undefined - }, - { - route: '/[...slug1=matches]', - path: '/', - expected: { slug1: '' } - }, - { - route: '/[...slug1=doesntmatch]', - path: '/', - expected: undefined - }, - { - route: '/[[slug=doesntmatch]]/[...rest]', - path: '/foo', - expected: { rest: 'foo' } - }, - { - route: '/[[slug1=doesntmatch]]/[slug2]/[...rest]', - path: '/foo/bar/baz', - expected: { slug2: 'foo', rest: 'bar/baz' } - }, - { - route: '/[[slug1=doesntmatch]]/[slug2]/[...rest]/baz', - path: '/foo/bar/baz', - expected: { slug2: 'foo', rest: 'bar' } - }, - { - route: '/[[a=doesntmatch]]/[[b=doesntmatch]]/[[c=doesntmatch]]/[...d]', - path: '/a/b/c/d', - expected: { d: 'a/b/c/d' } - }, - { - route: '/[[a=doesntmatch]]/[b]/[...c]/[d]/e', - path: '/foo/bar/baz/qux/e', - expected: { b: 'foo', c: 'bar/baz', d: 'qux' } - }, - { - route: '/[[slug1=doesntmatch]]/[[slug2=doesntmatch]]/[...rest]', - path: '/foo/bar/baz', - expected: { rest: 'foo/bar/baz' } - }, - { - 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 - }, - { - route: '/[[slug1=matches]]/[[slug2=matches]]/constant/[[slug3=matches]]', - path: '/a/b/constant/c', - expected: { slug1: 'a', slug2: 'b', slug3: 'c' } - }, - { - route: '/[[slug1=doesntmatch]]/[[slug2=matches]]/constant/[[slug3=matches]]', - path: '/b/constant/c', - expected: { slug2: 'b', slug3: 'c' } - }, - { - route: '/[[slug1=doesntmatch]]/[[slug2=matches]]/[[slug3=matches]]', - path: '/b/c', - expected: { slug2: 'b', slug3: 'c' } - }, - { - route: '/[slug1]/[[lang=doesntmatch]]/[[page=matches]]', - path: '/a/2', - expected: { slug1: 'a', lang: undefined, page: '2' } - }, - { - route: '/[[slug1=doesntmatch]]/[slug2=matches]/[slug3]', - path: '/a/b/c', - expected: undefined - }, - { - route: '/[[lang=doesntmatch]]/[asset=matches]/[[categoryType]]/[...categories]', - path: '/music', - expected: { asset: 'music', categories: '' } - }, - { - route: '/[[lang=doesntmatch]]/[asset=matches]/[[categoryType]]/[...categories]', - path: '/music/genre', - expected: { asset: 'music', categoryType: 'genre', categories: '' } - }, - { - route: '/[[lang=doesntmatch]]/[asset=matches]/[[categoryType]]/[...categories]', - path: '/music/genre/rock', - expected: { asset: 'music', categoryType: 'genre', categories: 'rock' } - }, - { - route: '/[[lang=doesntmatch]]/[asset=matches]/[[categoryType]]/[...categories]', - path: '/sfx/category/car/crash', - expected: { asset: 'sfx', categoryType: 'category', categories: 'car/crash' } - }, - { - route: '/[[lang=matches]]/[asset=matches]/[[categoryType]]/[...categories]', - path: '/es/sfx/category/car/crash', - expected: { lang: 'es', asset: 'sfx', categoryType: 'category', categories: 'car/crash' } - }, - { - route: '/[[slug1=doesntmatch]]/[...slug2=doesntmatch]', - path: '/a/b/c', - expected: undefined + for (const { path, route, expected } of tests) { + test(`exec extracts params correctly for ${path} from ${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, params, { + matches: () => true, + doesntmatch: () => false + }); + expect(actual).toEqual(expected); + }); } -]; +}); + +describe('resolve_route', () => { + const from_params_tests = [ + { + route: '/blog/[one]/[two]', + params: { one: 'one', two: 'two' }, + expected: '/blog/one/two' + }, + { + route: '/blog/[one=matcher]/[...two]', + params: { one: 'one', two: 'two/three' }, + expected: '/blog/one/two/three' + }, + { + route: '/blog/[one=matcher]/[[two]]', + params: { one: 'one' }, + expected: '/blog/one' + }, + { + route: '/blog/[one]/[two]-and-[three]', + params: { one: 'one', two: '2', three: '3' }, + expected: '/blog/one/2-and-3' + }, + { + route: '/blog/[...one]', + params: { one: '' }, + expected: '/blog' + }, + { + route: '/blog/[one]/[...two]-not-three', + params: { one: 'one', two: 'two/2' }, + expected: '/blog/one/two/2-not-three' + } + ]; -for (const { path, route, expected } of exec_tests) { - test(`exec extracts params correctly for ${path} from ${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, params, { - matches: () => true, - doesntmatch: () => false + for (const { route, params, expected } of from_params_tests) { + test(`resolvePath generates correct path for ${route}`, () => { + const result = resolve_route(route, params); + assert.equal(result, expected); }); - expect(actual).toEqual(expected); + } + + test('resolvePath errors on missing params for required param', () => { + expect(() => resolve_route('/blog/[one]/[two]', { one: 'one' })).toThrow( + "Missing parameter 'two' in route /blog/[one]/[two]" + ); }); -} -test('parse_route_id errors on bad param name', () => { - assert.throws(() => parse_route_id('abc/[b-c]'), /Invalid param: b-c/); - assert.throws(() => parse_route_id('abc/[bc=d-e]'), /Invalid param: bc=d-e/); + test('resolvePath errors on params values starting or ending with slashes', () => { + assert.throws( + () => resolve_route('/blog/[one]/[two]', { one: 'one', two: '/two' }), + "Parameter 'two' in route /blog/[one]/[two] cannot start or end with a slash -- this would cause an invalid route like foo//bar" + ); + assert.throws( + () => resolve_route('/blog/[one]/[two]', { one: 'one', two: 'two/' }), + "Parameter 'two' in route /blog/[one]/[two] cannot start or end with a slash -- this would cause an invalid route like foo//bar" + ); + }); }); diff --git a/packages/kit/types/index.d.ts b/packages/kit/types/index.d.ts index 0af573c9452d..3c55fa3db610 100644 --- a/packages/kit/types/index.d.ts +++ b/packages/kit/types/index.d.ts @@ -1738,22 +1738,6 @@ declare module '@sveltejs/kit' { * @param data Data associated with the failure (e.g. validation errors) * */ export function fail | undefined = undefined>(status: number, data: T): ActionFailure; - /** - * @deprecated Use `resolveRoute` from `$app/paths` instead. - * - * Populate a route ID with params to resolve a pathname. - * @example - * ```js - * resolvePath( - * `/blog/[slug]/[...somethingElse]`, - * { - * slug: 'hello-world', - * somethingElse: 'something/else' - * } - * ); // `/blog/hello-world/something/else` - * ``` - * */ - export function resolvePath(id: string, params: Record): string; export type LessThan = TNumber extends TArray['length'] ? TArray[number] : LessThan; export type NumericRange = Exclude, LessThan>; export const VERSION: string; diff --git a/packages/migrate/migrations/sveltekit-2/migrate.js b/packages/migrate/migrations/sveltekit-2/migrate.js index 8cba1eb54045..e956baa40601 100644 --- a/packages/migrate/migrations/sveltekit-2/migrate.js +++ b/packages/migrate/migrations/sveltekit-2/migrate.js @@ -93,6 +93,7 @@ export function transform_code(code, _is_ts, file_path) { const source = project.createSourceFile('svelte.ts', code); remove_throws(source); add_cookie_note(file_path, source); + replace_resolve_path(source); return source.getFullText(); } @@ -108,14 +109,13 @@ function remove_throws(source) { /** @param {string} id */ function remove_throw(id) { - const imports = get_imports(source, '@sveltejs/kit', id); - for (const namedImport of imports) { - for (const id of namedImport.getNameNode().findReferencesAsNodes()) { - const call_expression = id.getParent(); - const throw_stmt = call_expression?.getParent(); - if (Node.isCallExpression(call_expression) && Node.isThrowStatement(throw_stmt)) { - throw_stmt.replaceWithText(call_expression.getText() + ';'); - } + const namedImport = get_import(source, '@sveltejs/kit', id); + if (!namedImport) return; + for (const id of namedImport.getNameNode().findReferencesAsNodes()) { + const call_expression = id.getParent(); + const throw_stmt = call_expression?.getParent(); + if (Node.isCallExpression(call_expression) && Node.isThrowStatement(throw_stmt)) { + throw_stmt.replaceWithText(call_expression.getText() + ';'); } } } @@ -216,15 +216,45 @@ function add_cookie_note(file_path, source) { logger(); } +/** + * `resolvePath` from `@sveltejs/kit` -> `resolveRoute` from `$app/paths` + * @param {import('ts-morph').SourceFile} source + */ +function replace_resolve_path(source) { + const namedImport = get_import(source, '@sveltejs/kit', 'resolvePath'); + if (!namedImport) return; + + for (const id of namedImport.getNameNode().findReferencesAsNodes()) { + id.replaceWithText('resolveRoute'); + } + if (namedImport.getParent().getParent().getNamedImports().length === 1) { + namedImport.getParent().getParent().getParent().remove(); + } else { + namedImport.remove(); + } + + const paths_import = source.getImportDeclaration( + (i) => i.getModuleSpecifierValue() === '$app/paths' + ); + if (paths_import) { + paths_import.addNamedImport('resolveRoute'); + } else { + source.addImportDeclaration({ + moduleSpecifier: '$app/paths', + namedImports: ['resolveRoute'] + }); + } +} + /** * @param {import('ts-morph').SourceFile} source * @param {string} from * @param {string} name */ -function get_imports(source, from, name) { +function get_import(source, from, name) { return source .getImportDeclarations() .filter((i) => i.getModuleSpecifierValue() === from) .flatMap((i) => i.getNamedImports()) - .filter((i) => i.getName() === name); + .find((i) => i.getName() === name); } diff --git a/packages/migrate/migrations/sveltekit-2/tsjs-samples.md b/packages/migrate/migrations/sveltekit-2/tsjs-samples.md index fe0842cced93..499808ad7b24 100644 --- a/packages/migrate/migrations/sveltekit-2/tsjs-samples.md +++ b/packages/migrate/migrations/sveltekit-2/tsjs-samples.md @@ -90,7 +90,7 @@ export function load(event) { } ``` -## Recognizes false positives +## Recognizes cookies false positives ```js before export function load({ cookies }) { @@ -115,3 +115,48 @@ export function foo(event) { cookies.set('foo', 'bar'); ``` + +## Replaces resolvePath + +```js before +import { resolvePath } from '@sveltejs/kit'; + +resolvePath('x', y); +``` + + +```js after +import { resolveRoute } from "$app/paths"; + +resolveRoute('x', y); +``` + +## Replaces resolvePath taking care of imports + +```js before +import { resolvePath, x } from '@sveltejs/kit'; +import { y } from '$app/paths'; + +resolvePath('x'); +``` + +```js after +import { x } from '@sveltejs/kit'; +import { y, resolveRoute } from '$app/paths'; + +resolveRoute('x'); +``` + +## Doesn't replace resolvePath from other sources + +```js before +import { resolvePath } from 'x'; + +resolvePath('x'); +``` + +```js after +import { resolvePath } from 'x'; + +resolvePath('x'); +```