Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fix-prerender-dynamic-route-fallthrough.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

Fixes an issue where on-demand (SSR) dynamic routes would return 404 when a prerendered dynamic route with the same URL pattern was sorted first alphabetically. In production builds with `@astrojs/node` adapter, if `[a_prebuild].astro` (prerender=true) came before `[b_ssr].astro` alphabetically, requests to URLs not in the prerendered route's static paths would 404 instead of falling through to the SSR route. The fix adds fallthrough logic so that when a prerendered dynamic route matches but can't serve the request, Astro tries subsequent matching routes.
10 changes: 9 additions & 1 deletion packages/astro/src/core/app/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -296,8 +296,16 @@ export abstract class BaseApp<P extends Pipeline = AppPipeline> {
if (allowPrerenderedRoutes) {
return routeData;
}
// missing routes fall-through, pre rendered are handled by static layer
// Prerendered routes are served as static files by the hosting layer.
// When the first match is a prerendered *dynamic* route, try to find
// a non-prerendered route that can serve this path. Dynamic prerendered
// routes only cover their specific static paths, so an SSR route with
// the same pattern should handle all other URLs.
if (routeData.prerender) {
if (routeData.params.length > 0) {
const allMatches = this.pipeline.matchAllRoutes(decodeURI(pathname));
return allMatches.find((r) => !r.prerender);
}
return undefined;
}
return routeData;
Expand Down
10 changes: 10 additions & 0 deletions packages/astro/src/core/base-pipeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,16 @@ export abstract class Pipeline {
return match.route;
}

/**
* Returns all routes matching the given pathname, in priority order.
* Used when the first match cannot serve the request (e.g. a
* prerendered dynamic route that doesn't cover this specific path)
* and the caller needs to try subsequent matches.
*/
matchAllRoutes(pathname: string): RouteData[] {
return this.#router.matchAll(pathname, { allowWithoutBase: true });
}

/**
* Rebuilds the internal router after routes have been added or
* removed (e.g. by the dev server on HMR).
Expand Down
11 changes: 10 additions & 1 deletion packages/astro/src/core/fetch/fetch-state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -792,8 +792,17 @@ export class FetchState implements AstroFetchState {
const matched = pipeline.matchRoute(this.pathname);
// In production SSR, prerendered routes are served as static files
// by the hosting layer and should not be rendered by the app.
// When the first match is a prerendered *dynamic* route, try to find
// a non-prerendered route that can serve this path. Dynamic prerendered
// routes only cover their specific static paths, so an SSR route with
// the same pattern should handle all other URLs.
if (matched && matched.prerender && pipeline.manifest.serverLike) {
this.routeData = undefined;
if (matched.params.length > 0) {
const allMatches = pipeline.matchAllRoutes(this.pathname);
this.routeData = allMatches.find((r) => !r.prerender);
} else {
this.routeData = undefined;
}
} else {
this.routeData = matched;
}
Expand Down
35 changes: 35 additions & 0 deletions packages/astro/src/core/routing/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,41 @@ export class Router {
const params = getParams(route, pathname);
return { type: 'match', route, params, pathname };
}

/**
* Returns all routes that match the given pathname, in priority order.
* Used when the first match (e.g. a prerendered route) cannot serve
* the request and subsequent matches need to be tried.
*/
public matchAll(
inputPathname: string,
{ allowWithoutBase = false }: { allowWithoutBase?: boolean } = {},
): RouteData[] {
const normalized = getRedirectForPathname(inputPathname);
if (normalized.redirect) {
return [];
}

const baseResult = stripBase(
normalized.pathname,
this.#base,
this.#baseWithoutTrailingSlash,
this.#trailingSlash,
);
if (!baseResult && !allowWithoutBase) {
return [];
}

let pathname = baseResult ?? normalized.pathname;
if (this.#buildFormat === 'file') {
pathname = normalizeFileFormatPathname(pathname);
}

return this.#routes.filter((candidate) => {
if (candidate.pattern.test(pathname)) return true;
return candidate.fallbackRoutes.some((fallbackRoute) => fallbackRoute.pattern.test(pathname));
});
}
}

