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

Implement support for dynamic routes in redirects #7173

Merged
merged 3 commits into from
May 23, 2023
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/astro/src/@types/astro.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1841,6 +1841,7 @@ export interface RouteData {
type: RouteType;
prerender: boolean;
redirect?: string;
redirectRoute?: RouteData;
}

export type RedirectRouteData = RouteData & {
Expand Down
23 changes: 14 additions & 9 deletions packages/astro/src/core/build/generate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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}`);
Expand Down Expand Up @@ -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;
}
Expand Down
9 changes: 8 additions & 1 deletion packages/astro/src/core/redirects/helpers.ts
Original file line number Diff line number Diff line change
@@ -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 || '/';
}
2 changes: 1 addition & 1 deletion packages/astro/src/core/redirects/index.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
export { getRedirectLocationOrThrow } from './validate.js';
export { routeIsRedirect } from './helpers.js';
export { routeIsRedirect, redirectRouteGenerate } from './helpers.js';
4 changes: 2 additions & 2 deletions packages/astro/src/core/render/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
}
});
}
Expand Down
6 changes: 4 additions & 2 deletions packages/astro/src/core/routing/manifest/create.ts
Original file line number Diff line number Diff line change
Expand Up @@ -444,18 +444,20 @@ export function createRouteManifest(
.map(([{ dynamic, content }]) => (dynamic ? `[${content}]` : content))
.join('/')}`.toLowerCase();



routes.unshift({
type: 'redirect',
route,
pattern,
segments,
params,
component: '',
component: from,
generate,
pathname: pathname || void 0,
prerender: false,
redirect: to
redirect: to,
redirectRoute: routes.find(r => r.route === to)
});
});

Expand Down
Original file line number Diff line number Diff line change
@@ -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>
10 changes: 10 additions & 0 deletions packages/astro/test/fixtures/ssr-redirect/src/pages/index.astro
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
---
<html>
<head>
<title>Testing</title>
</head>
<body>
<h1>Testing</h1>
</body>
</html>
27 changes: 26 additions & 1 deletion packages/astro/test/redirects.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});
});
});
60 changes: 26 additions & 34 deletions packages/integrations/netlify/src/shared.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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',
]);
});
});
Original file line number Diff line number Diff line change
@@ -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>
9 changes: 5 additions & 4 deletions packages/integrations/netlify/test/static/redirects.test.js
Original file line number Diff line number Diff line change
@@ -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} */
Expand All @@ -16,7 +14,8 @@ describe('SSG - Redirects', () => {
site: `http://example.com`,
integrations: [testIntegration()],
redirects: {
'/other': '/'
'/other': '/',
'/blog/[...slug]': '/team/articles/[...slug]'
}
});
await fixture.build();
Expand All @@ -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'
]);
});
});