0
Fork 0
mirror of https://github.com/withastro/astro.git synced 2024-12-16 21:46:22 -05:00

fix(routing): correct redirects in dev (#12169)

* fix(routing): correct redirects in dev

* simplify code

* address feedback

* move delay inside the function, to keep the comment in one place
This commit is contained in:
Emanuele Stoppa 2024-10-09 16:24:34 +01:00 committed by GitHub
parent 1cd30852a3
commit 15fa9babf3
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 68 additions and 12 deletions

View file

@ -0,0 +1,7 @@
---
'astro': patch
---
Fixes a bug where configured redirects were incorrectly constructed when reading the file system.
This caused an issue where configuring a redirect in `astro.config.mjs` like `{ /old: /new }`, failed to trigger the correct redirect in the dev server.

View file

@ -0,0 +1,7 @@
---
'astro': patch
---
Fixes a bug where the dev server was not providing a consistent user experience for configured redirects.
With the fix, when you configure a redirect in `astro.config.mjs` like this `{ /old: "/new" }`, the dev server return an HTML response that matches the one emitted by a static build.

View file

@ -33,6 +33,7 @@ import { getRedirectLocationOrThrow, routeIsRedirect } from '../redirects/index.
import { RenderContext } from '../render-context.js'; import { RenderContext } from '../render-context.js';
import { callGetStaticPaths } from '../render/route-cache.js'; import { callGetStaticPaths } from '../render/route-cache.js';
import { createRequest } from '../request.js'; import { createRequest } from '../request.js';
import { redirectTemplate } from '../routing/3xx.js';
import { matchRoute } from '../routing/match.js'; import { matchRoute } from '../routing/match.js';
import { stringifyParams } from '../routing/params.js'; import { stringifyParams } from '../routing/params.js';
import { getOutputFilename } from '../util.js'; import { getOutputFilename } from '../util.js';
@ -445,17 +446,7 @@ async function generatePath(
const siteURL = config.site; const siteURL = config.site;
const location = siteURL ? new URL(locationSite, siteURL) : locationSite; const location = siteURL ? new URL(locationSite, siteURL) : locationSite;
const fromPath = new URL(request.url).pathname; const fromPath = new URL(request.url).pathname;
// A short delay causes Google to interpret the redirect as temporary. body = redirectTemplate({ status: response.status, location, from: fromPath });
// https://developers.google.com/search/docs/crawling-indexing/301-redirects#metarefresh
const delay = response.status === 302 ? 2 : 0;
body = `<!doctype html>
<title>Redirecting to: ${location}</title>
<meta http-equiv="refresh" content="${delay};url=${location}">
<meta name="robots" content="noindex">
<link rel="canonical" href="${location}">
<body>
<a href="${location}">Redirecting from <code>${fromPath}</code> to <code>${location}</code></a>
</body>`;
if (config.compressHTML === true) { if (config.compressHTML === true) {
body = body.replaceAll('\n', ''); body = body.replaceAll('\n', '');
} }

View file

@ -0,0 +1,19 @@
export type RedirectTemplate = {
from: string;
location: string | URL;
status: number;
};
export function redirectTemplate({ status, location, from }: RedirectTemplate) {
// A short delay causes Google to interpret the redirect as temporary.
// https://developers.google.com/search/docs/crawling-indexing/301-redirects#metarefresh
const delay = status === 302 ? 2 : 0;
return `<!doctype html>
<title>Redirecting to: ${location}</title>
<meta http-equiv="refresh" content="${delay};url=${location}">
<meta name="robots" content="noindex">
<link rel="canonical" href="${location}">
<body>
<a href="${location}">Redirecting from <code>${from}</code> to <code>${location}</code></a>
</body>`;
}

View file

@ -498,8 +498,14 @@ export async function createRouteManifest(
const redirectRoutes = createRedirectRoutes(params, routeMap, logger); const redirectRoutes = createRedirectRoutes(params, routeMap, logger);
// we remove the file based routes that were deemed redirects
const filteredFiledBasedRoutes = fileBasedRoutes.filter((fileBasedRoute) => {
const isRedirect = redirectRoutes.findIndex((rd) => rd.route === fileBasedRoute.route);
return isRedirect < 0;
});
const routes: RouteData[] = [ const routes: RouteData[] = [
...[...fileBasedRoutes, ...injectedRoutes, ...redirectRoutes].sort(routeComparator), ...[...filteredFiledBasedRoutes, ...injectedRoutes, ...redirectRoutes].sort(routeComparator),
]; ];
settings.buildOutput = getPrerenderDefault(config) ? 'static' : 'server'; settings.buildOutput = getPrerenderDefault(config) ? 'static' : 'server';

View file

@ -8,9 +8,11 @@ import {
import { AstroErrorData, isAstroError } from '../core/errors/index.js'; import { AstroErrorData, isAstroError } from '../core/errors/index.js';
import { req } from '../core/messages.js'; import { req } from '../core/messages.js';
import { loadMiddleware } from '../core/middleware/loadMiddleware.js'; import { loadMiddleware } from '../core/middleware/loadMiddleware.js';
import { routeIsRedirect } from '../core/redirects/index.js';
import { RenderContext } from '../core/render-context.js'; import { RenderContext } from '../core/render-context.js';
import { type SSROptions, getProps } from '../core/render/index.js'; import { type SSROptions, getProps } from '../core/render/index.js';
import { createRequest } from '../core/request.js'; import { createRequest } from '../core/request.js';
import { redirectTemplate } from '../core/routing/3xx.js';
import { matchAllRoutes } from '../core/routing/index.js'; import { matchAllRoutes } from '../core/routing/index.js';
import { getSortedPreloadedMatches } from '../prerender/routing.js'; import { getSortedPreloadedMatches } from '../prerender/routing.js';
import type { ComponentInstance, ManifestData } from '../types/astro.js'; import type { ComponentInstance, ManifestData } from '../types/astro.js';
@ -286,6 +288,30 @@ export async function handleRoute({
// By default, we should give priority to the status code passed, although it's possible that // By default, we should give priority to the status code passed, although it's possible that
// the `Response` emitted by the user is a redirect. If so, then return the returned response. // the `Response` emitted by the user is a redirect. If so, then return the returned response.
if (response.status < 400 && response.status >= 300) { if (response.status < 400 && response.status >= 300) {
if (
response.status >= 300 &&
response.status < 400 &&
routeIsRedirect(route) &&
!config.build.redirects &&
pipeline.settings.buildOutput === 'static'
) {
// If we're here, it means that the calling static redirect that was configured by the user
// We try to replicate the same behaviour that we provide during a static build
response = new Response(
redirectTemplate({
status: response.status,
location: response.headers.get('location')!,
from: pathname,
}),
{
status: 200,
headers: {
...response.headers,
'content-type': 'text/html',
},
},
);
}
await writeSSRResult(request, response, incomingResponse); await writeSSRResult(request, response, incomingResponse);
return; return;
} }