From 37e57c56d10cac56fd8b338f4a0a9975266f3dba Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Wed, 30 Jul 2025 11:32:05 +0100 Subject: [PATCH 1/3] fix(routing): improve error handling --- .changeset/witty-wasps-tie.md | 13 +++++ packages/astro/src/core/errors/errors-data.ts | 19 +++++++ .../astro/src/core/routing/manifest/create.ts | 57 ++++++++++++++++++- packages/astro/test/redirects.test.js | 18 ++++++ 4 files changed, 106 insertions(+), 1 deletion(-) create mode 100644 .changeset/witty-wasps-tie.md diff --git a/.changeset/witty-wasps-tie.md b/.changeset/witty-wasps-tie.md new file mode 100644 index 000000000000..d63081bb1598 --- /dev/null +++ b/.changeset/witty-wasps-tie.md @@ -0,0 +1,13 @@ +--- +'astro': patch +--- + +Improved the error message when a redirect can't be mapped to its destination. For example, the following redirect +will throw a new error because `/category/[categories]/` contains one dynamic segment, while `/category/[categories]/[page]` has two dynamic segments, +and Astro doesn't know how map the parameters: + +```js +export default defineConfig({ + "/category/[categories]": "/category/[categories]/[page]" +}) +``` diff --git a/packages/astro/src/core/errors/errors-data.ts b/packages/astro/src/core/errors/errors-data.ts index e8348355da46..eaf4937da454 100644 --- a/packages/astro/src/core/errors/errors-data.ts +++ b/packages/astro/src/core/errors/errors-data.ts @@ -981,6 +981,25 @@ export const UnsupportedExternalRedirect = { hint: 'An external redirect must start with http or https, and must be a valid URL.', } satisfies ErrorData; +/** + * @docs + * @see + * - [Astro.redirect](https://docs.astro.build/en/reference/api-reference/#redirect) + * @description + * A redirect can't be mapped when origin and destination have different segments. + */ +export const RedirectMappingMismatch = { + name: 'RedirectMappingMismatch', + title: "A redirect can't be mapped when origin and destination have different segments", + message: ( + fromRoute: string, + toRoute: string, + fromDynamicSegments: number, + toDynamicSegments: number, + ) => + `The number of dynamic segments don't match. The route ${fromRoute} has ${fromDynamicSegments} segments, while ${toRoute} has ${toDynamicSegments} segments.`, +} satisfies ErrorData; + /** * @docs * @see diff --git a/packages/astro/src/core/routing/manifest/create.ts b/packages/astro/src/core/routing/manifest/create.ts index ad093f04b73c..8ed6120df1b9 100644 --- a/packages/astro/src/core/routing/manifest/create.ts +++ b/packages/astro/src/core/routing/manifest/create.ts @@ -14,6 +14,7 @@ import type { RouteData, RoutePart } from '../../../types/public/internal.js'; import { SUPPORTED_MARKDOWN_FILE_EXTENSIONS } from '../../constants.js'; import { MissingIndexForInternationalization, + RedirectMappingMismatch, UnsupportedExternalRedirect, } from '../../errors/errors-data.js'; import { AstroError } from '../../errors/index.js'; @@ -364,6 +365,12 @@ function createRedirectRoutes( }); } + const redirectRoute = routeMap.get(destination); + + if (redirectRoute) { + validateRouteMapping(segments, from, redirectRoute.segments ?? [], redirectRoute.route ?? ''); + } + routes.push({ type: 'redirect', // For backwards compatibility, a redirect is never considered an index route. @@ -377,7 +384,7 @@ function createRedirectRoutes( pathname: pathname || void 0, prerender: getPrerenderDefault(config), redirect: to, - redirectRoute: routeMap.get(destination), + redirectRoute, fallbackRoutes: [], distURL: [], origin: 'project', @@ -387,6 +394,54 @@ function createRedirectRoutes( return routes; } +/** + * Checks whether `a` can be mapped to `b`. If the requirements don't match, + * an error is thrown. + * @param fromSegments + * @param fromRoute + * @param toSegments + * @param toRoute + */ +function validateRouteMapping( + fromSegments: RoutePart[][], + fromRoute: string, + toSegments: RoutePart[][], + toRoute: string, +) { + const fromHasSpread = fromSegments.some((routePart) => routePart.some((part) => part.spread)); + const toHasSpread = toSegments.some((routePart) => routePart.some((part) => part.spread)); + + if (fromHasSpread || toHasSpread) { + return; + } + + const fromDynamicSegments = fromSegments.reduce((dynamicSegments, routePart) => { + if (routePart.some((part) => part.dynamic || part.spread)) { + dynamicSegments += 1; + } + return dynamicSegments; + }, 0); + + const toDynamicSegments = toSegments.reduce((dynamicSegments, routePart) => { + if (routePart.some((part) => part.dynamic || part.spread)) { + dynamicSegments += 1; + } + return dynamicSegments; + }, 0); + + if (fromDynamicSegments != toDynamicSegments) { + throw new AstroError({ + ...RedirectMappingMismatch, + message: RedirectMappingMismatch.message( + fromRoute, + toRoute, + fromDynamicSegments, + toDynamicSegments, + ), + }); + } +} + /** * Checks whether a route segment is static. */ diff --git a/packages/astro/test/redirects.test.js b/packages/astro/test/redirects.test.js index 276924dfda14..70803285f95b 100644 --- a/packages/astro/test/redirects.test.js +++ b/packages/astro/test/redirects.test.js @@ -345,4 +345,22 @@ describe('Astro.redirect', () => { assert.equal(secretHtml.includes('url=/login'), true); }); }); + + describe('should fail for redirects that cannot be mapped ', () => { + it('should fail for redirects that cannot be mapped', async () => { + fixture = await loadFixture({ + root: './fixtures/redirects/', + output: 'static', + redirects: { + '/old/[category]/': '/old/[category]/[slug]', + }, + }); + try { + await fixture.build(); + assert.fail('Expected build to fail'); + } catch { + assert.ok(true); + } + }); + }); }); From 9bdc5ad94e3fbd998aa784ea45ded1ef0dfd9cdc Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Wed, 30 Jul 2025 15:49:36 +0100 Subject: [PATCH 2/3] Update .changeset/witty-wasps-tie.md Co-authored-by: Matt Kane --- .changeset/witty-wasps-tie.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/witty-wasps-tie.md b/.changeset/witty-wasps-tie.md index d63081bb1598..ddb7c987f715 100644 --- a/.changeset/witty-wasps-tie.md +++ b/.changeset/witty-wasps-tie.md @@ -8,6 +8,6 @@ and Astro doesn't know how map the parameters: ```js export default defineConfig({ - "/category/[categories]": "/category/[categories]/[page]" + redirects: { "/category/[categories]": "/category/[categories]/[page]" } }) ``` From 92632a6fb8c0497c5aed29a2ec9f4040ede2a29a Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Thu, 31 Jul 2025 14:19:15 +0100 Subject: [PATCH 3/3] chore: add proper test --- .../redirects/src/pages/old/[category]/[slug].astro | 12 ++++++++++++ .../redirects/src/pages/old/[category]/index.astro | 8 ++++++++ packages/astro/test/redirects.test.js | 8 ++++++-- 3 files changed, 26 insertions(+), 2 deletions(-) create mode 100644 packages/astro/test/fixtures/redirects/src/pages/old/[category]/[slug].astro create mode 100644 packages/astro/test/fixtures/redirects/src/pages/old/[category]/index.astro diff --git a/packages/astro/test/fixtures/redirects/src/pages/old/[category]/[slug].astro b/packages/astro/test/fixtures/redirects/src/pages/old/[category]/[slug].astro new file mode 100644 index 000000000000..f9162ed675b4 --- /dev/null +++ b/packages/astro/test/fixtures/redirects/src/pages/old/[category]/[slug].astro @@ -0,0 +1,12 @@ +--- + +export function getStaticPaths() { + return [{ params: { category: 'test', slug: 'test' } }] +} + +const { slug, category } = Astro.params +--- + +

