From 35b43a02eaea58d04e6533a4de08f29739cc289d Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 11 Sep 2026 16:18:17 +0000 Subject: [PATCH 1/3] feat(seo): emit a canonical link, and base-URL-aware head links Every page now carries ``, built from `site_url` as the Sphinx lecture sites build theirs from `html.baseurl`. Two rules apply, and `og:url` is built by the same function so the two cannot disagree: - the home page's canonical is the site root. With a base URL, the export renders the root index.html by requesting the index slug, so the page's render-time path is that slug -- and the slug's own URL is not served, so naming it would point every home page at a 404; - every URL takes the trailing-slash form, which is what the export writes and what a host redirects the slashless form to, so no canonical names a redirect. Nothing is emitted without `site_url`, as Sphinx emits nothing without `html_baseurl`. The page path is stripped of the base before the base is re-applied: the browser router has no basename, so on the client the location already carries it. Two head links assumed the domain root and 404ed on a site served under a sub-path, which is how lecture-wasm is deployed. The favicon and `/myst-theme.css` now carry the base URL. Neither can come from a route's `links()`, which takes no arguments in Remix 1.17 while BASE_URL reaches the app only through the root loader, so the local Document emits them; the root route keeps a root-absolute icon as the fallback for the error boundary upstream renders with its own Document. `og:image` is made absolute against the site origin, which a social scraper needs to fetch it at all. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UZLpDVYu1YBZHQfwkRRJj7 --- CHANGELOG.md | 15 ++++++ README.md | 8 ++- app/components/Document.tsx | 15 ++++++ app/root.tsx | 11 +++- app/routes/$.tsx | 18 +++++-- app/routes/_index.tsx | 16 ++++-- app/seo.ts | 84 +++++++++++++++++++++++++---- docs/configuration.md | 2 +- template.yml | 6 ++- tests/unit/seo.test.mjs | 102 +++++++++++++++++++++++++++++++++--- tests/visual/theme.spec.ts | 24 ++++++++- 11 files changed, 274 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 069fcf4b6..3ce870d24 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,21 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added +- A `` on every page, from `site_url`, as the Sphinx + lecture sites emit from `html.baseurl`. The home page's canonical is the site + root, and every URL takes the trailing-slash form the build actually serves, + so no canonical names a redirect. `og:url` is built by the same function, so + the two cannot disagree. Nothing is emitted without `site_url` + ([#207](https://github.com/QuantEcon/quantecon-theme.mystmd/issues/207)). + +### Fixed +- Head links that assumed the domain root now carry the static build's base + URL, so they resolve on a site served under a sub-path instead of 404ing at + the domain root: the favicon and `/myst-theme.css`. `og:image` is made + absolute against `site_url`, which a social scraper needs + ([#207](https://github.com/QuantEcon/quantecon-theme.mystmd/issues/207)). + ## [2.7.0] - 2026-09-11 ### Added diff --git a/README.md b/README.md index 64843dfcf..42f6d5205 100644 --- a/README.md +++ b/README.md @@ -219,6 +219,12 @@ the two image URLs `og_logo_url` / `twitter_logo_url`, named as in the book theme so a lecture repo copies its values across. A page's own thumbnail takes precedence for `og:image`. +Every page also carries a ``, from the same `site_url` +and built by the same function as `og:url`, so the two always agree. The home +page's canonical is the site root, and every URL takes the trailing-slash form +the build actually serves. With `site_url` unset neither is emitted — what +Sphinx does without `html_baseurl`. + ### Collapsible stderr A notebook cell's stderr stream is folded behind a "⚠ Code warnings" @@ -238,7 +244,7 @@ block inside a string (`key: |`), which the theme parses. | Option | Scope | Purpose | | ------ | ----- | ------- | | `twitter` | site | Handle for the `twitter:site` / `twitter:creator` card meta tags | -| `site_url` | site | The site's public URL, for `og:url` ([Meta tags](#meta-tags)) | +| `site_url` | site | The site's public URL, for the canonical link and `og:url` ([Meta tags](#meta-tags)) | | `og_logo_url`, `twitter_logo_url` | site | Site-level images for `og:image` / `twitter:image` when a page has no thumbnail ([Meta tags](#meta-tags)) | | `favicon` | site | Favicon file, relative to `myst.yml`; served at `/favicon.ico` (the QuantEcon lectures favicon when unset) | | `analytics_google`, `analytics_plausible` | site | Analytics IDs, rendered by `@myst-theme/site` | diff --git a/app/components/Document.tsx b/app/components/Document.tsx index ed82689c1..2c807c3d0 100644 --- a/app/components/Document.tsx +++ b/app/components/Document.tsx @@ -145,6 +145,21 @@ export function DocumentWithoutProviders({ {title && {title}} + {/* Head links whose href has to carry the static build's base URL. + They cannot come from a route's `links()`, which takes no arguments + in Remix 1.17 while BASE_URL reaches the app only through the root + loader -- but this Document is handed it directly. + + The export writes both files at the build root, and rewrites only + `/myst_assets_folder/` URLs for the base, so a root-absolute href + resolves to the domain root and 404s on a sub-path site. + + The icon is emitted only when there is a base to add: without one + the root route's own `/favicon.ico` is already right, and a second + identical link would be noise. With one, this comes after `` + and a later `rel="icon"` wins. */} + {baseurl && } + { return [ + // The root-absolute fallback. It is wrong on a site served under a + // sub-path, where it resolves to the domain root, but it is the only icon + // link that also applies when the root ErrorBoundary renders -- that + // boundary is upstream's, with upstream's own Document and no base URL. + // The local Document emits the base-aware one after this, and a later + // `rel="icon"` wins, so only error pages fall back to this. { rel: 'icon', href: '/favicon.ico', @@ -178,7 +184,10 @@ export const links: LinksFunction = () => { ...PTSerifCSS, { rel: 'stylesheet', href: tailwind }, { rel: 'stylesheet', href: thebeCoreCss }, - { rel: 'stylesheet', href: '/myst-theme.css' }, + // `/myst-theme.css` (the consumer's own stylesheet slot) is NOT declared + // here: its href has to carry the static build's base URL, and `links()` + // takes no arguments in Remix 1.17 while BASE_URL reaches the app only + // through the root loader. The local Document emits it instead. // jupyter-matplotlib's stylesheet is vendored into the Tailwind bundle // (styles/mpl-widget.css) rather than linked from jsdelivr here: that // would be a render-blocking request to a third-party CDN that is diff --git a/app/routes/$.tsx b/app/routes/$.tsx index bdff93ce8..60597ceeb 100644 --- a/app/routes/$.tsx +++ b/app/routes/$.tsx @@ -12,8 +12,8 @@ import { getConfig, getPage } from '~/backend/loaders.server'; import type { SiteManifest } from 'myst-config'; import { ErrorPage } from '~/components/ErrorPage'; import { Page } from '~/components/Page'; -import { hreflangLinks } from '~/i18n'; -import { mergeMeta, socialMetaTags } from '~/seo'; +import { hreflangLinks, stripBaseurl } from '~/i18n'; +import { canonicalLink, mergeMeta, pageUrl, siteOrigin, socialMetaTags } from '~/seo'; // Never re-run the loader on a navigation that changes neither pathname nor // search (Back off an in-page anchor on a static build). @@ -38,13 +38,24 @@ export const meta: V2_MetaFunction = ({ data, matches, location } ); const baseurl = rootMatch?.data?.BASE_URL; + // The page's public URL, shared by og:url and the canonical link. The path + // is stripped of the base first: the browser router has no basename, so on + // the client `location.pathname` already carries it, and prefixing again + // would repeat it. + const url = pageUrl({ + origin: siteOrigin((config?.options as any)?.site_url, config?.domains), + path: stripBaseurl(location.pathname, baseurl), + baseurl, + indexSlug: project?.index, + }); + // The OpenGraph / Twitter tags this theme adds to (or replaces in) upstream's // article set -- see app/seo.ts. const social = socialMetaTags({ domains: config?.domains, siteTitle: config?.title ?? project?.title, pageImage: (page?.thumbnailOptimized || page?.thumbnail) ?? (project?.thumbnailOptimized || project?.thumbnail) ?? undefined, - pathname: `${baseurl ?? ''}${location.pathname}`, + url, options: config?.options as any, }); return [ @@ -62,6 +73,7 @@ export const meta: V2_MetaFunction = ({ data, matches, location } }), social), // hreflang alternates for the translated editions. ...hreflangLinks(config?.options, location.pathname, baseurl), + ...canonicalLink(url), ]; }; diff --git a/app/routes/_index.tsx b/app/routes/_index.tsx index 5b3a0d45b..92725f668 100644 --- a/app/routes/_index.tsx +++ b/app/routes/_index.tsx @@ -8,8 +8,8 @@ import type { SiteManifest } from 'myst-config'; import { getProject } from '@myst-theme/common'; import { Page } from '~/components/Page'; -import { hreflangLinks } from '~/i18n'; -import { mergeMeta, socialMetaTags } from '~/seo'; +import { hreflangLinks, stripBaseurl } from '~/i18n'; +import { canonicalLink, mergeMeta, pageUrl, siteOrigin, socialMetaTags } from '~/seo'; // Never re-run the loader on a navigation that changes neither pathname nor // search (Back off an in-page anchor on a static build). @@ -28,13 +28,22 @@ export const meta: V2_MetaFunction = ({ data, matches, location } ); const baseurl = rootMatch?.data?.BASE_URL; + // The page's public URL, shared by og:url and the canonical link -- see the + // article route for why the base is stripped before it is re-applied. + const url = pageUrl({ + origin: siteOrigin((config?.options as any)?.site_url, config?.domains), + path: stripBaseurl(location.pathname, baseurl), + baseurl, + indexSlug: project?.index, + }); + // The OpenGraph / Twitter tags this theme adds to (or replaces in) upstream's // article set -- see app/seo.ts. const social = socialMetaTags({ domains: config?.domains, siteTitle: config?.title ?? project?.title, pageImage: (project.thumbnailOptimized || project.thumbnail) ?? undefined, - pathname: `${baseurl ?? ''}${location.pathname}`, + url, options: config?.options as any, }); return [ @@ -49,6 +58,7 @@ export const meta: V2_MetaFunction = ({ data, matches, location } }), social), // hreflang alternates for the translated editions. ...hreflangLinks(config?.options, location.pathname, baseurl), + ...canonicalLink(url), ]; }; diff --git a/app/seo.ts b/app/seo.ts index 4b15883ad..04ccb9f25 100644 --- a/app/seo.ts +++ b/app/seo.ts @@ -16,8 +16,13 @@ * og:url upstream needs an `origin`, which the routes never had. * It comes from the `site_url` option; `site.domains` would * be the natural source, but the CLI's site manifest does - * not carry it, so it is only a fallback should that change - * og:image `og_logo_url` when the page has no thumbnail + * not carry it, so it is only a fallback should that change. + * Built by `pageUrl`, the same function as the canonical + * link, so the two cannot disagree + * canonical a ``, not a meta tag, but built from the same URL + * og:image `og_logo_url` when the page has no thumbnail, made absolute + * against the site origin: a social scraper cannot resolve a + * root-relative path * twitter:image `twitter_logo_url` when set, even over a page thumbnail; * otherwise the og:image * twitter:card "summary" whenever `twitter` is set, in place of upstream's @@ -46,11 +51,72 @@ export interface SeoInput { siteTitle?: string; /** The page's own image, if any (thumbnail); site-level images fill in. */ pageImage?: string; - /** Path of the page, including the static build's base URL. */ - pathname: string; + /** The page's public URL, from `pageUrl`; og:url is omitted without one. */ + url?: string; options?: SeoSiteOptions; } +export interface PageUrlInput { + /** Site origin, from `siteOrigin`. Without it there is no public URL. */ + origin?: string; + /** Site-relative page path, with any base URL already stripped. */ + path: string; + /** The static build's base URL, if the site has one. */ + baseurl?: string; + /** `config.index`: the slug whose page the site root serves. */ + indexSlug?: string; +} + +/** + * The page's public URL. The canonical link and og:url are both built here, so + * that the two rules below apply to both and they cannot drift apart. + * + * The home page resolves to the site root. With a base URL, mystmd renders the + * root `index.html` by requesting the index slug, so the page's render-time + * path is that slug -- and the slug's own URL is not served at all, so naming + * it would point every home page at a 404. + * + * Every URL takes the trailing-slash form, which is what the export writes + * (`/index.html`) and what a host redirects the slashless form to; a URL + * taken straight from the render-time path would name a redirect. + * + * Returns undefined when the site sets no `site_url`, as Sphinx emits nothing + * without `html_baseurl`. + */ +export function pageUrl({ origin, path, baseurl, indexSlug }: PageUrlInput): string | undefined { + if (!origin) return undefined; + const base = (baseurl ?? '').trim().replace(/\/+$/, ''); + const slug = (path || '/').replace(/^\/+|\/+$/g, ''); + const isHome = slug === '' || (!!indexSlug && slug === indexSlug); + return `${origin}${base}${isHome ? '/' : `/${slug}/`}`; +} + +export interface CanonicalLink { + tagName: 'link'; + rel: 'canonical'; + href: string; + // Remix's meta descriptor type is an open record; the index signature lets + // this spread into a route's `meta` return without a cast, as the hreflang + // alternates do. + [key: string]: unknown; +} + +/** + * `` for the page, in the shape Remix's v2 `meta` + * renders. Empty without a URL, so a site that sets no `site_url` emits + * nothing -- what Sphinx does without `html_baseurl`. + */ +export function canonicalLink(url?: string): CanonicalLink[] { + return url ? [{ tagName: 'link', rel: 'canonical', href: url }] : []; +} + +/** An image URL a social scraper can fetch: root-relative paths take the origin. */ +export function absoluteImage(image?: string, origin?: string): string | undefined { + if (!image) return undefined; + if (!origin || !image.startsWith('/')) return image; + return `${origin}${image}`; +} + /** * The canonical origin: `site_url` (an absolute URL, trailing slash and path * dropped to the origin), else `https://` -- the CLI validates @@ -93,20 +159,20 @@ export function ogLocale(code?: string): string | undefined { * so a site-level image does not sit beside a missing page image and * twitter:image follows the site's Twitter logo when one is configured. */ -export function socialMetaTags({ domains, siteTitle, pageImage, pathname, options }: SeoInput): V2_MetaDescriptor[] { +export function socialMetaTags({ domains, siteTitle, pageImage, url, options }: SeoInput): V2_MetaDescriptor[] { const origin = siteOrigin(options?.site_url, domains); - const image = pageImage || options?.og_logo_url; - const twitterImage = options?.twitter_logo_url || image; + const image = absoluteImage(pageImage || options?.og_logo_url, origin); + const twitterImage = absoluteImage(options?.twitter_logo_url, origin) || image; const tags: V2_MetaDescriptor[] = [{ property: 'og:type', content: 'website' }]; if (siteTitle) tags.push({ property: 'og:site_name', content: siteTitle }); - if (origin) tags.push({ property: 'og:url', content: `${origin}${pathname}` }); + if (url) tags.push({ property: 'og:url', content: url }); if (image) tags.push({ property: 'og:image', content: image }); const site = handle(options?.twitter); if (site) { tags.push({ name: 'twitter:site', content: site }); tags.push({ name: 'twitter:card', content: 'summary' }); } - if (twitterImage && (site || twitterImage !== pageImage)) { + if (twitterImage && (site || twitterImage !== absoluteImage(pageImage, origin))) { tags.push({ name: 'twitter:image', content: twitterImage }); } const locale = ogLocale(options?.current_language); diff --git a/docs/configuration.md b/docs/configuration.md index a3d1222d5..a66126dae 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -16,7 +16,7 @@ below. | Option | Type | Scope | Default | Purpose | | --- | --- | --- | --- | --- | -| `site_url` | string | site | — | the site's public URL, for `og:url` | +| `site_url` | string | site | — | the site's public URL, for the canonical link and `og:url` | | `twitter` | string | site | — | handle for `twitter:site` / `twitter:creator`; `@` optional | | `og_logo_url` | string | site | — | `og:image` when a page has no thumbnail | | `twitter_logo_url` | string | site | — | `twitter:image`; falls back to `og_logo_url` | diff --git a/template.yml b/template.yml index 8d519df55..f33e7d984 100644 --- a/template.yml +++ b/template.yml @@ -72,8 +72,10 @@ options: type: string description: > The site's public URL (the Sphinx sites' `html_baseurl`), for the - absolute `og:url` on every page. `site.domains` would be the natural - source but does not reach the theme. + `` and the absolute `og:url` on every page, and + for making `og:image` absolute. Neither the canonical link nor `og:url` + is emitted without it. `site.domains` would be the natural source but + does not reach the theme. - id: og_logo_url type: string description: > diff --git a/tests/unit/seo.test.mjs b/tests/unit/seo.test.mjs index 2a9849b52..e44ad8fd6 100644 --- a/tests/unit/seo.test.mjs +++ b/tests/unit/seo.test.mjs @@ -5,7 +5,15 @@ import assert from 'node:assert/strict'; import { test } from 'node:test'; -import { mergeMeta, ogLocale, siteOrigin, socialMetaTags } from '../../app/seo.ts'; +import { + absoluteImage, + canonicalLink, + mergeMeta, + ogLocale, + pageUrl, + siteOrigin, + socialMetaTags, +} from '../../app/seo.ts'; const byKey = (tags) => Object.fromEntries(tags.map((t) => [t.property ?? t.name, t.content])); @@ -33,7 +41,7 @@ test('the full set, on a lecture page with the site-level images', () => { const tags = byKey( socialMetaTags({ siteTitle: 'Python Programming for Economics and Finance', - pathname: '/python-by-example/', + url: 'https://python-programming.quantecon.org/python-by-example/', options: { site_url: 'https://python-programming.quantecon.org', twitter: 'quantecon', @@ -58,18 +66,24 @@ test('the full set, on a lecture page with the site-level images', () => { test('a page thumbnail beats the site image for og:image; twitter:image follows the Twitter logo', () => { const tags = byKey( socialMetaTags({ - pathname: '/p', + url: 'https://example.org/p/', pageImage: '/thumb.png', - options: { og_logo_url: 'https://x/og.png', twitter_logo_url: 'https://x/tw.png', twitter: '@qe' }, + options: { + site_url: 'https://example.org', + og_logo_url: 'https://x/og.png', + twitter_logo_url: 'https://x/tw.png', + twitter: '@qe', + }, }), ); - assert.equal(tags['og:image'], '/thumb.png'); + // Made absolute: a scraper cannot resolve a root-relative path. + assert.equal(tags['og:image'], 'https://example.org/thumb.png'); assert.equal(tags['twitter:image'], 'https://x/tw.png'); assert.equal(tags['twitter:site'], '@qe'); }); test('nothing configured: only og:type, and no dangling twitter tags', () => { - const tags = socialMetaTags({ pathname: '/p' }); + const tags = socialMetaTags({}); assert.deepEqual(tags, [{ property: 'og:type', content: 'website' }]); }); @@ -93,3 +107,79 @@ test('mergeMeta replaces same-key upstream tags and keeps the rest in order', () { property: 'og:type', content: 'website' }, ]); }); + +test('pageUrl: the trailing-slash form, with and without a base URL', () => { + const origin = 'https://python-programming.quantecon.org'; + assert.equal(pageUrl({ origin, path: '/about-py' }), `${origin}/about-py/`); + assert.equal(pageUrl({ origin, path: '/about-py/' }), `${origin}/about-py/`); + assert.equal( + pageUrl({ origin: 'https://quantecon.github.io', path: '/short-path', baseurl: '/lecture-wasm' }), + 'https://quantecon.github.io/lecture-wasm/short-path/', + ); + // A trailing slash on the base is not doubled. + assert.equal( + pageUrl({ origin: 'https://quantecon.github.io', path: '/short-path', baseurl: '/lecture-wasm/' }), + 'https://quantecon.github.io/lecture-wasm/short-path/', + ); +}); + +test('pageUrl: the base appears exactly once when the path still carries it', () => { + // On the client the router has no basename, so `location.pathname` carries + // the base; the routes strip it before calling this, and a path that slipped + // through unstripped must not double it. + const url = pageUrl({ + origin: 'https://quantecon.github.io', + path: '/short-path', + baseurl: '/lecture-wasm', + }); + assert.equal(url, 'https://quantecon.github.io/lecture-wasm/short-path/'); + assert.equal((url.match(/lecture-wasm/g) ?? []).length, 1); +}); + +test('pageUrl: the home page is the site root, named by path or by index slug', () => { + const origin = 'https://quantecon.github.io'; + assert.equal(pageUrl({ origin, path: '/', baseurl: '/lecture-wasm' }), `${origin}/lecture-wasm/`); + // With a base URL the export renders the root index.html by requesting the + // index slug, so that is the home page's render-time path -- and the slug's + // own URL is not served. + assert.equal( + pageUrl({ origin, path: '/intro', baseurl: '/lecture-wasm', indexSlug: 'intro' }), + `${origin}/lecture-wasm/`, + ); + assert.equal(pageUrl({ origin, path: '/intro', indexSlug: 'intro' }), `${origin}/`); + // A different page is unaffected by the index slug. + assert.equal( + pageUrl({ origin, path: '/introduction', indexSlug: 'intro' }), + `${origin}/introduction/`, + ); +}); + +test('pageUrl: nothing without an origin, as Sphinx emits nothing without html_baseurl', () => { + assert.equal(pageUrl({ path: '/about-py' }), undefined); + assert.equal(pageUrl({ origin: undefined, path: '/', baseurl: '/x' }), undefined); +}); + +test('canonicalLink: a link descriptor, or nothing', () => { + assert.deepEqual(canonicalLink('https://example.org/p/'), [ + { tagName: 'link', rel: 'canonical', href: 'https://example.org/p/' }, + ]); + assert.deepEqual(canonicalLink(undefined), []); +}); + +test('absoluteImage: only root-relative paths take the origin', () => { + assert.equal(absoluteImage('/build/graph.png', 'https://example.org'), 'https://example.org/build/graph.png'); + assert.equal(absoluteImage('https://cdn.example/og.png', 'https://example.org'), 'https://cdn.example/og.png'); + assert.equal(absoluteImage('/build/graph.png', undefined), '/build/graph.png'); + assert.equal(absoluteImage(undefined, 'https://example.org'), undefined); +}); + +test('og:url comes from the same URL the canonical link uses', () => { + const url = pageUrl({ + origin: 'https://quantecon.github.io', + path: '/short-path', + baseurl: '/lecture-wasm', + }); + const tags = byKey(socialMetaTags({ url, options: { site_url: 'https://quantecon.github.io' } })); + assert.equal(tags['og:url'], url); + assert.equal(canonicalLink(url)[0].href, url); +}); diff --git a/tests/visual/theme.spec.ts b/tests/visual/theme.spec.ts index b563d34e9..9c1a43212 100644 --- a/tests/visual/theme.spec.ts +++ b/tests/visual/theme.spec.ts @@ -459,7 +459,8 @@ test.describe("Meta/SEO and notebook output", () => { expect(meta(page, sel)).toHaveAttribute("content", content); await expectTag('property="og:type"', "website"); await expectTag('property="og:site_name"', "QE Theme No-Thebe Fixture"); - await expectTag('property="og:url"', "https://example.org/notebook"); + // The trailing-slash form, which is the URL the build actually serves. + await expectTag('property="og:url"', "https://example.org/notebook/"); await expectTag('property="og:image"', "https://assets.example.org/qe-og-logo.png"); await expectTag('property="og:locale"', "en_US"); await expectTag('name="twitter:site"', "@quantecon"); @@ -469,6 +470,27 @@ test.describe("Meta/SEO and notebook output", () => { // Replaced, not duplicated: one og:image, one twitter:card. await expect(meta(page, 'property="og:image"')).toHaveCount(1); await expect(meta(page, 'name="twitter:card"')).toHaveCount(1); + + // The canonical link, from the same `site_url` and the same builder as + // og:url, so the two agree on every page. + const canonical = page.locator('head link[rel="canonical"]'); + await expect(canonical).toHaveCount(1); + await expect(canonical).toHaveAttribute("href", "https://example.org/notebook/"); + // The home page's canonical is the site root, not the index slug's URL. + await page.goto(`${noThebeBase}/`, { waitUntil: "domcontentloaded" }); + await expect(page.locator('head link[rel="canonical"]')).toHaveAttribute( + "href", + "https://example.org/" + ); + }); + + // A site that sets no `site_url` emits neither, as Sphinx emits nothing + // without `html_baseurl`. The main fixture sets none. + test("no-canonical-without-site-url", async ({ page }, testInfo) => { + test.skip(testInfo.project.name !== "desktop-chrome", "not viewport-dependent"); + await page.goto("/features", { waitUntil: "domcontentloaded" }); + await expect(page.locator('head link[rel="canonical"]')).toHaveCount(0); + await expect(meta(page, 'property="og:url"')).toHaveCount(0); }); // A cell's stderr stream is folded behind a "Code warnings" disclosure, From d1acc6b3f71fdbd148d1a73b7e7ee89d210cebef Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 11 Sep 2026 18:24:01 +0000 Subject: [PATCH 2/3] fix(seo): correct three canonical-URL defects found by review - On a site whose projects carry a slug, every project home page was canonicalised onto a URL the build never writes. The home test compared the bare index slug against the whole path, which can only match a project at the site root; with a base URL the export renders a project's index.html by requesting `/`, so the test failed and the page named `//` -- a 404 -- as its canonical. The project slug is now part of the test, and such a page resolves to the project root, which is where the build serves it. - A page whose own slug equalled the base-URL segment was canonicalised onto the home page. The base was stripped on an exact match as well as on a `/` prefix, and at render time the path carries no base at all, so `/notebook` under `BASE_URL=/notebook` looked like the site root. Only a real prefix is stripped now, which leaves the client path (which does carry the base) resolving to the same URL. - `absoluteImage` corrupted a protocol-relative image URL: `//host/x.png` begins with a slash but names its own host, and the site origin was prefixed to it. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UZLpDVYu1YBZHQfwkRRJj7 --- app/routes/$.tsx | 12 ++--- app/routes/_index.tsx | 5 +- app/seo.ts | 52 ++++++++++++++++---- tests/unit/seo.test.mjs | 103 +++++++++++++++++++++++++++++----------- 4 files changed, 128 insertions(+), 44 deletions(-) diff --git a/app/routes/$.tsx b/app/routes/$.tsx index 60597ceeb..df02f7fb8 100644 --- a/app/routes/$.tsx +++ b/app/routes/$.tsx @@ -12,7 +12,7 @@ import { getConfig, getPage } from '~/backend/loaders.server'; import type { SiteManifest } from 'myst-config'; import { ErrorPage } from '~/components/ErrorPage'; import { Page } from '~/components/Page'; -import { hreflangLinks, stripBaseurl } from '~/i18n'; +import { hreflangLinks } from '~/i18n'; import { canonicalLink, mergeMeta, pageUrl, siteOrigin, socialMetaTags } from '~/seo'; // Never re-run the loader on a navigation that changes neither pathname nor @@ -38,14 +38,14 @@ export const meta: V2_MetaFunction = ({ data, matches, location } ); const baseurl = rootMatch?.data?.BASE_URL; - // The page's public URL, shared by og:url and the canonical link. The path - // is stripped of the base first: the browser router has no basename, so on - // the client `location.pathname` already carries it, and prefixing again - // would repeat it. + // The page's public URL, shared by og:url and the canonical link. `pageUrl` + // strips the base before re-applying it: the browser router has no basename, + // so on the client `location.pathname` already carries it. const url = pageUrl({ origin: siteOrigin((config?.options as any)?.site_url, config?.domains), - path: stripBaseurl(location.pathname, baseurl), + pathname: location.pathname, baseurl, + projectSlug: project?.slug, indexSlug: project?.index, }); diff --git a/app/routes/_index.tsx b/app/routes/_index.tsx index 92725f668..9fc7da219 100644 --- a/app/routes/_index.tsx +++ b/app/routes/_index.tsx @@ -8,7 +8,7 @@ import type { SiteManifest } from 'myst-config'; import { getProject } from '@myst-theme/common'; import { Page } from '~/components/Page'; -import { hreflangLinks, stripBaseurl } from '~/i18n'; +import { hreflangLinks } from '~/i18n'; import { canonicalLink, mergeMeta, pageUrl, siteOrigin, socialMetaTags } from '~/seo'; // Never re-run the loader on a navigation that changes neither pathname nor @@ -32,8 +32,9 @@ export const meta: V2_MetaFunction = ({ data, matches, location } // article route for why the base is stripped before it is re-applied. const url = pageUrl({ origin: siteOrigin((config?.options as any)?.site_url, config?.domains), - path: stripBaseurl(location.pathname, baseurl), + pathname: location.pathname, baseurl, + projectSlug: project?.slug, indexSlug: project?.index, }); diff --git a/app/seo.ts b/app/seo.ts index 04ccb9f25..e30a92599 100644 --- a/app/seo.ts +++ b/app/seo.ts @@ -59,11 +59,13 @@ export interface SeoInput { export interface PageUrlInput { /** Site origin, from `siteOrigin`. Without it there is no public URL. */ origin?: string; - /** Site-relative page path, with any base URL already stripped. */ - path: string; + /** The page's render-time path, which may or may not carry the base URL. */ + pathname: string; /** The static build's base URL, if the site has one. */ baseurl?: string; - /** `config.index`: the slug whose page the site root serves. */ + /** `project.slug`, on a site whose projects are not at the site root. */ + projectSlug?: string; + /** `project.index`: the slug whose page the project's root serves. */ indexSlug?: string; } @@ -83,12 +85,38 @@ export interface PageUrlInput { * Returns undefined when the site sets no `site_url`, as Sphinx emits nothing * without `html_baseurl`. */ -export function pageUrl({ origin, path, baseurl, indexSlug }: PageUrlInput): string | undefined { +export function pageUrl({ + origin, + pathname, + baseurl, + projectSlug, + indexSlug, +}: PageUrlInput): string | undefined { if (!origin) return undefined; const base = (baseurl ?? '').trim().replace(/\/+$/, ''); - const slug = (path || '/').replace(/^\/+|\/+$/g, ''); - const isHome = slug === '' || (!!indexSlug && slug === indexSlug); - return `${origin}${base}${isHome ? '/' : `/${slug}/`}`; + + // The base is stripped only where it is a real prefix -- `/...` -- and + // never on an exact match. The browser router has no basename, so on the + // client the path carries the base and has to lose it before it is + // re-applied; at render time it does not carry it at all, and a page whose + // own slug happens to equal the base segment would otherwise be mistaken + // for the site root and canonicalised onto it. + let path = pathname || '/'; + if (base && path.startsWith(`${base}/`)) path = path.slice(base.length); + const slug = trim(path); + + // The project's root, which serves the index page. With a base URL the + // export renders that page by requesting the index slug -- under the + // project's own slug when it has one -- and writes it as the root's + // index.html, so the render-time path is that slug and its own URL is not + // served at all. + const project = trim(projectSlug ?? ''); + const home = [project, indexSlug].filter(Boolean).join('/'); + let tail: string; + if (slug === '') tail = '/'; + else if (indexSlug && slug === home) tail = project ? `/${project}/` : '/'; + else tail = `/${slug}/`; + return `${origin}${base}${tail}`; } export interface CanonicalLink { @@ -110,10 +138,14 @@ export function canonicalLink(url?: string): CanonicalLink[] { return url ? [{ tagName: 'link', rel: 'canonical', href: url }] : []; } -/** An image URL a social scraper can fetch: root-relative paths take the origin. */ +/** + * An image URL a social scraper can fetch: root-relative paths take the origin. + * A protocol-relative URL already names its own host, so it is left alone -- + * it begins with a slash but is not a path on this site. + */ export function absoluteImage(image?: string, origin?: string): string | undefined { if (!image) return undefined; - if (!origin || !image.startsWith('/')) return image; + if (!origin || !image.startsWith('/') || image.startsWith('//')) return image; return `${origin}${image}`; } @@ -137,6 +169,8 @@ export function siteOrigin(siteUrl?: string, domains?: string[]): string | undef return /^https?:\/\//i.test(h) ? h : `https://${h}`; } +const trim = (value: string): string => value.replace(/^\/+|\/+$/g, ''); + function handle(twitter?: string): string | undefined { const t = twitter?.trim().replace(/^@/, ''); return t ? `@${t}` : undefined; diff --git a/tests/unit/seo.test.mjs b/tests/unit/seo.test.mjs index e44ad8fd6..cc8e91c51 100644 --- a/tests/unit/seo.test.mjs +++ b/tests/unit/seo.test.mjs @@ -108,55 +108,101 @@ test('mergeMeta replaces same-key upstream tags and keeps the rest in order', () ]); }); +const QE = 'https://quantecon.github.io'; + test('pageUrl: the trailing-slash form, with and without a base URL', () => { const origin = 'https://python-programming.quantecon.org'; - assert.equal(pageUrl({ origin, path: '/about-py' }), `${origin}/about-py/`); - assert.equal(pageUrl({ origin, path: '/about-py/' }), `${origin}/about-py/`); + assert.equal(pageUrl({ origin, pathname: '/about-py' }), `${origin}/about-py/`); + assert.equal(pageUrl({ origin, pathname: '/about-py/' }), `${origin}/about-py/`); assert.equal( - pageUrl({ origin: 'https://quantecon.github.io', path: '/short-path', baseurl: '/lecture-wasm' }), - 'https://quantecon.github.io/lecture-wasm/short-path/', + pageUrl({ origin: QE, pathname: '/short-path', baseurl: '/lecture-wasm' }), + `${QE}/lecture-wasm/short-path/`, ); // A trailing slash on the base is not doubled. assert.equal( - pageUrl({ origin: 'https://quantecon.github.io', path: '/short-path', baseurl: '/lecture-wasm/' }), - 'https://quantecon.github.io/lecture-wasm/short-path/', + pageUrl({ origin: QE, pathname: '/short-path', baseurl: '/lecture-wasm/' }), + `${QE}/lecture-wasm/short-path/`, ); }); -test('pageUrl: the base appears exactly once when the path still carries it', () => { - // On the client the router has no basename, so `location.pathname` carries - // the base; the routes strip it before calling this, and a path that slipped - // through unstripped must not double it. - const url = pageUrl({ - origin: 'https://quantecon.github.io', - path: '/short-path', - baseurl: '/lecture-wasm', - }); - assert.equal(url, 'https://quantecon.github.io/lecture-wasm/short-path/'); - assert.equal((url.match(/lecture-wasm/g) ?? []).length, 1); +test('pageUrl: the base appears exactly once, whether or not the path carries it', () => { + // At render time the path has no base; on the client the router has no + // basename so it does. Both must give the same URL. + for (const pathname of ['/short-path', '/lecture-wasm/short-path']) { + const url = pageUrl({ origin: QE, pathname, baseurl: '/lecture-wasm' }); + assert.equal(url, `${QE}/lecture-wasm/short-path/`); + assert.equal((url.match(/lecture-wasm/g) ?? []).length, 1); + } +}); + +test('pageUrl: a page whose slug equals the base segment is not mistaken for the root', () => { + // The base is stripped only as a `/` prefix. At render time the path + // is the bare page path, so an exact match is the page, not the site root. + assert.equal( + pageUrl({ origin: QE, pathname: '/notebook', baseurl: '/notebook' }), + `${QE}/notebook/notebook/`, + ); + // ...and the client path for that same page agrees. + assert.equal( + pageUrl({ origin: QE, pathname: '/notebook/notebook', baseurl: '/notebook' }), + `${QE}/notebook/notebook/`, + ); + // The home page of that site still resolves to the root. + assert.equal(pageUrl({ origin: QE, pathname: '/notebook/', baseurl: '/notebook' }), `${QE}/notebook/`); }); test('pageUrl: the home page is the site root, named by path or by index slug', () => { - const origin = 'https://quantecon.github.io'; - assert.equal(pageUrl({ origin, path: '/', baseurl: '/lecture-wasm' }), `${origin}/lecture-wasm/`); + assert.equal(pageUrl({ origin: QE, pathname: '/', baseurl: '/lecture-wasm' }), `${QE}/lecture-wasm/`); // With a base URL the export renders the root index.html by requesting the // index slug, so that is the home page's render-time path -- and the slug's // own URL is not served. assert.equal( - pageUrl({ origin, path: '/intro', baseurl: '/lecture-wasm', indexSlug: 'intro' }), - `${origin}/lecture-wasm/`, + pageUrl({ origin: QE, pathname: '/intro', baseurl: '/lecture-wasm', indexSlug: 'intro' }), + `${QE}/lecture-wasm/`, ); - assert.equal(pageUrl({ origin, path: '/intro', indexSlug: 'intro' }), `${origin}/`); + assert.equal(pageUrl({ origin: QE, pathname: '/intro', indexSlug: 'intro' }), `${QE}/`); // A different page is unaffected by the index slug. assert.equal( - pageUrl({ origin, path: '/introduction', indexSlug: 'intro' }), - `${origin}/introduction/`, + pageUrl({ origin: QE, pathname: '/introduction', indexSlug: 'intro' }), + `${QE}/introduction/`, + ); +}); + +test('pageUrl: a project home page resolves to the project root, not the site root', () => { + // On a site whose projects carry a slug, the export renders the project's + // own index.html by requesting `/` and writes it as + // `/index.html`, so the served URL is the project root. + assert.equal( + pageUrl({ + origin: QE, + pathname: '/alpha/index', + baseurl: '/lecture-wasm', + projectSlug: 'alpha', + indexSlug: 'index', + }), + `${QE}/lecture-wasm/alpha/`, + ); + // Without a base URL the same project home is requested at its slug. + assert.equal( + pageUrl({ origin: QE, pathname: '/alpha', projectSlug: 'alpha', indexSlug: 'index' }), + `${QE}/alpha/`, + ); + // Any other page of that project keeps its own path. + assert.equal( + pageUrl({ + origin: QE, + pathname: '/alpha/page1', + baseurl: '/lecture-wasm', + projectSlug: 'alpha', + indexSlug: 'index', + }), + `${QE}/lecture-wasm/alpha/page1/`, ); }); test('pageUrl: nothing without an origin, as Sphinx emits nothing without html_baseurl', () => { - assert.equal(pageUrl({ path: '/about-py' }), undefined); - assert.equal(pageUrl({ origin: undefined, path: '/', baseurl: '/x' }), undefined); + assert.equal(pageUrl({ pathname: '/about-py' }), undefined); + assert.equal(pageUrl({ origin: undefined, pathname: '/', baseurl: '/x' }), undefined); }); test('canonicalLink: a link descriptor, or nothing', () => { @@ -171,12 +217,15 @@ test('absoluteImage: only root-relative paths take the origin', () => { assert.equal(absoluteImage('https://cdn.example/og.png', 'https://example.org'), 'https://cdn.example/og.png'); assert.equal(absoluteImage('/build/graph.png', undefined), '/build/graph.png'); assert.equal(absoluteImage(undefined, 'https://example.org'), undefined); + // A protocol-relative URL names its own host: it starts with a slash but is + // not a path on this site, so prefixing the origin would break it. + assert.equal(absoluteImage('//assets.example.org/og.png', 'https://example.org'), '//assets.example.org/og.png'); }); test('og:url comes from the same URL the canonical link uses', () => { const url = pageUrl({ origin: 'https://quantecon.github.io', - path: '/short-path', + pathname: '/short-path', baseurl: '/lecture-wasm', }); const tags = byKey(socialMetaTags({ url, options: { site_url: 'https://quantecon.github.io' } })); From 8f8e889358eb81e12d0f56980413029e8896c1d4 Mon Sep 17 00:00:00 2001 From: Matt McKay Date: Sat, 12 Sep 2026 12:37:52 +1000 Subject: [PATCH 3/3] fix(seo): normalise BASE_URL where it enters the app A trailing slash on `BASE_URL` doubled the separator in every href built from it. `Document` builds `${baseurl}/favicon.ico` and `${baseurl}/myst-theme.css`, so `/lecture-wasm/` produced `/lecture-wasm//favicon.ico` -- and the same unnormalised value reaches `BaseUrlProvider`, so every link `withBaseurl()` builds carried it too. Whoever deploys the site writes `BASE_URL` by hand, so both spellings arrive. `normalizeBaseurl` trims it and strips trailing slashes once, in the root loader where the value enters, and `pageUrl` calls it instead of repeating the rule -- so the canonical URL and the head links cannot disagree about the shape of the base. The helper lives in `app/seo.ts` rather than `root.tsx` because a `.tsx` module cannot be imported by the unit suite: Node strips types but not JSX. Mutation-tested: with the trailing-slash strip removed the new test fails, and it passes again with the strip restored. Also drops the migration doc's "Not shipped yet" note for the canonical link, which this branch is what ships. Co-Authored-By: Claude Opus 5 (1M context) --- app/root.tsx | 6 +++++- app/seo.ts | 18 +++++++++++++++++- docs/migrating.md | 4 ---- tests/unit/seo.test.mjs | 19 +++++++++++++++++++ 4 files changed, 41 insertions(+), 6 deletions(-) diff --git a/app/root.tsx b/app/root.tsx index c90a77b74..062808f7e 100644 --- a/app/root.tsx +++ b/app/root.tsx @@ -23,6 +23,7 @@ import { JUPYTER_RENDERERS } from '@myst-theme/jupyter'; import { LIST_RENDERERS, STDERR_RENDERERS } from './renderers'; import { Document } from './components/Document'; import { htmlDir, htmlLang } from './i18n'; +import { normalizeBaseurl } from './seo'; import type { TemplateOptions } from './types'; export { AppErrorBoundary as ErrorBoundary } from '@myst-theme/site'; // Never re-run the loader on a navigation that changes neither pathname nor @@ -198,7 +199,10 @@ export const links: LinksFunction = () => { }; export const loader: LoaderFunction = async ({ request }): Promise => { - const baseURL = process.env.BASE_URL || undefined; + // Normalised here, at the one place the value enters the app: it reaches the + // Document's head links and the base-URL provider unchanged, and both join it + // to a path that already starts with a slash. + const baseURL = normalizeBaseurl(process.env.BASE_URL); const [config, themeSession] = await Promise.all([ getConfig().catch(() => null), getThemeSession(request), diff --git a/app/seo.ts b/app/seo.ts index e30a92599..b81dc8c1c 100644 --- a/app/seo.ts +++ b/app/seo.ts @@ -85,6 +85,22 @@ export interface PageUrlInput { * Returns undefined when the site sets no `site_url`, as Sphinx emits nothing * without `html_baseurl`. */ +/** + * The base URL in the one shape the rest of the app can append to: trimmed, + * with any trailing slashes removed, and undefined when there is nothing left. + * + * `BASE_URL` is written by whoever deploys the site, so `/lecture-wasm/` is as + * likely as `/lecture-wasm`, and every consumer joins it to a path that already + * starts with a slash. Normalising once here keeps the doubled separator out of + * a head link's href and out of every link the base-URL provider builds. + * Undefined rather than `''` for an empty value, so `baseurl && ...` guards + * still tell absent from present. + */ +export function normalizeBaseurl(value?: string): string | undefined { + const base = (value ?? '').trim().replace(/\/+$/, ''); + return base || undefined; +} + export function pageUrl({ origin, pathname, @@ -93,7 +109,7 @@ export function pageUrl({ indexSlug, }: PageUrlInput): string | undefined { if (!origin) return undefined; - const base = (baseurl ?? '').trim().replace(/\/+$/, ''); + const base = normalizeBaseurl(baseurl) ?? ''; // The base is stripped only where it is a real prefix -- `/...` -- and // never on an exact match. The browser router has no basename, so on the diff --git a/docs/migrating.md b/docs/migrating.md index e5071defe..618a89cfd 100644 --- a/docs/migrating.md +++ b/docs/migrating.md @@ -182,10 +182,6 @@ site: Without it the theme emits **neither** the canonical link nor `og:url`, and `og:image` stays root-relative. See [configuration](configuration.md). -> **Not shipped yet.** The canonical link is #207, implemented by #227. Until -> that merges `site_url` feeds `og:url` and `og:image` only, and no page carries -> a canonical link whether or not the option is set. - ## Old URLs Every page URL changes at cutover: `about_py.html` becomes `about-py/`. Inbound diff --git a/tests/unit/seo.test.mjs b/tests/unit/seo.test.mjs index cc8e91c51..2f7764c17 100644 --- a/tests/unit/seo.test.mjs +++ b/tests/unit/seo.test.mjs @@ -9,6 +9,7 @@ import { absoluteImage, canonicalLink, mergeMeta, + normalizeBaseurl, ogLocale, pageUrl, siteOrigin, @@ -28,6 +29,24 @@ test('siteOrigin: site_url wins and is reduced to an origin; domains are the fal assert.equal(siteOrigin(' ', []), undefined); }); +test('normalizeBaseurl: no trailing slash survives, and empty means absent', () => { + // Whoever deploys the site writes BASE_URL by hand, so both spellings arrive. + // Every consumer joins the result to a path that already starts with a slash + // -- the head links build `${baseurl}/favicon.ico` and `${baseurl}/myst-theme.css`, + // and the base-URL provider builds every in-site link -- so a surviving + // trailing slash would double the separator in all of them. + assert.equal(normalizeBaseurl('/lecture-wasm'), '/lecture-wasm'); + assert.equal(normalizeBaseurl('/lecture-wasm/'), '/lecture-wasm'); + assert.equal(normalizeBaseurl('/lecture-wasm//'), '/lecture-wasm'); + assert.equal(normalizeBaseurl(' /quantecon-theme.mystmd/pr-preview/pr-9/ '), '/quantecon-theme.mystmd/pr-preview/pr-9'); + // Undefined rather than '' for an absent value, so `baseurl && ...` guards in + // the Document still tell absent from present. + assert.equal(normalizeBaseurl(undefined), undefined); + assert.equal(normalizeBaseurl(''), undefined); + assert.equal(normalizeBaseurl(' '), undefined); + assert.equal(normalizeBaseurl('/'), undefined); +}); + test('ogLocale: BCP 47 to OpenGraph', () => { assert.equal(ogLocale('en'), 'en_US'); assert.equal(ogLocale('zh-cn'), 'zh_CN');