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

@@ -0,0 +1,5 @@
+---
+'astro': patch
+---
+
+Fixes a bug in the canonical URL when using `1` as a route parameter in `getStaticPaths()`

packages/astro/src/core/render/core.ts

@@ -133,6 +133,7 @@ export async function render(
 		markdown,
 		origin,
 		params,
+		props: pageProps,
 		pathname,
 		resolve,
 		renderers,

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<string>;
 	site: string | undefined;
@@ -99,11 +102,16 @@ class Slots {
 
 let renderMarkdown: any = null;
 
-export function createResult(args: CreateResultArgs): SSRResult {
+function isPaginatedRoute({ page }: { page?: Page }) {
-	const { markdown, params, pathname, renderers, request, resolve, site } = args;
+	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',

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);

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`);
+		}
+	});
+});

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
+---
+
+<html lang="en">
+	<head>
+		<meta charset="utf-8" />
+		<meta name="viewport" content="width=device-width" />
+		<title>Posts Page {page}</title>
+		<link rel="canonical" href={Astro.canonicalURL.href}>
+	</head>
+	<body>
+		<h1>Welcome to page {page}</h1>
+	</body>
+</html>