/**
Expand Down
29 changes: 28 additions & 1 deletion packages/astro/test/units/fetch/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ import {
i18n,
} from '../../../dist/core/fetch/index.js';
import { createComponent, render } from '../../../dist/runtime/server/index.js';
import { createEndpoint, createPage, createRedirect, createTestApp } from '../mocks.ts';
import { createEndpoint, createPage, createRedirect, createTestApp, createRouteData } from '../mocks.ts';
import { dynamicPart } from '../routing/test-helpers.ts';

/** A simple page component that renders `<h1>Hello</h1>`. */
const simplePage = createComponent((_result: any, _props: any, _slots: any) => {
Expand Down Expand Up @@ -52,6 +53,32 @@ describe('FetchState (astro/fetch)', () => {
assert.ok(state.routeData, 'routeData should be set by the constructor');
assert.equal(state.routeData!.route, '/');
});

it('falls through to SSR route when prerendered dynamic route matches first', () => {
// Regression test for #16834: when a prerendered dynamic route like
// [a_prebuild].astro matches before an SSR dynamic route like [b_ssr].astro,
// the SSR route should be used instead of returning 404.
const prerenderPage = createPage(simplePage, {
route: '/[a_prebuild]',
prerender: true,
pathname: undefined,
segments: [[dynamicPart('a_prebuild')]],
});
const ssrPage = createPage(simplePage, {
route: '/[b_ssr]',
prerender: false,
pathname: undefined,
segments: [[dynamicPart('b_ssr')]],
});

const app = createTestApp([prerenderPage, ssrPage], { serverLike: true });
const request = stampApp(new Request('http://example.com/foobar'), app);
const state = new FetchState(request);

assert.ok(state.routeData, 'routeData should be set to the SSR route');
assert.equal(state.routeData!.route, '/[b_ssr]');
assert.equal(state.routeData!.prerender, false);
});
});

// #endregion
Expand Down
82 changes: 82 additions & 0 deletions packages/astro/test/units/routing/router-match.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import assert from 'node:assert/strict';
import { describe, it } from 'node:test';
import { Router } from '../../../dist/core/routing/router.js';
import type { RouteData } from '../../../dist/types/public/internal.js';
import { dynamicPart, makeRoute, spreadPart, staticPart } from './test-helpers.ts';

describe('Router.match', () => {
Expand Down Expand Up @@ -427,3 +428,84 @@ describe('Router.match', () => {
}
});
});

describe('Router.matchAll', () => {
it('returns all matching routes in priority order', () => {
const trailingSlash = 'ignore' as const;
const prerenderRoute = makeRoute({
segments: [[dynamicPart('a_prebuild')]],
trailingSlash,
route: '/[a_prebuild]',
pathname: undefined,
prerender: true,
});
const ssrRoute = makeRoute({
segments: [[dynamicPart('b_ssr')]],
trailingSlash,
route: '/[b_ssr]',
pathname: undefined,
prerender: false,
});

const router = new Router([prerenderRoute, ssrRoute], {
base: '/',
trailingSlash,
buildFormat: 'directory',
});

const allMatches = router.matchAll('/foobar');
assert.equal(allMatches.length, 2, 'should return both matching routes');
// Both have the same pattern, order determined by routeComparator
const routeNames = allMatches.map((r: RouteData) => r.route);
assert.ok(routeNames.includes('/[a_prebuild]'));
assert.ok(routeNames.includes('/[b_ssr]'));
});

it('returns empty array when no routes match', () => {
const trailingSlash = 'ignore' as const;
const route = makeRoute({
segments: [[staticPart('about')]],
trailingSlash,
route: '/about',
pathname: '/about',
});

const router = new Router([route], {
base: '/',
trailingSlash,
buildFormat: 'directory',
});

const allMatches = router.matchAll('/nonexistent');
assert.equal(allMatches.length, 0);
});

it('allows finding non-prerendered fallback when prerendered route matches first', () => {
const trailingSlash = 'ignore' as const;
const prerenderRoute = makeRoute({
segments: [[dynamicPart('a_prebuild')]],
trailingSlash,
route: '/[a_prebuild]',
pathname: undefined,
prerender: true,
});
const ssrRoute = makeRoute({
segments: [[dynamicPart('b_ssr')]],
trailingSlash,
route: '/[b_ssr]',
pathname: undefined,
prerender: false,
});

const router = new Router([prerenderRoute, ssrRoute], {
base: '/',
trailingSlash,
buildFormat: 'directory',
});

const allMatches = router.matchAll('/foobar');
const ssrMatch = allMatches.find((r: RouteData) => !r.prerender);
assert.ok(ssrMatch, 'should find a non-prerendered route');
assert.equal(ssrMatch!.route, '/[b_ssr]');
});
});
Loading