summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGravatar Matthew Phillips <matthew@skypack.dev> 2023-07-13 16:09:44 -0400
committerGravatar GitHub <noreply@github.com> 2023-07-13 16:09:44 -0400
commit213e10991af337a7c4fd38c39be5c266c16fa600 (patch)
tree0754d988c34017c75343a592300227137995eca1
parentaf5827d4f7af9437c0c3fcff5c0239577aa68498 (diff)
downloadastro-213e10991af337a7c4fd38c39be5c266c16fa600.tar.gz
astro-213e10991af337a7c4fd38c39be5c266c16fa600.tar.zst
astro-213e10991af337a7c4fd38c39be5c266c16fa600.zip
Fixes for redirects config (#7644)
* Update redirects static generation based on recs Got some great recommendations on how to handle our HTML written redirect code based on SEO best practices. See https://github.com/withastro/roadmap/issues/466#issuecomment-1595940678 This implements them all. * Fix for using the root path / as a redirect Fixes https://github.com/withastro/astro/issues/7478 * Fix static redirects prefer over dynamic page Fixes https://github.com/withastro/astro/issues/7581
Diffstat
-rw-r--r--.changeset/four-pumpkins-try.md5
-rw-r--r--.changeset/light-eggs-hug.md5
-rw-r--r--packages/astro/src/core/build/generate.ts11
-rw-r--r--packages/astro/src/core/build/static-build.ts7
-rw-r--r--packages/astro/src/core/routing/manifest/create.ts23
-rw-r--r--packages/astro/test/fixtures/ssr-redirect/src/middleware.ts2
-rw-r--r--packages/astro/test/fixtures/ssr-redirect/src/pages/test.astro (renamed from packages/astro/test/fixtures/ssr-redirect/src/pages/index.astro)0
-rw-r--r--packages/astro/test/redirects.test.js43
-rw-r--r--packages/astro/test/units/routing/manifest.test.js28
9 files changed, 110 insertions, 14 deletions
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 = `<!doctype html>
<title>Redirecting to: ${location}</title>
-<meta http-equiv="refresh" content="0;url=${location}" />`;
+<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>`;
// 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
index e06d49b85..e06d49b85 100644
--- a/packages/astro/test/fixtures/ssr-redirect/src/pages/index.astro
+++ b/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('<a href="/login">');
+ });
+
+ it('Includes a canonical link', async () => {
+ const html = await fixture.readFile('/secret/index.html');
+ expect(html).to.include('<link rel="canonical" href="/login">');
+ });
+
+ 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': `<h1>test</h1>`
+ },
+ 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');
});
});
generated by cgit v1.2.3 (git 2.46.0) at 2025-08-14 04:18:17 +0000