Make status code check more strict

withastro · Apr 12, 2024 · 1f67782 · 1f67782
1 parent 3ff6b40
commit 1f67782
Show file tree

Hide file tree

Showing 5 changed files with 49 additions and 10 deletions.
diff --git a/.changeset/nasty-turtles-obey.md b/.changeset/nasty-turtles-obey.md
@@ -0,0 +1,5 @@
+---
+"@astrojs/sitemap": patch
+---
+
+Site map status code exclusion now only excludes pages like /[error-code] or /[locale]/[error-code]
diff --git a/packages/integrations/sitemap/src/index.ts b/packages/integrations/sitemap/src/index.ts
@@ -7,6 +7,7 @@ import { ZodError } from 'zod';
 
 import { generateSitemap } from './generate-sitemap.js';
 import { validateOptions } from './validate-options.js';
+import { STATUS_CODES } from 'node:http';
 
 export { EnumChangefreq as ChangeFreqEnum } from 'sitemap';
 export type ChangeFreq = `${EnumChangefreq}`;
@@ -47,14 +48,23 @@ const PKG_NAME = '@astrojs/sitemap';
 const OUTFILE = 'sitemap-index.xml';
 const STATUS_CODE_PAGES = new Set(['404', '500']);
 
-function isStatusCodePage(pathname: string): boolean {
-	if (pathname.endsWith('/')) {
-		pathname = pathname.slice(0, -1);
+const isStatusCodePage = (locales: string[]) => {
+	const statusPathNames = new Set(
+		locales
+		.flatMap(locale => [...STATUS_CODE_PAGES].map(page => `${locale}/${page}`))
+		.concat([...STATUS_CODE_PAGES])
+	);
+
+	return (pathname: string): boolean => {
+		if (pathname.endsWith('/')) {
+			pathname = pathname.slice(0, -1);
+		}
+		if (pathname.startsWith('/')) {
+			pathname = pathname.slice(1);
+		}
+		return statusPathNames.has(pathname);
 	}
-	const end = pathname.split('/').pop() ?? '';
-	return STATUS_CODE_PAGES.has(end);
 }
-
 const createPlugin = (options?: SitemapOptions): AstroIntegration => {
 	let config: AstroConfig;
 
@@ -88,9 +98,9 @@ const createPlugin = (options?: SitemapOptions): AstroIntegration => {
 						);
 						return;
 					}
-
+					const shouldIgnoreStatus = isStatusCodePage(Object.keys(opts.i18n?.locales ?? {}));
 					let pageUrls = pages
-						.filter((p) => !isStatusCodePage(p.pathname))
+						.filter((p) => !shouldIgnoreStatus(p.pathname))
 						.map((p) => {
 							if (p.pathname !== '' && !finalSiteUrl.pathname.endsWith('/'))
 								finalSiteUrl.pathname += '/';
@@ -107,7 +117,7 @@ const createPlugin = (options?: SitemapOptions): AstroIntegration => {
 						 * Dynamic URLs have entries with `undefined` pathnames
 						 */
 						if (r.pathname) {
-							if (isStatusCodePage(r.pathname ?? r.route)) return urls;
+							if (shouldIgnoreStatus(r.pathname ?? r.route)) return urls;
 
 							// `finalSiteUrl` may end with a trailing slash
 							// or not because of base paths.

diff --git a/packages/integrations/sitemap/test/fixtures/static/astro.config.mjs b/packages/integrations/sitemap/test/fixtures/static/astro.config.mjs
@@ -2,7 +2,15 @@ import sitemap from '@astrojs/sitemap';
 import { defineConfig } from 'astro/config';
 
 export default defineConfig({
-  integrations: [sitemap()],
+  integrations: [sitemap({
+      i18n: {
+          defaultLocale: 'it',
+          locales: {
+              it: 'it-IT',
+              de: 'de-DE',
+          }
+      }
+  })],
 	site: 'http://example.com',
       redirects: {
         '/redirect': '/'

diff --git a/packages/integrations/sitemap/test/fixtures/static/src/pages/products-by-id/[id].astro b/packages/integrations/sitemap/test/fixtures/static/src/pages/products-by-id/[id].astro
@@ -0,0 +1,11 @@
+---
+export async function getStaticPaths() {
+    return [
+        { params: { id: '404' } },
+        { params: { id: '405' } },
+    ]
+}
+
+const { id } = Astro.params
+---
+<!DOCTYPE html><html> <head><title>Product #{id}</title></head> <body> <h1>Product #404</h1> <p>This is a product that just happens to have an ID of {id}. It is found!</p></body></html>
diff --git a/packages/integrations/sitemap/test/staticPaths.test.js b/packages/integrations/sitemap/test/staticPaths.test.js
@@ -36,6 +36,11 @@ describe('getStaticPaths support', () => {
 		assert.equal(urls.includes('http://example.com/123/'), true);
 	});
 
+	it('includes numerical 404 pages if not for i18n', () => {
+		assert.equal(urls.includes('http://example.com/products-by-id/405/'), true);
+		assert.equal(urls.includes('http://example.com/products-by-id/404/'), true);
+	});
+
 	it('should render the endpoint', async () => {
 		const page = await fixture.readFile('./it/manifest');
 		assert.match(page, /I'm a route in the "it" language./);