From d372d29ef8f540fca077f5e1318a2bbb5e7ec360 Mon Sep 17 00:00:00 2001 From: Tony Sullivan Date: Fri, 20 May 2022 20:07:30 +0000 Subject: [PATCH] Fix: Only trim `/1` from the canonical URL for paginate() routes (#3393) * only trim /1 from canonical URLs for paginate() routes * chore: fixing eslint warning * chore: add changeset * typo: copy paste error * adding a test validation error message * verifying canonical for all three test routes * TEMP: extra test logging to track down the failure * TEMP: additional test logging to see what the failing CLI messages are * TEMP: digging deeper, it's getting stuck on port 3000 is taken * TEMP: why is it breaking when LOCAL isn't logged? * TEMP: still digging, strange how consistent this failure is * finally found it - the new test wasn't closing the dev server... --- .changeset/quick-waves-act.md | 5 +++ packages/astro/src/core/render/core.ts | 1 + packages/astro/src/core/render/result.ts | 14 ++++++-- packages/astro/src/core/render/util.ts | 7 ++-- .../astro/test/astro-get-static-paths.test.js | 34 ++++++++++++++++++- .../src/pages/posts/[page].astro | 29 ++++++++++++++++ 6 files changed, 84 insertions(+), 6 deletions(-) create mode 100644 .changeset/quick-waves-act.md create mode 100644 packages/astro/test/fixtures/astro-get-static-paths/src/pages/posts/[page].astro diff --git a/.changeset/quick-waves-act.md b/.changeset/quick-waves-act.md new file mode 100644 index 0000000000..1eb7a53cdf --- /dev/null +++ b/.changeset/quick-waves-act.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Fixes a bug in the canonical URL when using `1` as a route parameter in `getStaticPaths()` diff --git a/packages/astro/src/core/render/core.ts b/packages/astro/src/core/render/core.ts index 48af241da4..b1a0fc81ce 100644 --- a/packages/astro/src/core/render/core.ts +++ b/packages/astro/src/core/render/core.ts @@ -133,6 +133,7 @@ export async function render( markdown, origin, params, + props: pageProps, pathname, resolve, renderers, diff --git a/packages/astro/src/core/render/result.ts b/packages/astro/src/core/render/result.ts index d0f7ef1b79..1c3b66268a 100644 --- a/packages/astro/src/core/render/result.ts +++ b/packages/astro/src/core/render/result.ts @@ -2,7 +2,9 @@ import { bold } from 'kleur/colors'; import type { AstroGlobal, AstroGlobalPartial, + Page, Params, + Props, SSRElement, SSRLoadedRenderer, SSRResult, @@ -27,6 +29,7 @@ export interface CreateResultArgs { markdown: MarkdownRenderingOptions; params: Params; pathname: string; + props: Props; renderers: SSRLoadedRenderer[]; resolve: (s: string) => Promise; site: string | undefined; @@ -99,11 +102,16 @@ class Slots { let renderMarkdown: any = null; -export function createResult(args: CreateResultArgs): SSRResult { - const { markdown, params, pathname, renderers, request, resolve, site } = args; +function isPaginatedRoute({ page }: { page?: Page }) { + return page && 'currentPage' in page; +} +export function createResult(args: CreateResultArgs): SSRResult { + const { markdown, params, pathname, props: pageProps, renderers, request, resolve, site } = args; + + const paginated = isPaginatedRoute(pageProps); const url = new URL(request.url); - const canonicalURL = createCanonicalURL('.' + pathname, site ?? url.origin); + const canonicalURL = createCanonicalURL('.' + pathname, site ?? url.origin, paginated); const response: ResponseInit = { status: 200, statusText: 'OK', diff --git a/packages/astro/src/core/render/util.ts b/packages/astro/src/core/render/util.ts index ed8bc8c951..fa370c7c0d 100644 --- a/packages/astro/src/core/render/util.ts +++ b/packages/astro/src/core/render/util.ts @@ -3,9 +3,12 @@ import type { ModuleNode, ViteDevServer } from 'vite'; import type { Metadata } from '../../runtime/server/metadata.js'; /** Normalize URL to its canonical form */ -export function createCanonicalURL(url: string, base?: string): URL { +export function createCanonicalURL(url: string, base?: string, paginated?: boolean): URL { let pathname = url.replace(/\/index.html$/, ''); // index.html is not canonical - pathname = pathname.replace(/\/1\/?$/, ''); // neither is a trailing /1/ (impl. detail of collections) + // Only trim the first page's /1 param if Astro's paginated() was used + if (paginated) { + pathname = pathname.replace(/\/1\/?$/, ''); // neither is a trailing /1/ (impl. detail of collections) + } if (!npath.extname(pathname)) pathname = pathname.replace(/(\/+)?$/, '/'); // add trailing slash if there’s no extension pathname = pathname.replace(/\/+/g, '/'); // remove duplicate slashes (URL() won’t) return new URL(pathname, base); diff --git a/packages/astro/test/astro-get-static-paths.test.js b/packages/astro/test/astro-get-static-paths.test.js index 6301346600..e5112139d6 100644 --- a/packages/astro/test/astro-get-static-paths.test.js +++ b/packages/astro/test/astro-get-static-paths.test.js @@ -1,5 +1,6 @@ import { expect } from 'chai'; import { loadFixture } from './test-utils.js'; +import * as cheerio from 'cheerio'; describe('getStaticPaths - build calls', () => { before(async () => { @@ -59,7 +60,7 @@ describe('getStaticPaths - route params type validation', () => { devServer = await fixture.startDevServer(); }); - it('resolves 200 on mathcing static path - string params', async () => { + it('resolves 200 on matching static path - string params', async () => { // route provided with { params: { year: "2022", slug: "post-2" }} const res = await fixture.fetch('/blog/2022/post-1'); expect(res.status).to.equal(200); @@ -71,3 +72,34 @@ describe('getStaticPaths - route params type validation', () => { expect(res.status).to.equal(200); }); }); + +describe ('getStaticPaths - numeric route params', () => { + let fixture; + let devServer; + + before(async () => { + fixture = await loadFixture({ + root: './fixtures/astro-get-static-paths/', + site: 'https://mysite.dev/' + }); + devServer = await fixture.startDevServer(); + }); + + after(async () => { + await devServer.stop(); + }); + + it('resolves 200 on matching static paths', async () => { + // routes params provided for pages /posts/1, /posts/2, and /posts/3 + for (const page of [1, 2, 3]) { + let res = await fixture.fetch(`/posts/${page}`); + expect(res.status).to.equal(200); + + const html = await res.text(); + const $ = cheerio.load(html); + + const canonical = $('link[rel=canonical]'); + expect(canonical.attr('href')).to.equal(`https://mysite.dev/posts/${page}/`, `doesn't trim the /${page}/ route param`); + } + }); +}); diff --git a/packages/astro/test/fixtures/astro-get-static-paths/src/pages/posts/[page].astro b/packages/astro/test/fixtures/astro-get-static-paths/src/pages/posts/[page].astro new file mode 100644 index 0000000000..8d7a0c0c5b --- /dev/null +++ b/packages/astro/test/fixtures/astro-get-static-paths/src/pages/posts/[page].astro @@ -0,0 +1,29 @@ +--- +export async function getStaticPaths() { + return [ + { + params: { page: 1 }, + }, + { + params: { page: 2 }, + }, + { + params: { page: 3 } + } + ] +}; + +const { page } = Astro.params +--- + + + + + + Posts Page {page} + + + +

Welcome to page {page}

+ +