+ {slug} and {category} +

diff --git a/packages/astro/test/fixtures/redirects/src/pages/old/[category]/index.astro b/packages/astro/test/fixtures/redirects/src/pages/old/[category]/index.astro new file mode 100644 index 000000000000..70d870acd8df --- /dev/null +++ b/packages/astro/test/fixtures/redirects/src/pages/old/[category]/index.astro @@ -0,0 +1,8 @@ +--- + +export function getStaticPaths() { + return [{ params: { category: 'test' } }] +} +--- + +

Redirect me!

diff --git a/packages/astro/test/redirects.test.js b/packages/astro/test/redirects.test.js index 70803285f95b..bfe13245fd21 100644 --- a/packages/astro/test/redirects.test.js +++ b/packages/astro/test/redirects.test.js @@ -352,13 +352,17 @@ describe('Astro.redirect', () => { root: './fixtures/redirects/', output: 'static', redirects: { - '/old/[category]/': '/old/[category]/[slug]', + '/old/[category]/1': '/old/[category]/[slug]', }, }); try { await fixture.build(); assert.fail('Expected build to fail'); - } catch { + } catch (e) { + assert.equal( + e.message, + "The number of dynamic segments don't match. The route /old/[category]/1 has 1 segments, while /old/[category]/[slug] has 2 segments.", + ); assert.ok(true); } });