summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGravatar Happydev <81974850+MoustaphaDev@users.noreply.github.com> 2023-05-30 14:12:16 +0000
committerGravatar GitHub <noreply@github.com> 2023-05-30 14:12:16 +0000
commitee2aca80a71afe843af943b11966fcf77f556cfb (patch)
tree8ed211f738eb407cce8680d3b08154d913483f06
parente20a3b20b7f5f743c25533e81e633ad089cb2564 (diff)
downloadastro-ee2aca80a71afe843af943b11966fcf77f556cfb.tar.gz
astro-ee2aca80a71afe843af943b11966fcf77f556cfb.tar.zst
astro-ee2aca80a71afe843af943b11966fcf77f556cfb.zip
fix: prioritize dynamic prerendered routes over dynamic server routes (#7235)
* test: add unit tests * fix: prioritize prerendered routes * chore: fix test * add comment * test: try avoiding race condition * chore: changeset * try avoiding race conditions attempt #2 * try avoiding race conditions (attempt 3) * final fix hopefuly * tet: add more tests * sort conflicting dynamic routes aplhabetically * test: fix test
-rw-r--r--.changeset/hungry-peaches-speak.md5
-rw-r--r--packages/astro/src/core/render/dev/index.ts3
-rw-r--r--packages/astro/src/core/routing/manifest/create.ts1
-rw-r--r--packages/astro/src/prerender/routing.ts67
-rw-r--r--packages/astro/src/vite-plugin-astro-server/route.ts22
-rw-r--r--packages/astro/test/units/routing/route-matching.test.js282
6 files changed, 361 insertions, 19 deletions
diff --git a/.changeset/hungry-peaches-speak.md b/.changeset/hungry-peaches-speak.md
new file mode 100644
index 000000000..4467b521d
--- /dev/null
+++ b/.changeset/hungry-peaches-speak.md
@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Prioritize dynamic prerendered routes over dynamic server routes
diff --git a/packages/astro/src/core/render/dev/index.ts b/packages/astro/src/core/render/dev/index.ts
index 26e7c85d5..c01137392 100644
--- a/packages/astro/src/core/render/dev/index.ts
+++ b/packages/astro/src/core/render/dev/index.ts
@@ -1,4 +1,3 @@
-import { fileURLToPath } from 'url';
import type {
AstroMiddlewareInstance,
AstroSettings,
@@ -65,7 +64,7 @@ export async function preload({
try {
// Load the module from the Vite SSR Runtime.
- const mod = (await env.loader.import(fileURLToPath(filePath))) as ComponentInstance;
+ const mod = (await env.loader.import(viteID(filePath))) as ComponentInstance;
return [renderers, mod];
} catch (error) {
diff --git a/packages/astro/src/core/routing/manifest/create.ts b/packages/astro/src/core/routing/manifest/create.ts
index a673e199b..a8680a104 100644
--- a/packages/astro/src/core/routing/manifest/create.ts
+++ b/packages/astro/src/core/routing/manifest/create.ts
@@ -173,6 +173,7 @@ function comparator(a: Item, b: Item) {
}
}
+ // endpoints are prioritized over pages
if (a.isPage !== b.isPage) {
return a.isPage ? 1 : -1;
}
diff --git a/packages/astro/src/prerender/routing.ts b/packages/astro/src/prerender/routing.ts
new file mode 100644
index 000000000..928789044
--- /dev/null
+++ b/packages/astro/src/prerender/routing.ts
@@ -0,0 +1,67 @@
+import type { AstroSettings, RouteData } from '../@types/astro';
+import { preload, type DevelopmentEnvironment } from '../core/render/dev/index.js';
+import { getPrerenderStatus } from './metadata.js';
+
+type GetSortedPreloadedMatchesParams = {
+ env: DevelopmentEnvironment;
+ matches: RouteData[];
+ settings: AstroSettings;
+};
+export async function getSortedPreloadedMatches({
+ env,
+ matches,
+ settings,
+}: GetSortedPreloadedMatchesParams) {
+ return (
+ await preloadAndSetPrerenderStatus({
+ env,
+ matches,
+ settings,
+ })
+ ).sort((a, b) => prioritizePrerenderedMatchesComparator(a.route, b.route));
+}
+
+type PreloadAndSetPrerenderStatusParams = {
+ env: DevelopmentEnvironment;
+ matches: RouteData[];
+ settings: AstroSettings;
+};
+async function preloadAndSetPrerenderStatus({
+ env,
+ matches,
+ settings,
+}: PreloadAndSetPrerenderStatusParams) {
+ const preloaded = await Promise.all(
+ matches.map(async (route) => {
+ const filePath = new URL(`./${route.component}`, settings.config.root);
+ const preloadedComponent = await preload({ env, filePath });
+
+ // gets the prerender metadata set by the `astro:scanner` vite plugin
+ const prerenderStatus = getPrerenderStatus({
+ filePath,
+ loader: env.loader,
+ });
+
+ if (prerenderStatus !== undefined) {
+ route.prerender = prerenderStatus;
+ }
+
+ return { preloadedComponent, route, filePath } as const;
+ })
+ );
+ return preloaded;
+}
+
+function prioritizePrerenderedMatchesComparator(a: RouteData, b: RouteData): number {
+ if (areRegexesEqual(a.pattern, b.pattern)) {
+ if (a.prerender !== b.prerender) {
+ return a.prerender ? -1 : 1;
+ }
+ return a.component < b.component ? -1 : 1;
+ }
+ return 0;
+}
+
+function areRegexesEqual(regexp1: RegExp, regexp2: RegExp) {
+ return regexp1.source === regexp2.source && regexp1.global === regexp2.global;
+}
diff --git a/packages/astro/src/vite-plugin-astro-server/route.ts b/packages/astro/src/vite-plugin-astro-server/route.ts
index 86b8e5814..56fd60f14 100644
--- a/packages/astro/src/vite-plugin-astro-server/route.ts
+++ b/packages/astro/src/vite-plugin-astro-server/route.ts
@@ -16,10 +16,10 @@ import { preload, renderPage } from '../core/render/dev/index.js';
import { getParamsAndProps, GetParamsAndPropsError } from '../core/render/index.js';
import { createRequest } from '../core/request.js';
import { matchAllRoutes } from '../core/routing/index.js';
-import { getPrerenderStatus } from '../prerender/metadata.js';
import { isHybridOutput } from '../prerender/utils.js';
import { log404 } from './common.js';
import { handle404Response, writeSSRResult, writeWebResponse } from './response.js';
+import { getSortedPreloadedMatches } from '../prerender/routing.js';
type AsyncReturnType<T extends (...args: any) => Promise<any>> = T extends (
...args: any
@@ -47,24 +47,12 @@ export async function matchRoute(
): Promise<MatchedRoute | undefined> {
const { logging, settings, routeCache } = env;
const matches = matchAllRoutes(pathname, manifest);
+ const preloadedMatches = await getSortedPreloadedMatches({ env, matches, settings });
- for await (const maybeRoute of matches) {
- const filePath = new URL(`./${maybeRoute.component}`, settings.config.root);
- const preloadedComponent = await preload({ env, filePath });
-
- // gets the prerender metadata set by the `astro:scanner` vite plugin
- const prerenderStatus = getPrerenderStatus({
- filePath,
- loader: env.loader,
- });
-
- if (prerenderStatus !== undefined) {
- maybeRoute.prerender = prerenderStatus;
- }
-
- const [, mod] = preloadedComponent;
+ for await (const { preloadedComponent, route: maybeRoute, filePath } of preloadedMatches) {
// attempt to get static paths
// if this fails, we have a bad URL match!
+ const [, mod] = preloadedComponent;
const paramsAndPropsRes = await getParamsAndProps({
mod,
route: maybeRoute,
@@ -210,7 +198,7 @@ export async function handleRoute(
await writeWebResponse(res, result.response);
} else {
let contentType = 'text/plain';
- // Dynamic routes don’t include `route.pathname`, so synthesize a path for these (e.g. 'src/pages/[slug].svg')
+ // Dynamic routes don't include `route.pathname`, so synthesize a path for these (e.g. 'src/pages/[slug].svg')
const filepath =
route.pathname ||
route.segments.map((segment) => segment.map((p) => p.content).join('')).join('/');
diff --git a/packages/astro/test/units/routing/route-matching.test.js b/packages/astro/test/units/routing/route-matching.test.js
new file mode 100644
index 000000000..692518382
--- /dev/null
+++ b/packages/astro/test/units/routing/route-matching.test.js
@@ -0,0 +1,282 @@
+// @ts-check
+import { createFs, createRequestAndResponse } from '../test-utils.js';
+import { createRouteManifest, matchAllRoutes } from '../../../dist/core/routing/index.js';
+import { fileURLToPath } from 'url';
+import { defaultLogging } from '../../test-utils.js';
+import { createViteLoader } from '../../../dist/core/module-loader/vite.js';
+import { createDevelopmentEnvironment } from '../../../dist/core/render/dev/environment.js';
+import { expect } from 'chai';
+import { createContainer } from '../../../dist/core/dev/container.js';
+import * as cheerio from 'cheerio';
+import testAdapter from '../../test-adapter.js';
+import { getSortedPreloadedMatches } from '../../../dist/prerender/routing.js';
+
+const root = new URL('../../fixtures/alias/', import.meta.url);
+const fileSystem = {
+ '/src/pages/[serverDynamic].astro': `
+ ---
+ export const prerender = false;
+ ---
+ <p>Server dynamic route! slug:{Astro.params.serverDynamic}</p>
+ `,
+
+ '/src/pages/[xStaticDynamic].astro': `
+ ---
+ export function getStaticPaths() {
+ return [
+ {
+ params: {
+ xStaticDynamic: "static-dynamic-route-here",
+ },
+ },
+ ];
+ }
+ ---
+ <p>Prerendered dynamic route!</p>
+ `,
+ '/src/pages/[aStaticDynamic].astro': `
+ ---
+ export function getStaticPaths() {
+ return [
+ {
+ params: {
+ aStaticDynamic: "another-static-dynamic-route-here",
+ },
+ },
+ ];
+ }
+ ---
+ <p>Another prerendered dynamic route!</p>
+ `,
+ '/src/pages/[...serverRest].astro': `
+ ---
+ export const prerender = false;
+ ---
+ <p>Server rest route! slug:{Astro.params.serverRest}</p>
+ `,
+ '/src/pages/[...xStaticRest].astro': `
+ ---
+ export function getStaticPaths() {
+ return [
+ {
+ params: {
+ xStaticRest: undefined,
+ },
+ },
+ ];
+ }
+ ---
+ <p>Prerendered rest route!</p>
+`,
+ '/src/pages/[...aStaticRest].astro': `
+ ---
+ export function getStaticPaths() {
+ return [
+ {
+ params: {
+ aStaticRest: "another/static-rest-route-here",
+ },
+ },
+ ];
+ }
+ ---
+ <p>Another prerendered rest route!</p>
+`,
+
+ '/src/pages/nested/[...serverRest].astro': `
+ ---
+ export const prerender = false;
+ ---
+ <p>Nested server rest route! slug: {Astro.params.serverRest}</p>
+ `,
+ '/src/pages/nested/[...xStaticRest].astro': `
+ ---
+ export function getStaticPaths() {
+ return [
+ {
+ params: {
+ xStaticRest: undefined,
+ },
+ },
+ ];
+ }
+ ---
+ <p>Nested prerendered rest route!</p>
+`,
+ '/src/pages/nested/[...aStaticRest].astro': `
+ ---
+ export function getStaticPaths() {
+ return [
+ {
+ params: {
+ aStaticRest: "another-nested-static-dynamic-rest-route-here",
+ },
+ },
+ ];
+ }
+ ---
+ <p>Another nested prerendered rest route!</p>
+`,
+};
+
+describe('Route matching', () => {
+ let env;
+ let manifest;
+ let container;
+ let settings;
+
+ before(async () => {
+ const fs = createFs(fileSystem, root);
+ container = await createContainer({
+ fs,
+ root,
+ userConfig: {
+ trailingSlash: 'never',
+ output: 'hybrid',
+ experimental: {
+ hybridOutput: true,
+ },
+ adapter: testAdapter(),
+ },
+ disableTelemetry: true,
+ });
+ settings = container.settings;
+
+ const loader = createViteLoader(container.viteServer);
+ env = createDevelopmentEnvironment(container.settings, defaultLogging, loader);
+ manifest = createRouteManifest(
+ {
+ cwd: fileURLToPath(root),
+ settings,
+ fsMod: fs,
+ },
+ defaultLogging
+ );
+ });
+
+ after(async () => {
+ await container.close();
+ });
+
+ describe('Matched routes', () => {
+ it('should be sorted correctly', async () => {
+ const matches = matchAllRoutes('/try-matching-a-route', manifest);
+ const preloadedMatches = await getSortedPreloadedMatches({ env, matches, settings });
+ const sortedRouteNames = preloadedMatches.map((match) => match.route.route);
+
+ expect(sortedRouteNames).to.deep.equal([
+ '/[astaticdynamic]',
+ '/[xstaticdynamic]',
+ '/[serverdynamic]',
+ '/[...astaticrest]',
+ '/[...xstaticrest]',
+ '/[...serverrest]',
+ ]);
+ });
+ it('nested should be sorted correctly', async () => {
+ const matches = matchAllRoutes('/nested/try-matching-a-route', manifest);
+ const preloadedMatches = await getSortedPreloadedMatches({ env, matches, settings });
+ const sortedRouteNames = preloadedMatches.map((match) => match.route.route);
+
+ expect(sortedRouteNames).to.deep.equal([
+ '/nested/[...astaticrest]',
+ '/nested/[...xstaticrest]',
+ '/nested/[...serverrest]',
+ '/[...astaticrest]',
+ '/[...xstaticrest]',
+ '/[...serverrest]',
+ ]);
+ });
+ });
+
+ describe('Request', () => {
+ it('should correctly match a static dynamic route I', async () => {
+ const { req, res, text } = createRequestAndResponse({
+ method: 'GET',
+ url: '/static-dynamic-route-here',
+ });
+ container.handle(req, res);
+ const html = await text();
+ const $ = cheerio.load(html);
+ expect($('p').text()).to.equal('Prerendered dynamic route!');
+ });
+
+ it('should correctly match a static dynamic route II', async () => {
+ const { req, res, text } = createRequestAndResponse({
+ method: 'GET',
+ url: '/another-static-dynamic-route-here',
+ });
+ container.handle(req, res);
+ const html = await text();
+ const $ = cheerio.load(html);
+ expect($('p').text()).to.equal('Another prerendered dynamic route!');
+ });
+
+ it('should correctly match a server dynamic route', async () => {
+ const { req, res, text } = createRequestAndResponse({
+ method: 'GET',
+ url: '/a-random-slug-was-matched',
+ });
+ container.handle(req, res);
+ const html = await text();
+ const $ = cheerio.load(html);
+ expect($('p').text()).to.equal('Server dynamic route! slug:a-random-slug-was-matched');
+ });
+
+ it('should correctly match a static rest route I', async () => {
+ const { req, res, text } = createRequestAndResponse({
+ method: 'GET',
+ url: '',
+ });
+ container.handle(req, res);
+ const html = await text();
+ const $ = cheerio.load(html);
+ expect($('p').text()).to.equal('Prerendered rest route!');
+ });
+
+ it('should correctly match a static rest route II', async () => {
+ const { req, res, text } = createRequestAndResponse({
+ method: 'GET',
+ url: '/another/static-rest-route-here',
+ });
+ container.handle(req, res);
+ const html = await text();
+ const $ = cheerio.load(html);
+ expect($('p').text()).to.equal('Another prerendered rest route!');
+ });
+
+ it('should correctly match a nested static rest route index', async () => {
+ const { req, res, text } = createRequestAndResponse({
+ method: 'GET',
+ url: '/nested',
+ });
+ container.handle(req, res);
+ const html = await text();
+ const $ = cheerio.load(html);
+ expect($('p').text()).to.equal('Nested prerendered rest route!');
+ });
+
+ it('should correctly match a nested static rest route', async () => {
+ const { req, res, text } = createRequestAndResponse({
+ method: 'GET',
+ url: '/nested/another-nested-static-dynamic-rest-route-here',
+ });
+ container.handle(req, res);
+ const html = await text();
+ const $ = cheerio.load(html);
+ expect($('p').text()).to.equal('Another nested prerendered rest route!');
+ });
+
+ it('should correctly match a nested server rest route', async () => {
+ const { req, res, text } = createRequestAndResponse({
+ method: 'GET',
+ url: '/nested/a-random-slug-was-matched',
+ });
+ container.handle(req, res);
+
+ const html = await text();
+ const $ = cheerio.load(html);
+ expect($('p').text()).to.equal('Nested server rest route! slug: a-random-slug-was-matched');
+ });
+ });
+});