Implement support for dynamic routes in redirects (#7173)

* Implement support for dynamic routes in redirects * Remove the .only * No need to special-case redirects in static build
2023-05-23 09:01:46 -04:00 · 2023-05-23 09:01:46 -04:00 · 25d7d208ba
commit 25d7d208ba
parent 4857c7d317
13 changed files with 151 additions and 55 deletions
--- a/packages/astro/src/@types/astro.ts
+++ b/packages/astro/src/@types/astro.ts
@ -1841,6 +1841,7 @@ export interface RouteData {
 	type: RouteType;
 	prerender: boolean;
 	redirect?: string;
+	redirectRoute?: RouteData;
 }

 export type RedirectRouteData = RouteData & {
--- a/packages/astro/src/core/build/generate.ts
+++ b/packages/astro/src/core/build/generate.ts
@ -35,8 +35,8 @@ import { debug, info } from '../logger/core.js';
 import { callMiddleware } from '../middleware/callMiddleware.js';
 import { createEnvironment, createRenderContext, renderPage } from '../render/index.js';
 import { callGetStaticPaths } from '../render/route-cache.js';
-import { getRedirectLocationOrThrow } from '../redirects/index.js';
-import {
+import { getRedirectLocationOrThrow, routeIsRedirect } from '../redirects/index.js';
+import {	
 	createAssetLink,
 	createModuleScriptsSet,
 	createStylesheetElementSet,
@ -173,15 +173,18 @@ async function generatePage(
 	let pageModulePromise = ssrEntry.pageMap?.get(pageData.component);
 	const middleware = ssrEntry.middleware;

-	if (!pageModulePromise) {
-		if(pageData.route.type === 'redirect') {
-			pageModulePromise = () => Promise.resolve({ 'default': Function.prototype }) as any;
+	if (!pageModulePromise && routeIsRedirect(pageData.route)) {
+		if(pageData.route.redirectRoute) {
+			pageModulePromise = ssrEntry.pageMap?.get(pageData.route.redirectRoute!.component);
 		} else {
-			throw new Error(
-				`Unable to find the module for ${pageData.component}. This is unexpected and likely a bug in Astro, please report.`
-			);
+			pageModulePromise = { default: () => {} } as any;
 		}
 	}
+	if (!pageModulePromise) {
+		throw new Error(
+			`Unable to find the module for ${pageData.component}. This is unexpected and likely a bug in Astro, please report.`
+		);
+	}
 	const pageModule = await pageModulePromise();
 	if (shouldSkipDraft(pageModule, opts.settings)) {
 		info(opts.logging, null, `${magenta('⚠️')}  Skipping draft ${pageData.route.component}`);
@ -519,7 +522,9 @@ async function generatePath(
 			case 301:
 			case 302: {
 				const location = getRedirectLocationOrThrow(response.headers);
-				body = `<!doctype html><meta http-equiv="refresh" content="0;url=${location}" />`;
+				body = `<!doctype html>
+<title>Redirecting to: ${location}</title>
+<meta http-equiv="refresh" content="0;url=${location}" />`;
 				pageData.route.redirect = location;
 				break;
 			}
--- a/packages/astro/src/core/redirects/helpers.ts
+++ b/packages/astro/src/core/redirects/helpers.ts
@ -1,5 +1,12 @@
-import type { RouteData, RedirectRouteData } from '../../@types/astro';
+import type { RouteData, RedirectRouteData, Params } from '../../@types/astro';

 export function routeIsRedirect(route: RouteData | undefined): route is RedirectRouteData {
 	return route?.type === 'redirect';
 }
+
+export function redirectRouteGenerate(redirectRoute: RouteData, data: Params): string {
+	const routeData = redirectRoute.redirectRoute;
+	const route = redirectRoute.redirect;
+
+	return routeData?.generate(data) || routeData?.pathname || route || '/';
+}
--- a/packages/astro/src/core/redirects/index.ts
+++ b/packages/astro/src/core/redirects/index.ts
@ -1,2 +1,2 @@
 export { getRedirectLocationOrThrow } from './validate.js';
-export { routeIsRedirect } from './helpers.js';
+export { routeIsRedirect, redirectRouteGenerate } from './helpers.js';
--- a/packages/astro/src/core/render/core.ts
+++ b/packages/astro/src/core/render/core.ts
@ -8,7 +8,7 @@ import type { RenderContext } from './context.js';
 import type { Environment } from './environment.js';
 import { createResult } from './result.js';
 import { callGetStaticPaths, findPathItemByKey, RouteCache } from './route-cache.js';
-import { routeIsRedirect } from '../redirects/index.js';
+import { routeIsRedirect, redirectRouteGenerate } from '../redirects/index.js';

 interface GetParamsAndPropsOptions {
 	mod: ComponentInstance;
@ -116,7 +116,7 @@ export async function renderPage({ mod, renderContext, env, apiContext }: Render
 		return new Response(null, {
 			status: 301,
 			headers: {
-				location: renderContext.route!.redirect
+				location: redirectRouteGenerate(renderContext.route!, renderContext.params)
 			}
 		});
 	}
--- a/packages/astro/src/core/routing/manifest/create.ts
+++ b/packages/astro/src/core/routing/manifest/create.ts
@ -444,6 +444,7 @@ export function createRouteManifest(
 			.map(([{ dynamic, content }]) => (dynamic ? `[${content}]` : content))
 			.join('/')}`.toLowerCase();

+		

 		routes.unshift({
 			type: 'redirect',
@ -451,11 +452,12 @@ export function createRouteManifest(
 			pattern,
 			segments,
 			params,
-			component: '',
+			component: from,
 			generate,
 			pathname: pathname || void 0,
 			prerender: false,
-			redirect: to
+			redirect: to,
+			redirectRoute: routes.find(r => r.route === to)
 		});
 	});

--- a/packages/astro/test/fixtures/ssr-redirect/src/pages/articles/[...slug].astro
+++ b/packages/astro/test/fixtures/ssr-redirect/src/pages/articles/[...slug].astro
@ -0,0 +1,25 @@
+---
+export const getStaticPaths = (async () => {
+  const posts = [
+		{ slug: 'one', data: {draft: false, title: 'One'} },
+		{ slug: 'two', data: {draft: false, title: 'Two'} }
+	];
+  return posts.map((post) => {
+    return {
+      params: { slug: post.slug },
+      props: { draft: post.data.draft, title: post.data.title },
+    };
+  });
+})
+
+const { slug } = Astro.params;
+const { title } = Astro.props;
+---
+<html>
+	<head>
+		<title>{ title }</title>
+	</head>
+	<body>
+		<h1>{ title }</h1>
+	</body>
+</html>
--- a/packages/astro/test/fixtures/ssr-redirect/src/pages/index.astro
+++ b/packages/astro/test/fixtures/ssr-redirect/src/pages/index.astro
@ -0,0 +1,10 @@
+---
+---
+<html>
+	<head>
+		<title>Testing</title>
+	</head>
+	<body>
+		<h1>Testing</h1>
+	</body>
+</html>
--- a/packages/astro/test/redirects.test.js
+++ b/packages/astro/test/redirects.test.js
@ -45,14 +45,39 @@ describe('Astro.redirect', () => {
 			fixture = await loadFixture({
 				root: './fixtures/ssr-redirect/',
 				output: 'static',
+				redirects: {
+					'/one': '/',
+					'/two': '/',
+					'/blog/[...slug]': '/articles/[...slug]'
+				}
 			});
 			await fixture.build();
 		});
 	
-		it('Includes the meta refresh tag.', async () => {
+		it('Includes the meta refresh tag in Astro.redirect pages', async () => {
 			const html = await fixture.readFile('/secret/index.html');
 			expect(html).to.include('http-equiv="refresh');
 			expect(html).to.include('url=/login');
 		});
+
+		it('Includes the meta refresh tag in `redirect` config pages', async () => {
+			let html = await fixture.readFile('/one/index.html');
+			expect(html).to.include('http-equiv="refresh');
+			expect(html).to.include('url=/');
+
+			html = await fixture.readFile('/two/index.html');
+			expect(html).to.include('http-equiv="refresh');
+			expect(html).to.include('url=/');
+		});
+
+		it('Generates page for dynamic routes', async () => {
+			let html = await fixture.readFile('/blog/one/index.html');
+			expect(html).to.include('http-equiv="refresh');
+			expect(html).to.include('url=/articles/one');
+
+			html = await fixture.readFile('/blog/two/index.html');
+			expect(html).to.include('http-equiv="refresh');
+			expect(html).to.include('url=/articles/two');
+		});
 	});
 });
--- a/packages/integrations/netlify/src/shared.ts
+++ b/packages/integrations/netlify/src/shared.ts
@ -65,46 +65,18 @@ export async function createRedirects(
 				}
 			}
 		} else {
-			const pattern =
-				'/' +
-				route.segments
-					.map(([part]) => {
-						//(part.dynamic ? '*' : part.content)
-						if (part.dynamic) {
-							if (part.spread) {
-								return '*';
-							} else {
-								return ':' + part.content;
-							}
-						} else {
-							return part.content;
-						}
-					})
-					.join('/');
+			const pattern = generateDynamicPattern(route);

-			//if(kind === 'static') {
-				if(route.redirect) {
-					definitions.push({
-						dynamic: true,
-						input: pattern,
-						target: route.redirect,
-						status: 301,
-						weight: 1
-					});
-					continue;
-				}
-
-			if(kind === 'static') {
-				continue;	
-			}
-			else if (route.distURL) {
+			if (route.distURL) {
+				const targetRoute = route.redirectRoute ?? route;
+				const targetPattern = generateDynamicPattern(targetRoute);
 				const target =
-					`${pattern}` + (config.build.format === 'directory' ? '/index.html' : '.html');
+					`${targetPattern}` + (config.build.format === 'directory' ? '/index.html' : '.html');
 				definitions.push({
 					dynamic: true,
 					input: pattern,
 					target,
-					status: 200,
+					status: route.type === 'redirect' ? 301 : 200,
 					weight: 1,
 				});
 			} else {
@ -127,6 +99,26 @@ export async function createRedirects(
 	await fs.promises.appendFile(_redirectsURL, _redirects, 'utf-8');
 }

+function generateDynamicPattern(route: RouteData) {
+	const pattern =
+		'/' +
+		route.segments
+			.map(([part]) => {
+				//(part.dynamic ? '*' : part.content)
+				if (part.dynamic) {
+					if (part.spread) {
+						return '*';
+					} else {
+						return ':' + part.content;
+					}
+				} else {
+					return part.content;
+				}
+			})
+			.join('/');
+	return pattern;
+}
+
 function prettify(definitions: RedirectDefinition[]) {
 	let minInputLength = 4,
 		minTargetLength = 4;
--- a/packages/integrations/netlify/test/functions/redirects.test.js
+++ b/packages/integrations/netlify/test/functions/redirects.test.js
@ -33,7 +33,10 @@ describe('SSG - Redirects', () => {

 			// This uses the dynamic Astro.redirect, so we don't know that it's a redirect
 			// until runtime. This is correct!
-			'/nope', '/.netlify/functions/entry', '200'
+			'/nope', '/.netlify/functions/entry', '200',
+
+			// A real route
+			'/team/articles/*', '/.netlify/functions/entry', '200',
 		]);
 	});
 });
--- a/packages/integrations/netlify/test/static/fixtures/redirects/src/pages/team/articles/[...slug].astro
+++ b/packages/integrations/netlify/test/static/fixtures/redirects/src/pages/team/articles/[...slug].astro
@ -0,0 +1,25 @@
+---
+export const getStaticPaths = (async () => {
+  const posts = [
+		{ slug: 'one', data: {draft: false, title: 'One'} },
+		{ slug: 'two', data: {draft: false, title: 'Two'} }
+	];
+  return posts.map((post) => {
+    return {
+      params: { slug: post.slug },
+      props: { draft: post.data.draft, title: post.data.title },
+    };
+  });
+})
+
+const { slug } = Astro.params;
+const { title } = Astro.props;
+---
+<html>
+	<head>
+		<title>{ title }</title>
+	</head>
+	<body>
+		<h1>{ title }</h1>
+	</body>
+</html>
--- a/packages/integrations/netlify/test/static/redirects.test.js
+++ b/packages/integrations/netlify/test/static/redirects.test.js
@ -1,8 +1,6 @@
 import { expect } from 'chai';
-import { load as cheerioLoad } from 'cheerio';
 import { loadFixture, testIntegration } from './test-utils.js';
 import { netlifyStatic } from '../../dist/index.js';
-import { fileURLToPath } from 'url';

 describe('SSG - Redirects', () => {
 	/** @type {import('../../../astro/test/test-utils').Fixture} */
@ -16,7 +14,8 @@ describe('SSG - Redirects', () => {
 			site: `http://example.com`,
 			integrations: [testIntegration()],
 			redirects: {
-				'/other': '/'
+				'/other': '/',
+				'/blog/[...slug]': '/team/articles/[...slug]'
 			}
 		});
 		await fixture.build();
@ -26,8 +25,10 @@ describe('SSG - Redirects', () => {
 		let redirects = await fixture.readFile('/_redirects');
 		let parts = redirects.split(/\s+/);
 		expect(parts).to.deep.equal([
+			'/blog/*', '/team/articles/*/index.html', '301',
 			'/other', '/', '301',
-			'/nope', '/', '301'
+			'/nope', '/', '301',
+			'/team/articles/*', '/team/articles/*/index.html', '200'
 		]);
 	});
 });