From 8ce9fffd44b0740621178d61fb1425bf4155c2d7 Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Thu, 7 Mar 2024 14:48:39 +0000 Subject: [PATCH] fix(routing): regression on partial dynamic routes (#10355) * fix(routing): regression on partial dynamic routes * Update .changeset/eight-spoons-nail.md Co-authored-by: Bjorn Lu --------- Co-authored-by: Bjorn Lu --- .changeset/eight-spoons-nail.md | 5 +++++ .../astro/src/core/routing/manifest/create.ts | 20 +++++++++++++++---- .../astro/test/units/routing/manifest.test.js | 3 +++ 3 files changed, 24 insertions(+), 4 deletions(-) create mode 100644 .changeset/eight-spoons-nail.md diff --git a/.changeset/eight-spoons-nail.md b/.changeset/eight-spoons-nail.md new file mode 100644 index 0000000000..7d0304e2c1 --- /dev/null +++ b/.changeset/eight-spoons-nail.md @@ -0,0 +1,5 @@ +--- +"astro": patch +--- + +Fixes a regression where full dynamic routes were prioritized over partial dynamic routes. Now a route like `food-[name].astro` is matched **before** `[name].astro`. diff --git a/packages/astro/src/core/routing/manifest/create.ts b/packages/astro/src/core/routing/manifest/create.ts index c0560a5e73..00b87a049d 100644 --- a/packages/astro/src/core/routing/manifest/create.ts +++ b/packages/astro/src/core/routing/manifest/create.ts @@ -42,11 +42,14 @@ function countOccurrences(needle: string, haystack: string) { return count; } +// Disable eslint as we're not sure how to improve this regex yet +// eslint-disable-next-line regexp/no-super-linear-backtracking +const ROUTE_DYNAMIC_SPLIT = /\[(.+?\(.+?\)|.+?)\]/; +const ROUTE_SPREAD = /^\.{3}.+$/; + function getParts(part: string, file: string) { const result: RoutePart[] = []; - // Disable eslint as we're not sure how to improve this regex yet - // eslint-disable-next-line regexp/no-super-linear-backtracking - part.split(/\[(.+?\(.+?\)|.+?)\]/).map((str, i) => { + part.split(ROUTE_DYNAMIC_SPLIT).map((str, i) => { if (!str) return; const dynamic = i % 2 === 1; @@ -59,7 +62,7 @@ function getParts(part: string, file: string) { result.push({ content, dynamic, - spread: dynamic && /^\.{3}.+$/.test(content), + spread: dynamic && ROUTE_SPREAD.test(content), }); }); @@ -218,6 +221,15 @@ function routeComparator(a: RouteData, b: RouteData) { return aIsStatic ? -1 : 1; } + const aAllDynamic = aSegment.every((part) => part.dynamic); + const bAllDynamic = bSegment.every((part) => part.dynamic); + + // Some route might have partial dynamic segments, e.g. game-[title].astro + // These routes should have higher priority against route that have **only** dynamic segments, e.g. [title].astro + if (aAllDynamic !== bAllDynamic) { + return aAllDynamic ? 1 : -1; + } + const aHasSpread = aSegment.some((part) => part.spread); const bHasSpread = bSegment.some((part) => part.spread); diff --git a/packages/astro/test/units/routing/manifest.test.js b/packages/astro/test/units/routing/manifest.test.js index 2e7ff9e7cf..91816d1bc6 100644 --- a/packages/astro/test/units/routing/manifest.test.js +++ b/packages/astro/test/units/routing/manifest.test.js @@ -122,6 +122,7 @@ describe('routing - createRouteManifest', () => { '/src/pages/[dynamic].astro': `

test

`, '/src/pages/[...rest].astro': `

test

`, '/src/pages/static.astro': `

test

`, + '/src/pages/static-[dynamic].astro': `

test

`, '/src/pages/index.astro': `

test

`, }, root @@ -143,6 +144,8 @@ describe('routing - createRouteManifest', () => { assertRouteRelations(getManifestRoutes(manifest), [ ['/', '/[...rest]'], + ['/static', '/static-'], + ['/static-', '/[dynamic]'], ['/static', '/[dynamic]'], ['/static', '/[...rest]'], ['/[dynamic]', '/[...rest]'],