Infrastructure/astro
0
Fork 0
mirror of https://github.com/withastro/astro.git synced 2024-12-16 21:46:22 -05:00
Code Issues Activity

Fix regression on dynamic sibling trees and index inside rest parameter folders (#9786)

Browse source 
* fix: Fix regression on dynamic sibling trees and index inside rest parameter folders

* Add extra test scenarios

* Make `/[foo].astro` also win over `/[foo]/[...rest].astro`

* Make `/[foo].astro` also win over `/[foo]/[...rest].astro`

* Update tests

* Remove commented out code

* Update .changeset/six-shrimps-glow.md

* Fix sorting cycle

---------

Co-authored-by: Nate Moore <natemoo-re@users.noreply.github.com>
This commit is contained in:
Luiz Ferraz 2024-01-23 21:23:10 -03:00 committed by GitHub
parent 659087be1a
commit 5b29550996
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 204 additions and 43 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

39
.changeset/six-shrimps-glow.md Normal file
View file

@ -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.

109
packages/astro/src/core/routing/manifest/create.ts
View file

@ -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) {
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 static routes before dynamic routes
if (aIsStatic !== bIsStatic) {
return aIsStatic ? -1 : 1;
}
const aHasSpread = aSegment.some((part) => part.spread);
const bHasSpread = bSegment.some((part) => part.spread);
// 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;
}
}
// 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`.
if (a.segments.length > b.segments.length && !bEndsInRest) {
return 1;
}
if (b.segments.length > a.segments.length && !aEndsInRest) {
return -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;
// Sort more specific routes before less specific routes
if (aLength !== bLength) {
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;
}
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)
);
// Sort static routes before dynamic routes
if (aIsStatic !== bIsStatic) {
return aIsStatic ? -1 : 1;
}
const aHasSpread = a.segments.some((segment) => segment.some((part) => part.spread));
const bHasSpread = b.segments.some((segment) => segment.some((part) => part.spread));
// 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;
}
// Sort prerendered routes before non-prerendered routes
if (a.prerender !== b.prerender) {
return a.prerender ? -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);
}

2
packages/astro/src/prerender/utils.ts
View file

@ -6,7 +6,7 @@ export function isServerLikeOutput(config: AstroConfig) {
}
export function getPrerenderDefault(config: AstroConfig) {
return config.output === 'hybrid';
return config.output !== 'server';
}
/**

97
packages/astro/test/units/routing/manifest.test.js
View file

@ -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
);
@ -84,20 +84,20 @@ describe('routing - createRouteManifest', () => {
});
expect(getManifestRoutes(manifest)).to.deep.equal([
{
route: '/api',
type: 'endpoint',
},
{
route: '/sendcontact',
type: 'endpoint',
},
{
route: '/about',
type: 'page',
},
{
route: '/contact-me',
route: '/api',
type: 'endpoint',
},
{
route: '/[contact]',
type: 'endpoint',
},
{
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(
{