summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGravatar Tony Sullivan <tony.f.sullivan@outlook.com> 2022-05-20 20:07:30 +0000
committerGravatar GitHub <noreply@github.com> 2022-05-20 20:07:30 +0000
commitd372d29ef8f540fca077f5e1318a2bbb5e7ec360 (patch)
tree7a253c8bac6f5afbc0651dfb002e909eccd0ebbf
parent1f148dbcfd10c1088cf7ec995aefe506961a9523 (diff)
downloadastro-d372d29ef8f540fca077f5e1318a2bbb5e7ec360.tar.gz
astro-d372d29ef8f540fca077f5e1318a2bbb5e7ec360.tar.zst
astro-d372d29ef8f540fca077f5e1318a2bbb5e7ec360.zip
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...
-rw-r--r--.changeset/quick-waves-act.md5
-rw-r--r--packages/astro/src/core/render/core.ts1
-rw-r--r--packages/astro/src/core/render/result.ts12
-rw-r--r--packages/astro/src/core/render/util.ts7
-rw-r--r--packages/astro/test/astro-get-static-paths.test.js34
-rw-r--r--packages/astro/test/fixtures/astro-get-static-paths/src/pages/posts/[page].astro29
6 files changed, 83 insertions, 5 deletions
diff --git a/.changeset/quick-waves-act.md b/.changeset/quick-waves-act.md
new file mode 100644
index 000000000..1eb7a53cd
--- /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 48af241da..b1a0fc81c 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 d0f7ef1b7..1c3b66268 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<string>;
site: string | undefined;
@@ -99,11 +102,16 @@ class Slots {
let renderMarkdown: any = null;
+function isPaginatedRoute({ page }: { page?: Page }) {
+ return page && 'currentPage' in page;
+}
+
export function createResult(args: CreateResultArgs): SSRResult {
- const { markdown, params, pathname, renderers, request, resolve, site } = args;
+ 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 ed8bc8c95..fa370c7c0 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 630134660..e5112139d 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 000000000..8d7a0c0c5
--- /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
+---
+
+<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>