Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(routing): partially dynamic segments are being truncated #10379

Merged
merged 6 commits into from
Mar 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/five-colts-rescue.md
@@ -0,0 +1,5 @@
---
"astro": patch
---

Fixes a routing issue with partially truncated dynamic segments.
20 changes: 11 additions & 9 deletions packages/astro/src/core/routing/manifest/create.ts
Expand Up @@ -401,9 +401,7 @@ function createFileBasedRoutes(
const pathname = segments.every((segment) => segment.length === 1 && !segment[0].dynamic)
? `/${segments.map((segment) => segment[0].content).join('/')}`
: null;
const route = `/${segments
.map(([{ dynamic, content }]) => (dynamic ? `[${content}]` : content))
.join('/')}`.toLowerCase();
const route = joinSegments(segments);
routes.push({
route,
isIndex: item.isIndex,
Expand Down Expand Up @@ -478,9 +476,7 @@ function createInjectedRoutes({ settings, cwd }: CreateRouteManifestParams): Pri
.flat()
.filter((p) => p.dynamic)
.map((p) => p.content);
const route = `/${segments
.map(([{ dynamic, content }]) => (dynamic ? `[${content}]` : content))
.join('/')}`.toLowerCase();
const route = joinSegments(segments);

routes[priority].push({
type,
Expand Down Expand Up @@ -536,9 +532,7 @@ function createRedirectRoutes(
.flat()
.filter((p) => p.dynamic)
.map((p) => p.content);
const route = `/${segments
.map(([{ dynamic, content }]) => (dynamic ? `[${content}]` : content))
.join('/')}`.toLowerCase();
const route = joinSegments(segments);

let destination: string;
if (typeof to === 'string') {
Expand Down Expand Up @@ -899,3 +893,11 @@ function computeRoutePriority(config: AstroConfig): RoutePriorityOverride {
}
return 'legacy';
}

function joinSegments(segments: RoutePart[][]): string {
const arr = segments.map((segment) => {
return segment.map((rp) => (rp.dynamic ? `[${rp.content}]` : rp.content)).join('');
});

return `/${arr.join('/')}`.toLowerCase();
}
48 changes: 46 additions & 2 deletions packages/astro/test/units/routing/manifest.test.js
Expand Up @@ -144,8 +144,8 @@ describe('routing - createRouteManifest', () => {

assertRouteRelations(getManifestRoutes(manifest), [
['/', '/[...rest]'],
['/static', '/static-'],
['/static-', '/[dynamic]'],
1574242600 marked this conversation as resolved.
Show resolved Hide resolved
['/static', '/static-[dynamic]'],
['/static-[dynamic]', '/[dynamic]'],
['/static', '/[dynamic]'],
['/static', '/[...rest]'],
['/[dynamic]', '/[...rest]'],
Expand Down Expand Up @@ -567,4 +567,48 @@ describe('routing - createRouteManifest', () => {
},
]);
});

it('should concatenate each part of the segment. issues#10122', async () => {
const fs = createFs(
{
'/src/pages/a-[b].astro': `<h1>test</h1>`,
'/src/pages/blog/a-[b].233.ts': ``,
},
root
);

const settings = await createBasicSettings({
root: fileURLToPath(root),
output: 'server',
base: '/search',
trailingSlash: 'never',
redirects: {
'/posts/a-[b].233': '/blog/a-[b].233',
},
experimental: {
globalRoutePriority: true,
},
});

settings.injectedRoutes = [
{
pattern: '/[c]-d',
entrypoint: '@lib/legacy/dynamic.astro',
priority: 'normal',
},
];

const manifest = createRouteManifest({
cwd: fileURLToPath(root),
settings,
fsMod: fs,
});

assert.deepEqual(getManifestRoutes(manifest), [
{ type: 'endpoint', route: '/blog/a-[b].233' },
{ type: 'redirect', route: '/posts/a-[b].233' },
{ type: 'page', route: '/[c]-d' },
{ type: 'page', route: '/a-[b]' },
]);
});
});