Skip to content

Commit

Permalink
Fix dynamic prerender conflict (#10298)
Browse files Browse the repository at this point in the history
* Reproduce issues

* Handle inconsistency between static, dynamic and rest routes

* Add extra test cases

* Add changeset

* Revert unrelated changes

* Update lockfile
  • Loading branch information
Fryuni authored Mar 4, 2024
1 parent c99bbd0 commit 819d20a
Show file tree
Hide file tree
Showing 17 changed files with 322 additions and 2 deletions.
5 changes: 5 additions & 0 deletions .changeset/strange-forks-exist.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"astro": patch
---

Fix an incorrect conflict resolution between pages generated from static routes and rest parameters
2 changes: 1 addition & 1 deletion packages/astro/src/core/build/generate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ async function getPathsForRoute(
let paths: Array<string> = [];
if (route.pathname) {
paths.push(route.pathname);
builtPaths.add(route.pathname);
builtPaths.add(removeTrailingForwardSlash(route.pathname));
} else {
const staticPaths = await callGetStaticPaths({
mod,
Expand Down
10 changes: 9 additions & 1 deletion packages/astro/src/core/routing/manifest/generator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,5 +37,13 @@ export function getRouteGenerator(
trailing = '/';
}
const toPath = compile(template + trailing);
return toPath;
return (params: object): string => {
const path = toPath(params);

// 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.
return path || '/';
};
}
59 changes: 59 additions & 0 deletions packages/astro/test/dynamic-route-collision.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import assert from 'node:assert/strict';
import { before, describe, it } from 'node:test';
import * as cheerio from 'cheerio';
import { loadFixture } from './test-utils.js';

describe('Dynamic route collision', () => {
/** @type {import('./test-utils').Fixture} */
let fixture;

before(async () => {
fixture = await loadFixture({
root: './fixtures/dynamic-route-collision',
});

await fixture.build().catch(console.log);
});

it('Builds a static route when in conflict with a dynamic route', async () => {
const html = await fixture.readFile('/about/index.html');
const $ = cheerio.load(html);
assert.equal($('h1').text(), 'Static About');
});

it('Builds a static route when in conflict with a spread route', async () => {
const html = await fixture.readFile('/who/index.html');
const $ = cheerio.load(html);
assert.equal($('h1').text(), 'Static Who We Are');
});

it('Builds a static nested index when in conflict with a spread route', async () => {
const html = await fixture.readFile('/tags/index.html');
const $ = cheerio.load(html);
assert.equal($('h1').text(), 'Static Tags Index');
});

it('Builds a static root index when in conflict with a spread route', async () => {
const html = await fixture.readFile('/index.html');
const $ = cheerio.load(html);
assert.equal($('h1').text(), 'Static Index');
});

it('Builds a static index a nested when in conflict with a dynamic+spread route', async () => {
const html = await fixture.readFile('/en/index.html');
const $ = cheerio.load(html);
assert.equal($('h1').text(), 'Dynamic-only Localized Index');
});

it('Builds a dynamic route when in conflict with a spread route', async () => {
const html = await fixture.readFile('/blog/index.html');
const $ = cheerio.load(html);
assert.equal($('h1').text(), 'Dynamic Blog');
});

it('Builds the highest priority route out of two conflicting dynamic routes', async () => {
const html = await fixture.readFile('/order/index.html');
const $ = cheerio.load(html);
assert.equal($('h1').text(), 'Order from A');
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"name": "@test/dynamic-route-collision",
"version": "0.0.0",
"private": true,
"dependencies": {
"astro": "workspace:*"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
---
export async function getStaticPaths() {
const pages = [
{
slug: undefined,
title: 'Rest Index',
},
{
slug: 'blog',
title: 'Rest Blog',
},
{
slug: 'who',
title: 'Rest Who We Are',
},
];
return pages.map(({ slug, title }) => {
return {
params: { slug },
props: { title },
};
});
}
const { title } = Astro.props;
---

<html>
<head>
<title>{title}</title>
</head>
<body>
<h1>{title}</h1>
</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
---
export async function getStaticPaths() {
const pages = [
{
page: 'order',
title: 'Order from A',
},
];
return pages.map(({ page, title }) => {
return {
params: { 'aOrder': page },
props: { title },
};
});
}
const { title } = Astro.props;
---

<html>
<head>
<title>{title}</title>
</head>
<body>
<h1>{title}</h1>
</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
---
export async function getStaticPaths() {
const pages = [
{
page: 'order',
title: 'Order from B',
},
];
return pages.map(({ page, title }) => {
return {
params: { 'bOrder': page },
props: { title },
};
});
}
const { title } = Astro.props;
---

<html>
<head>
<title>{title}</title>
</head>
<body>
<h1>{title}</h1>
</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
---
export async function getStaticPaths() {
const pages = [
{
locale: 'en',
page: undefined,
title: 'Dynamic+Rest Localized Index',
},
];
return pages.map(({ page, locale, title }) => {
return {
params: { page, locale },
props: { title },
};
});
}
const { title } = Astro.props;
---

<html>
<head>
<title>{title}</title>
</head>
<body>
<h1>{title}</h1>
</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
---
export async function getStaticPaths() {
const pages = [
{
locale: 'en',
title: 'Dynamic-only Localized Index',
},
];
return pages.map(({ page, locale, title }) => {
return {
params: { page, locale },
props: { title },
};
});
}
const { title } = Astro.props;
---

<html>
<head>
<title>{title}</title>
</head>
<body>
<h1>{title}</h1>
</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
---
export async function getStaticPaths() {
const pages = [
{
page: 'about',
title: 'Dynamic About',
},
{
page: 'blog',
title: 'Dynamic Blog',
},
];
return pages.map(({ page, title }) => {
return {
params: { page },
props: { title },
};
});
}
const { title } = Astro.props;
---

<html>
<head>
<title>{title}</title>
</head>
<body>
<h1>{title}</h1>
</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<html>
<head>
<title>Static About Page</title>
</head>
<body>
<h1>Static About</h1>
</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<html>
<head>
<title>Static Index Page</title>
</head>
<body>
<h1>Static Index</h1>
</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
---
export async function getStaticPaths() {
const pages = [
{
page: undefined,
title: 'Rest Tag Index',
},
];
return pages.map(({ page, title }) => {
return {
params: { tag: page },
props: { title },
};
});
}
const { title } = Astro.props;
---

<html>
<head>
<title>{title}</title>
</head>
<body>
<h1>{title}</h1>
</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<html>
<head>
<title>Static Tags Index</title>
</head>
<body>
<h1>Static Tags Index</h1>
</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<html>
<head>
<title>Static Who We Are</title>
</head>
<body>
<h1>Static Who We Are</h1>
</body>
</html>
6 changes: 6 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 819d20a

Please sign in to comment.