From 15fa9babf31a9b8ab8fc8e611c931c178137e2f9 Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Wed, 9 Oct 2024 16:24:34 +0100 Subject: [PATCH] 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 --- .changeset/light-pears-accept.md | 7 +++++ .changeset/three-days-cough.md | 7 +++++ packages/astro/src/core/build/generate.ts | 13 ++-------- packages/astro/src/core/routing/3xx.ts | 19 ++++++++++++++ .../astro/src/core/routing/manifest/create.ts | 8 +++++- .../src/vite-plugin-astro-server/route.ts | 26 +++++++++++++++++++ 6 files changed, 68 insertions(+), 12 deletions(-) create mode 100644 .changeset/light-pears-accept.md create mode 100644 .changeset/three-days-cough.md create mode 100644 packages/astro/src/core/routing/3xx.ts diff --git a/.changeset/light-pears-accept.md b/.changeset/light-pears-accept.md new file mode 100644 index 0000000000..58db0e33fd --- /dev/null +++ b/.changeset/light-pears-accept.md @@ -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. diff --git a/.changeset/three-days-cough.md b/.changeset/three-days-cough.md new file mode 100644 index 0000000000..42b3c288e8 --- /dev/null +++ b/.changeset/three-days-cough.md @@ -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. diff --git a/packages/astro/src/core/build/generate.ts b/packages/astro/src/core/build/generate.ts index 66a42807fe..64ba643092 100644 --- a/packages/astro/src/core/build/generate.ts +++ b/packages/astro/src/core/build/generate.ts @@ -33,6 +33,7 @@ import { getRedirectLocationOrThrow, routeIsRedirect } from '../redirects/index. import { RenderContext } from '../render-context.js'; import { callGetStaticPaths } from '../render/route-cache.js'; import { createRequest } from '../request.js'; +import { redirectTemplate } from '../routing/3xx.js'; import { matchRoute } from '../routing/match.js'; import { stringifyParams } from '../routing/params.js'; import { getOutputFilename } from '../util.js'; @@ -445,17 +446,7 @@ async function generatePath( const siteURL = config.site; const location = siteURL ? new URL(locationSite, siteURL) : locationSite; const fromPath = new URL(request.url).pathname; - // A short delay causes Google to interpret the redirect as temporary. - // https://developers.google.com/search/docs/crawling-indexing/301-redirects#metarefresh - const delay = response.status === 302 ? 2 : 0; - body = ` -Redirecting to: ${location} - - - - - Redirecting from ${fromPath} to ${location} -`; + body = redirectTemplate({ status: response.status, location, from: fromPath }); if (config.compressHTML === true) { body = body.replaceAll('\n', ''); } diff --git a/packages/astro/src/core/routing/3xx.ts b/packages/astro/src/core/routing/3xx.ts new file mode 100644 index 0000000000..fd4e211ed7 --- /dev/null +++ b/packages/astro/src/core/routing/3xx.ts @@ -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 ` +Redirecting to: ${location} + + + + + Redirecting from ${from} to ${location} +`; +} diff --git a/packages/astro/src/core/routing/manifest/create.ts b/packages/astro/src/core/routing/manifest/create.ts index 4e0fb964a8..38794b771c 100644 --- a/packages/astro/src/core/routing/manifest/create.ts +++ b/packages/astro/src/core/routing/manifest/create.ts @@ -498,8 +498,14 @@ export async function createRouteManifest( 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[] = [ - ...[...fileBasedRoutes, ...injectedRoutes, ...redirectRoutes].sort(routeComparator), + ...[...filteredFiledBasedRoutes, ...injectedRoutes, ...redirectRoutes].sort(routeComparator), ]; settings.buildOutput = getPrerenderDefault(config) ? 'static' : 'server'; diff --git a/packages/astro/src/vite-plugin-astro-server/route.ts b/packages/astro/src/vite-plugin-astro-server/route.ts index faccf22943..6568b3f1f5 100644 --- a/packages/astro/src/vite-plugin-astro-server/route.ts +++ b/packages/astro/src/vite-plugin-astro-server/route.ts @@ -8,9 +8,11 @@ import { import { AstroErrorData, isAstroError } from '../core/errors/index.js'; import { req } from '../core/messages.js'; import { loadMiddleware } from '../core/middleware/loadMiddleware.js'; +import { routeIsRedirect } from '../core/redirects/index.js'; import { RenderContext } from '../core/render-context.js'; import { type SSROptions, getProps } from '../core/render/index.js'; import { createRequest } from '../core/request.js'; +import { redirectTemplate } from '../core/routing/3xx.js'; import { matchAllRoutes } from '../core/routing/index.js'; import { getSortedPreloadedMatches } from '../prerender/routing.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 // 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 >= 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); return; }