From a504d0bb9aea9b0d97d024c9ca0f139fe865f98b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20J=C3=A4ger?= Date: Fri, 13 Sep 2024 12:49:44 +0200 Subject: [PATCH] Remove dependency on path-to-regexp --- .changeset/shiny-worms-rest.md | 5 + packages/astro/package.json | 1 - .../src/core/routing/manifest/generator.ts | 84 +++++----- .../test/units/routing/generator.test.js | 152 ++++++++++++++++++ pnpm-lock.yaml | 8 - 5 files changed, 197 insertions(+), 53 deletions(-) create mode 100644 .changeset/shiny-worms-rest.md create mode 100644 packages/astro/test/units/routing/generator.test.js diff --git a/.changeset/shiny-worms-rest.md b/.changeset/shiny-worms-rest.md new file mode 100644 index 000000000000..90e869eab24d --- /dev/null +++ b/.changeset/shiny-worms-rest.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Remove dependency on path-to-regexp diff --git a/packages/astro/package.json b/packages/astro/package.json index d338b037ca45..3de8ebdb31d9 100644 --- a/packages/astro/package.json +++ b/packages/astro/package.json @@ -168,7 +168,6 @@ "ora": "^8.1.0", "p-limit": "^6.1.0", "p-queue": "^8.0.1", - "path-to-regexp": "6.2.2", "preferred-pm": "^4.0.0", "prompts": "^2.4.2", "rehype": "^13.0.1", diff --git a/packages/astro/src/core/routing/manifest/generator.ts b/packages/astro/src/core/routing/manifest/generator.ts index 4ab635ec6608..48ae26b1933b 100644 --- a/packages/astro/src/core/routing/manifest/generator.ts +++ b/packages/astro/src/core/routing/manifest/generator.ts @@ -1,15 +1,11 @@ import type { AstroConfig, RoutePart } from '../../../@types/astro.js'; -import { compile } from 'path-to-regexp'; - /** * Sanitizes the parameters object by normalizing string values and replacing certain characters with their URL-encoded equivalents. - * @param {Record} params - The parameters object to be sanitized. - * @returns {Record} The sanitized parameters object. + * @param {Record} params - The parameters object to be sanitized. + * @returns {Record} The sanitized parameters object. */ -function sanitizeParams( - params: Record, -): Record { +function sanitizeParams(params: Record): Record { return Object.fromEntries( Object.entries(params).map(([key, value]) => { if (typeof value === 'string') { @@ -20,49 +16,49 @@ function sanitizeParams( ); } +function getParameter(part: RoutePart, params: Record): string | number { + if (part.spread) { + return params[part.content.slice(3)] || ''; + } + + if (part.dynamic) { + if (!params[part.content]) { + throw new TypeError(`Missing parameter: ${part.content}`); + } + + return params[part.content]; + } + + return part.content + .normalize() + .replace(/\?/g, '%3F') + .replace(/#/g, '%23') + .replace(/%5B/g, '[') + .replace(/%5D/g, ']'); +} + +function getSegment(segment: RoutePart[], params: Record): string { + const segmentPath = segment.map((part) => getParameter(part, params)).join(''); + + return segmentPath ? '/' + segmentPath : ''; +} + export function getRouteGenerator( segments: RoutePart[][], addTrailingSlash: AstroConfig['trailingSlash'], ) { - const template = segments - .map((segment) => { - return ( - '/' + - segment - .map((part) => { - if (part.spread) { - return `:${part.content.slice(3)}(.*)?`; - } else if (part.dynamic) { - return `:${part.content}`; - } else { - return part.content - .normalize() - .replace(/\?/g, '%3F') - .replace(/#/g, '%23') - .replace(/%5B/g, '[') - .replace(/%5D/g, ']') - .replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); - } - }) - .join('') - ); - }) - .join(''); - - // Unless trailingSlash config is set to 'always', don't automatically append it. - let trailing: '/' | '' = ''; - if (addTrailingSlash === 'always' && segments.length) { - trailing = '/'; - } - const toPath = compile(template + trailing); - return (params: Record): string => { + return (params: Record): string => { const sanitizedParams = sanitizeParams(params); - const path = toPath(sanitizedParams); - // When generating an index from a rest parameter route, `path-to-regexp` will return an - // empty string instead "/". This causes an inconsistency with static indexes that may result - // in the incorrect routes being rendered. - // To fix this, we return "/" when the path is empty. + // Unless trailingSlash config is set to 'always', don't automatically append it. + let trailing: '/' | '' = ''; + if (addTrailingSlash === 'always' && segments.length) { + trailing = '/'; + } + + const path = + segments.map((segment) => getSegment(segment, sanitizedParams)).join('') + trailing; + return path || '/'; }; } diff --git a/packages/astro/test/units/routing/generator.test.js b/packages/astro/test/units/routing/generator.test.js new file mode 100644 index 000000000000..d176c73ebc4c --- /dev/null +++ b/packages/astro/test/units/routing/generator.test.js @@ -0,0 +1,152 @@ +import * as assert from 'node:assert/strict'; +import { describe, it } from 'node:test'; + +import { getRouteGenerator } from '../../../dist/core/routing/manifest/generator.js'; + +describe('routing - generator', () => { + [ + { + routeData: [], + trailingSlash: 'never', + params: {}, + path: '/', + }, + { + routeData: [], + trailingSlash: 'always', + params: {}, + path: '/', + }, + { + routeData: [[{ spread: false, content: 'test', dynamic: false }]], + trailingSlash: 'never', + params: {}, + path: '/test', + }, + { + routeData: [[{ spread: false, content: 'test', dynamic: false }]], + trailingSlash: 'always', + params: {}, + path: '/test/', + }, + { + routeData: [[{ spread: false, content: 'test', dynamic: false }]], + trailingSlash: 'always', + params: { foo: 'bar' }, + path: '/test/', + }, + { + routeData: [[{ spread: false, content: 'foo', dynamic: true }]], + trailingSlash: 'always', + params: { foo: 'bar' }, + path: '/bar/', + }, + { + routeData: [[{ spread: false, content: 'foo', dynamic: true }]], + trailingSlash: 'never', + params: { foo: 'bar' }, + path: '/bar', + }, + { + routeData: [[{ spread: true, content: '...foo', dynamic: true }]], + trailingSlash: 'never', + params: {}, + path: '/', + }, + { + routeData: [ + [ + { spread: true, content: '...foo', dynamic: true }, + { spread: false, content: '-', dynamic: false }, + { spread: true, content: '...bar', dynamic: true }, + ], + ], + trailingSlash: 'never', + params: { foo: 'one', bar: 'two' }, + path: '/one-two', + }, + { + routeData: [ + [ + { spread: true, content: '...foo', dynamic: true }, + { spread: false, content: '-', dynamic: false }, + { spread: true, content: '...bar', dynamic: true }, + ], + ], + trailingSlash: 'never', + params: {}, + path: '/-', + }, + { + routeData: [ + [{ spread: true, content: '...foo', dynamic: true }], + [{ spread: true, content: '...bar', dynamic: true }], + ], + trailingSlash: 'never', + params: { foo: 'one' }, + path: '/one', + }, + { + routeData: [ + [{ spread: false, content: 'fix', dynamic: false }], + [{ spread: true, content: '...foo', dynamic: true }], + [{ spread: true, content: '...bar', dynamic: true }], + ], + trailingSlash: 'never', + params: { foo: 'one' }, + path: '/fix/one', + }, + { + routeData: [ + [{ spread: false, content: 'fix', dynamic: false }], + [{ spread: true, content: '...foo', dynamic: true }], + [{ spread: true, content: '...bar', dynamic: true }], + ], + trailingSlash: 'always', + params: { foo: 'one' }, + path: '/fix/one/', + }, + { + routeData: [ + [{ spread: false, content: 'fix', dynamic: false }], + [{ spread: true, content: '...foo', dynamic: true }], + [{ spread: true, content: '...bar', dynamic: true }], + ], + trailingSlash: 'never', + params: { foo: 'one', bar: 'two' }, + path: '/fix/one/two', + }, + { + routeData: [ + [{ spread: false, content: 'fix', dynamic: false }], + [{ spread: true, content: '...foo', dynamic: true }], + [{ spread: true, content: '...bar', dynamic: true }], + ], + trailingSlash: 'never', + params: { foo: 'one&two' }, + path: '/fix/one&two', + }, + { + routeData: [ + [{ spread: false, content: 'fix', dynamic: false }], + [{ spread: false, content: 'page', dynamic: true }], + ], + trailingSlash: 'never', + params: { page: 1 }, + path: '/fix/1', + }, + ].forEach(({ routeData, trailingSlash, params, path }) => { + it(`generates ${path}`, () => { + const generator = getRouteGenerator(routeData, trailingSlash); + assert.equal(generator(params), path); + }); + }); + + it('should throw an error when a dynamic parameter is missing', () => { + const generator = getRouteGenerator( + [[{ spread: false, content: 'foo', dynamic: true }]], + 'never', + ); + assert.throws(() => generator({}), TypeError); + }); +}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 6c5752503857..b32dadadad4e 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -693,9 +693,6 @@ importers: p-queue: specifier: ^8.0.1 version: 8.0.1 - path-to-regexp: - specifier: 6.2.2 - version: 6.2.2 preferred-pm: specifier: ^4.0.0 version: 4.0.0 @@ -9546,9 +9543,6 @@ packages: resolution: {integrity: sha512-7xTavNy5RQXnsjANvVvMkEjvloOinkAjv/Z6Ildz9v2RinZ4SBKTWFOVRbaF8p0vpHnyjV/UwNDdKuUv6M5qcA==} engines: {node: '>=16 || 14 >=14.17'} - path-to-regexp@6.2.2: - resolution: {integrity: sha512-GQX3SSMokngb36+whdpRXE+3f9V8UzyAorlYvOGx87ufGHehNTn5lCxrKtLyZ4Yl/wEKnNnr98ZzOwwDZV5ogw==} - path-type@4.0.0: resolution: {integrity: sha512-gDKb8aZMDeD/tZWs9P6+q0J9Mwkdl6xMV8TjnGP3qJVJ06bdMgkbBlLU8IdfOsIsFz2BW1rNVT3XuNEl8zPAvw==} engines: {node: '>=8'} @@ -15582,8 +15576,6 @@ snapshots: lru-cache: 10.2.0 minipass: 7.1.2 - path-to-regexp@6.2.2: {} - path-type@4.0.0: {} path-type@5.0.0: {}