diff options
| -rw-r--r-- | .changeset/six-shrimps-glow.md | 39 | ||||
| -rw-r--r-- | packages/astro/src/core/routing/manifest/create.ts | 99 | ||||
| -rw-r--r-- | packages/astro/src/prerender/utils.ts | 2 | ||||
| -rw-r--r-- | packages/astro/test/units/routing/manifest.test.js | 91 |
4 files changed, 196 insertions, 35 deletions
diff --git a/.changeset/six-shrimps-glow.md b/.changeset/six-shrimps-glow.md new file mode 100644 index 000000000..2d925bb8c --- /dev/null +++ b/.changeset/six-shrimps-glow.md @@ -0,0 +1,39 @@ +--- +"astro": patch +--- + +Fixes a regression in routing priority for index pages in rest parameter folders and dynamic sibling trees. + +Considering the following tree: +``` +src/pages/ +├── index.astro +├── static.astro +├── [dynamic_file].astro +├── [...rest_file].astro +├── blog/ +│ └── index.astro +├── [dynamic_folder]/ +│ ├── index.astro +│ ├── static.astro +│ └── [...rest].astro +└── [...rest_folder]/ + ├── index.astro + └── static.astro +``` + +The routes are sorted in this order: +``` +/src/pages/index.astro +/src/pages/blog/index.astro +/src/pages/static.astro +/src/pages/[dynamic_folder]/index.astro +/src/pages/[dynamic_file].astro +/src/pages/[dynamic_folder]/static.astro +/src/pages/[dynamic_folder]/[...rest].astro +/src/pages/[...rest_folder]/static.astro +/src/pages/[...rest_folder]/index.astro +/src/pages/[...rest_file]/index.astro +``` + +This allows for index files to be used as overrides to rest parameter routes on SSR when the rest parameter matching `undefined` is not desired. diff --git a/packages/astro/src/core/routing/manifest/create.ts b/packages/astro/src/core/routing/manifest/create.ts index 6ab297a5e..1b4909a2e 100644 --- a/packages/astro/src/core/routing/manifest/create.ts +++ b/packages/astro/src/core/routing/manifest/create.ts @@ -194,47 +194,94 @@ function isSemanticallyEqualSegment(segmentA: RoutePart[], segmentB: RoutePart[] * The definition of "alphabetically" is dependent on the default locale of the running system. */ function routeComparator(a: ManifestRouteData, b: ManifestRouteData) { - // For sorting purposes, an index route is considered to have one more segment than the URL it represents. - const aLength = a.isIndex ? a.segments.length + 1 : a.segments.length; - const bLength = b.isIndex ? b.segments.length + 1 : b.segments.length; + const commonLength = Math.min(a.segments.length, b.segments.length); + + for (let index = 0; index < commonLength; index++) { + const aSegment = a.segments[index]; + const bSegment = b.segments[index]; + + const aIsStatic = aSegment.every((part) => !part.dynamic && !part.spread); + const bIsStatic = bSegment.every((part) => !part.dynamic && !part.spread); + + if (aIsStatic && bIsStatic) { + // Both segments are static, they are sorted alphabetically if they are different + const aContent = aSegment.map((part) => part.content).join(''); + const bContent = bSegment.map((part) => part.content).join(''); + + if (aContent !== bContent) { + return aContent.localeCompare(bContent); + } + } - // Sort more specific routes before less specific routes - if (aLength !== bLength) { - return aLength > bLength ? -1 : 1; - } + // Sort static routes before dynamic routes + if (aIsStatic !== bIsStatic) { + return aIsStatic ? -1 : 1; + } - const aIsStatic = a.segments.every((segment) => - segment.every((part) => !part.dynamic && !part.spread) - ); - const bIsStatic = b.segments.every((segment) => - segment.every((part) => !part.dynamic && !part.spread) - ); + const aHasSpread = aSegment.some((part) => part.spread); + const bHasSpread = bSegment.some((part) => part.spread); - // Sort static routes before dynamic routes - if (aIsStatic !== bIsStatic) { - return aIsStatic ? -1 : 1; + // Sort dynamic routes with rest parameters after dynamic routes with single parameters + // (also after static, but that is already covered by the previous condition) + if (aHasSpread !== bHasSpread) { + return aHasSpread ? 1 : -1; + } } - const aHasSpread = a.segments.some((segment) => segment.some((part) => part.spread)); - const bHasSpread = b.segments.some((segment) => segment.some((part) => part.spread)); + // Special case to have `/[foo].astro` be equivalent to `/[foo]/index.astro` + // when compared against `/[foo]/[...rest].astro`. + if (Math.abs(a.segments.length - b.segments.length) === 1) { + const aEndsInRest = a.segments.at(-1)?.some((part) => part.spread); + const bEndsInRest = b.segments.at(-1)?.some((part) => part.spread); + + // Routes with rest parameters are less specific than their parent route. + // For example, `/foo/[...bar]` is sorted after `/foo`. - // Sort dynamic routes with rest parameters after dynamic routes with single parameters - // (also after static, but that is already covered by the previous condition) - if (aHasSpread !== bHasSpread) { - return aHasSpread ? 1 : -1; + if (a.segments.length > b.segments.length && !bEndsInRest) { + return 1; + } + + if (b.segments.length > a.segments.length && !aEndsInRest) { + return -1; + } } - // Sort prerendered routes before non-prerendered routes - if (a.prerender !== b.prerender) { - return a.prerender ? -1 : 1; + if (a.isIndex !== b.isIndex) { + // Index pages are lower priority than other static segments in the same prefix. + // They match the path up to their parent, but are more specific than the parent. + // For example: + // - `/foo/index.astro` is sorted before `/foo` + // - `/foo/index.astro` is sorted before `/foo/[bar].astro` + // - `/[...foo]/index.astro` is sorted after `/[...foo]/bar.astro` + + if (a.isIndex) { + const followingBSegment = b.segments.at(a.segments.length); + const followingBSegmentIsStatic = followingBSegment?.every((part) => !part.dynamic && !part.spread); + + return followingBSegmentIsStatic ? 1 : -1; + } + + const followingASegment = a.segments.at(b.segments.length); + const followingASegmentIsStatic = followingASegment?.every((part) => !part.dynamic && !part.spread); + + return followingASegmentIsStatic ? -1 : 1; } + // For sorting purposes, an index route is considered to have one more segment than the URL it represents. + const aLength = a.isIndex ? a.segments.length + 1 : a.segments.length; + const bLength = b.isIndex ? b.segments.length + 1 : b.segments.length; + + if (aLength !== bLength){ + // Routes are equal up to the smaller of the two lengths, so the longer route is more specific + return aLength > bLength ? -1 : 1; + } + // Sort endpoints before pages if ((a.type === 'endpoint') !== (b.type === 'endpoint')) { return a.type === 'endpoint' ? -1 : 1; } - // Sort alphabetically + // Both routes have segments with the same properties return a.route.localeCompare(b.route); } diff --git a/packages/astro/src/prerender/utils.ts b/packages/astro/src/prerender/utils.ts index b44435213..5cc627343 100644 --- a/packages/astro/src/prerender/utils.ts +++ b/packages/astro/src/prerender/utils.ts @@ -6,7 +6,7 @@ export function isServerLikeOutput(config: AstroConfig) { } export function getPrerenderDefault(config: AstroConfig) { - return config.output === 'hybrid'; + return config.output !== 'server'; } /** diff --git a/packages/astro/test/units/routing/manifest.test.js b/packages/astro/test/units/routing/manifest.test.js index 7bfdf6839..3ad1ab302 100644 --- a/packages/astro/test/units/routing/manifest.test.js +++ b/packages/astro/test/units/routing/manifest.test.js @@ -52,8 +52,8 @@ describe('routing - createRouteManifest', () => { it('endpoint routes are sorted before page routes', async () => { const fs = createFs( { - '/src/pages/contact-me.astro': `<h1>test</h1>`, - '/src/pages/sendContact.ts': `<h1>test</h1>`, + '/src/pages/[contact].astro': `<h1>test</h1>`, + '/src/pages/[contact].ts': `<h1>test</h1>`, }, root ); @@ -85,19 +85,19 @@ describe('routing - createRouteManifest', () => { expect(getManifestRoutes(manifest)).to.deep.equal([ { - route: '/api', - type: 'endpoint', + route: '/about', + type: 'page', }, { - route: '/sendcontact', + route: '/api', type: 'endpoint', }, { - route: '/about', - type: 'page', + route: '/[contact]', + type: 'endpoint', }, { - route: '/contact-me', + route: '/[contact]', type: 'page', }, ]); @@ -148,6 +148,81 @@ describe('routing - createRouteManifest', () => { ]); }); + it('route sorting respects the file tree', async () => { + const fs = createFs( + { + '/src/pages/[dynamic_folder]/static.astro': `<h1>test</h1>`, + '/src/pages/[dynamic_folder]/index.astro': `<h1>test</h1>`, + '/src/pages/[dynamic_folder]/[...rest].astro': `<h1>test</h1>`, + '/src/pages/[...rest]/static.astro': `<h1>test</h1>`, + '/src/pages/[...rest]/index.astro': `<h1>test</h1>`, + '/src/pages/blog/index.astro': `<h1>test</h1>`, + '/src/pages/[dynamic_file].astro': `<h1>test</h1>`, + '/src/pages/[...other].astro': `<h1>test</h1>`, + '/src/pages/static.astro': `<h1>test</h1>`, + '/src/pages/index.astro': `<h1>test</h1>`, + }, + root + ); + const settings = await createBasicSettings({ + root: fileURLToPath(root), + base: '/search', + trailingSlash: 'never', + experimental: { + globalRoutePriority: true, + }, + }); + + const manifest = createRouteManifest({ + cwd: fileURLToPath(root), + settings, + fsMod: fs, + }); + + expect(getManifestRoutes(manifest)).to.deep.equal([ + { + "route": "/", + "type": "page", + }, + { + "route": "/blog", + "type": "page", + }, + { + "route": "/static", + "type": "page", + }, + { + "route": "/[dynamic_folder]", + "type": "page", + }, + { + "route": "/[dynamic_file]", + "type": "page", + }, + { + "route": "/[dynamic_folder]/static", + "type": "page", + }, + { + "route": "/[dynamic_folder]/[...rest]", + "type": "page", + }, + { + "route": "/[...rest]/static", + "type": "page", + }, + { + "route": "/[...rest]", + "type": "page", + }, + { + "route": "/[...other]", + "type": "page", + }, + ]); + }); + it('injected routes are sorted in legacy mode above filesystem routes', async () => { const fs = createFs( { |