From cefeadf0a4a51420130445b6dc5ab1e5b331732b Mon Sep 17 00:00:00 2001 From: Adrian Lyjak Date: Tue, 21 May 2024 07:01:35 -0400 Subject: [PATCH] Make status code check more strict for sitemap plugin (#10779) Co-authored-by: Ben Holmes Co-authored-by: Florian Lefebvre --- .changeset/nasty-turtles-obey.md | 5 +++ packages/integrations/sitemap/src/index.ts | 31 ++++++++++++------- .../test/fixtures/static/astro.config.mjs | 10 +++++- .../src/pages/products-by-id/[id].astro | 11 +++++++ .../sitemap/test/staticPaths.test.js | 5 +++ 5 files changed, 50 insertions(+), 12 deletions(-) create mode 100644 .changeset/nasty-turtles-obey.md create mode 100644 packages/integrations/sitemap/test/fixtures/static/src/pages/products-by-id/[id].astro diff --git a/.changeset/nasty-turtles-obey.md b/.changeset/nasty-turtles-obey.md new file mode 100644 index 000000000000..210322a73626 --- /dev/null +++ b/.changeset/nasty-turtles-obey.md @@ -0,0 +1,5 @@ +--- +"@astrojs/sitemap": patch +--- + +Fixes false positives for status code routes like `404` and `500` when generating sitemaps. diff --git a/packages/integrations/sitemap/src/index.ts b/packages/integrations/sitemap/src/index.ts index 36ecc8eac677..622ce958ff10 100644 --- a/packages/integrations/sitemap/src/index.ts +++ b/packages/integrations/sitemap/src/index.ts @@ -47,14 +47,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 end = pathname.split('/').pop() ?? ''; - return STATUS_CODE_PAGES.has(end); -} - +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 createPlugin = (options?: SitemapOptions): AstroIntegration => { let config: AstroConfig; @@ -88,9 +97,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 +116,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 index 15787abb1828..ae53a9342c46 100644 --- 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 new file mode 100644 index 000000000000..b10438a681b6 --- /dev/null +++ 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 +--- + Product #{id}

Product #404

This is a product that just happens to have an ID of {id}. It is found!

\ No newline at end of file diff --git a/packages/integrations/sitemap/test/staticPaths.test.js b/packages/integrations/sitemap/test/staticPaths.test.js index abc687faf357..7df9d5cb6ac7 100644 --- 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./);