diff --git a/.changeset/four-pumpkins-try.md b/.changeset/four-pumpkins-try.md new file mode 100644 index 000000000..81b49bc63 --- /dev/null +++ b/.changeset/four-pumpkins-try.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Fix for allowing the root path / as a redirect diff --git a/.changeset/light-eggs-hug.md b/.changeset/light-eggs-hug.md new file mode 100644 index 000000000..9825ea5a9 --- /dev/null +++ b/.changeset/light-eggs-hug.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Fix static redirects prefered over dynamic regular routes diff --git a/packages/astro/src/core/build/generate.ts b/packages/astro/src/core/build/generate.ts index b0ea26ab1..501d8ff26 100644 --- a/packages/astro/src/core/build/generate.ts +++ b/packages/astro/src/core/build/generate.ts @@ -619,9 +619,18 @@ async function generatePath( return; } const location = getRedirectLocationOrThrow(response.headers); + const fromPath = new URL(renderContext.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} +`; // A dynamic redirect, set the location so that integrations know about it. if (pageData.route.type !== 'redirect') { pageData.route.redirect = location; diff --git a/packages/astro/src/core/build/static-build.ts b/packages/astro/src/core/build/static-build.ts index 9bef0d681..2de49c487 100644 --- a/packages/astro/src/core/build/static-build.ts +++ b/packages/astro/src/core/build/static-build.ts @@ -32,6 +32,7 @@ import { RESOLVED_SPLIT_MODULE_ID, SSR_VIRTUAL_MODULE_ID } from './plugins/plugi import { ASTRO_PAGE_EXTENSION_POST_PATTERN } from './plugins/util.js'; import type { PageBuildData, StaticBuildOptions } from './types'; import { getTimeStat } from './util.js'; +import { routeIsRedirect } from '../redirects/index.js'; export async function viteBuild(opts: StaticBuildOptions) { const { allPages, settings } = opts; @@ -60,8 +61,10 @@ export async function viteBuild(opts: StaticBuildOptions) { // Track the page data in internals trackPageData(internals, component, pageData, astroModuleId, astroModuleURL); - pageInput.add(astroModuleId); - facadeIdToPageDataMap.set(fileURLToPath(astroModuleURL), pageData); + if(!routeIsRedirect(pageData.route)) { + pageInput.add(astroModuleId); + facadeIdToPageDataMap.set(fileURLToPath(astroModuleURL), pageData); + } } // Empty out the dist folder, if needed. Vite has a config for doing this diff --git a/packages/astro/src/core/routing/manifest/create.ts b/packages/astro/src/core/routing/manifest/create.ts index 7695ccaec..bb0fa4ba3 100644 --- a/packages/astro/src/core/routing/manifest/create.ts +++ b/packages/astro/src/core/routing/manifest/create.ts @@ -458,7 +458,28 @@ export function createRouteManifest( redirectRoute: routes.find((r) => r.route === to), }; - // Push so that redirects are selected last. + const lastSegmentIsDynamic = (r: RouteData) => !!r.segments.at(-1)?.at(-1)?.dynamic; + + const redirBase = path.posix.dirname(route); + const dynamicRedir = lastSegmentIsDynamic(routeData); + let i = 0; + for(const existingRoute of routes) { + // An exact match, prefer the page/endpoint. This matches hosts. + if(existingRoute.route === route) { + routes.splice(i+1, 0, routeData); + return; + } + + // If the existing route is dynamic, prefer the static redirect. + const base = path.posix.dirname(existingRoute.route); + if(base === redirBase && !dynamicRedir && lastSegmentIsDynamic(existingRoute)) { + routes.splice(i, 0, routeData); + return; + } + i++; + } + + // Didn't find a good place, insert last routes.push(routeData); }); diff --git a/packages/astro/test/fixtures/ssr-redirect/src/middleware.ts b/packages/astro/test/fixtures/ssr-redirect/src/middleware.ts index 5f8243a43..a8de8d130 100644 --- a/packages/astro/test/fixtures/ssr-redirect/src/middleware.ts +++ b/packages/astro/test/fixtures/ssr-redirect/src/middleware.ts @@ -5,7 +5,7 @@ export const onRequest = defineMiddleware(({ request }, next) => { return new Response(null, { status: 301, headers: { - 'Location': '/' + 'Location': '/test' } }); } diff --git a/packages/astro/test/fixtures/ssr-redirect/src/pages/index.astro b/packages/astro/test/fixtures/ssr-redirect/src/pages/test.astro similarity index 100% rename from packages/astro/test/fixtures/ssr-redirect/src/pages/index.astro rename to packages/astro/test/fixtures/ssr-redirect/src/pages/test.astro diff --git a/packages/astro/test/redirects.test.js b/packages/astro/test/redirects.test.js index 6cbf0fdfc..91ec19dfa 100644 --- a/packages/astro/test/redirects.test.js +++ b/packages/astro/test/redirects.test.js @@ -13,7 +13,7 @@ describe('Astro.redirect', () => { output: 'server', adapter: testAdapter(), redirects: { - '/api/redirect': '/', + '/api/redirect': '/test', }, experimental: { redirects: true, @@ -75,12 +75,13 @@ describe('Astro.redirect', () => { redirects: true, }, redirects: { - '/one': '/', - '/two': '/', + '/': '/test', + '/one': '/test', + '/two': '/test', '/blog/[...slug]': '/articles/[...slug]', '/three': { status: 302, - destination: '/', + destination: '/test', }, }, }); @@ -93,18 +94,44 @@ describe('Astro.redirect', () => { expect(html).to.include('url=/login'); }); + it('Includes the meta noindex tag', async () => { + const html = await fixture.readFile('/secret/index.html'); + expect(html).to.include('name="robots'); + expect(html).to.include('content="noindex'); + }); + + it('Includes a link to the new pages for bots to follow', async () => { + const html = await fixture.readFile('/secret/index.html'); + expect(html).to.include(''); + }); + + it('Includes a canonical link', async () => { + const html = await fixture.readFile('/secret/index.html'); + expect(html).to.include(''); + }); + + it('A 302 status generates a "temporary redirect" through a short delay', async () => { + // https://developers.google.com/search/docs/crawling-indexing/301-redirects#metarefresh + const html = await fixture.readFile('/secret/index.html'); + expect(html).to.include('content="2;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=/'); + expect(html).to.include('url=/test'); html = await fixture.readFile('/two/index.html'); expect(html).to.include('http-equiv="refresh'); - expect(html).to.include('url=/'); + expect(html).to.include('url=/test'); html = await fixture.readFile('/three/index.html'); expect(html).to.include('http-equiv="refresh'); - expect(html).to.include('url=/'); + expect(html).to.include('url=/test'); + + html = await fixture.readFile('/index.html'); + expect(html).to.include('http-equiv="refresh'); + expect(html).to.include('url=/test'); }); it('Generates page for dynamic routes', async () => { @@ -120,7 +147,7 @@ describe('Astro.redirect', () => { it('Generates redirect pages for redirects created by middleware', async () => { let html = await fixture.readFile('/middleware-redirect/index.html'); expect(html).to.include('http-equiv="refresh'); - expect(html).to.include('url=/'); + expect(html).to.include('url=/test'); }); }); diff --git a/packages/astro/test/units/routing/manifest.test.js b/packages/astro/test/units/routing/manifest.test.js index 7678ed77b..94ed3eb8a 100644 --- a/packages/astro/test/units/routing/manifest.test.js +++ b/packages/astro/test/units/routing/manifest.test.js @@ -4,6 +4,7 @@ import { createFs } from '../test-utils.js'; import { createRouteManifest } from '../../../dist/core/routing/manifest/create.js'; import { createDefaultDevSettings } from '../../../dist/core/config/index.js'; import { fileURLToPath } from 'url'; +import { defaultLogging } from '../test-utils.js'; const root = new URL('../../fixtures/alias/', import.meta.url); @@ -59,6 +60,31 @@ describe('routing - createRouteManifest', () => { expect(manifest.routes[1].route).to.equal('/blog/contributing'); expect(manifest.routes[1].type).to.equal('page'); - expect(manifest.routes[2].route).to.equal('/blog/[...slug]'); + expect(manifest.routes[3].route).to.equal('/blog/[...slug]'); + }); + + it('static redirect route is prioritized over dynamic file route', async () => { + const fs = createFs( + { + '/src/pages/[...slug].astro': `

test

` + }, + root + ); + const settings = await createDefaultDevSettings( + { + trailingSlash: 'never', + redirects: { + '/foo': '/bar' + } + }, + root + ); + const manifest = createRouteManifest({ + cwd: fileURLToPath(root), + settings, + fsMod: fs, + }, defaultLogging); + + expect(manifest.routes[0].route).to.equal('/foo'); }); });