diff --git a/.changeset/locale-cluster-shape.md b/.changeset/locale-cluster-shape.md new file mode 100644 index 0000000..9eb8257 --- /dev/null +++ b/.changeset/locale-cluster-shape.md @@ -0,0 +1,44 @@ +--- +"@ingram-tech/nk-i18n": minor +"@ingram-tech/nk-seo": minor +--- + +Fix the locale cluster shape, and give middleware one way to be written. + +**Breaking, deliberately.** `prefixDefaultLocale` is removed and nothing +replaces it: the cluster's shape is no longer configurable. Whichever strategy +you pick, every locale gets its own address (the default included) and the bare +path belongs to no locale — it negotiates, and it is `x-default`. + +Previously the prefix strategy made the bare path the default locale's URL, so +`localeFromUrl` returned `defaultLocale` for it. That is the URL signal, which +outranks the cookie, so a visitor who chose French snapped back to English on +the first bare internal link — and every site that starts with a cookie switcher +has bare internal links. The two shapes now excluded (bare path IS the default +locale, bare path redirects on perceived language) are the two that go wrong; +offering either as an option is how a fleet drifts. + +- **`localeProxy(routing, request)`** is the whole middleware side: forwards the + pathname and locale headers, rewrites `/fr/about` → `/about` so the app keeps + one route tree, remembers an explicit choice in the cookie, never redirects. + Middleware that does more passes `requestHeaders` in and keeps editing the + response. Replaces `forwardUrlLocale` and the strip/rewrite/cookie/consolidate + code every prefix site was hand-writing. +- **`forwardRequestContext`** sets nk-seo's `x-pathname` and the locale header + together, so the two conventions can't be wired separately and one forgotten. +- **`defineLocaleRouting` is generic over the locale union.** `isLocale` is a + type guard, `resolve` / `localeFromUrl` / `createLocaleResolver` return `L`. + Sites stop writing their own guards and casts. +- **`hrefLangTags` and `cookieName` move onto routing.** A site with regional + tags no longer builds a second config object, which was exactly the drift this + package exists to prevent. `routing.htmlLang(locale)` gives the `` + value, and `hreflangConfigFor` passes the tags through. +- **`routing.stripLocale(pathname)`** exposes the app-facing path. + +nk-seo's `HreflangConfig` drops `defaultLocale` and `prefixDefaultLocale`; +`x-default` is always the bare path now, so neither is needed. + +Migration: delete `prefixDefaultLocale`, replace `forwardUrlLocale` + +manual `x-pathname` with `localeProxy`, drop any local `isLocale` guard and +`as Locale` cast. Prefix sites gain `/en/…` as a real address — verify with +`assertHreflangCluster` from `@ingram-tech/nk-seo/verify`. diff --git a/.changeset/nk-dev-cold-type-check.md b/.changeset/nk-dev-cold-type-check.md new file mode 100644 index 0000000..2d71bbe --- /dev/null +++ b/.changeset/nk-dev-cold-type-check.md @@ -0,0 +1,12 @@ +--- +"@ingram-tech/nk-dev": minor +--- + +`nk type-check` starts cold when the dependency tree moved: a `*.tsbuildinfo` +older than `bun.lock` / `package.json` is dropped before the run, because +`tsc --incremental` does not reliably re-check a program after a dependency's +`.d.ts` changes and a green result against the stale cache means nothing. +`--cold` drops the cache unconditionally. `nk doctor` now flags a `"prettier"` +key in `package.json` and `.prettierrc*` files alongside `.prettierignore` +(all `--fix`able), and warns when a site has no `ci` script or one that skips +`nk check` / `nk type-check`. diff --git a/docs/i18n-routing.md b/docs/i18n-routing.md index 1c5437d..3867378 100644 --- a/docs/i18n-routing.md +++ b/docs/i18n-routing.md @@ -38,32 +38,52 @@ in 1998 and no browser implemented it. So: negotiation is a fine convenience at a front door, and never the addressing scheme. -## Strategies +## The cluster shape is fixed -`defineLocaleRouting({ strategy })` picks how a locale is encoded. +Whichever strategy you pick: -**`"query"` (default)** — every locale gets `?hl=`, including the -default. The bare path negotiates and belongs to no locale: it is `x-default`. +- every locale has its own address, **the default included**; +- the bare path belongs to **no** locale. It negotiates, and it is `x-default`. ``` -x-default → /pricing negotiates, language varies by visitor -en → /pricing?hl=en -fr → /pricing?hl=fr -nl → /pricing?hl=nl +x-default → /pricing negotiates; language varies by visitor +en → /pricing?hl=en or /en/pricing +fr → /pricing?hl=fr or /fr/pricing +nl → /pricing?hl=nl or /nl/pricing ``` -Use it when the site wants one shareable address per page and negotiation for -humans. Google supports parameter-based locale URLs and does not recommend them, -so accept a slightly thinner margin than prefixes in exchange for not having a -locale segment to maintain. +This is not configurable, deliberately. The two shapes it excludes are the ones +that go wrong: + +- **bare path IS the default locale.** A French visitor who follows a bare + internal link gets English, and every site that starts with a cookie switcher + has bare internal links. This is the bug that made one fleet site fork its + middleware rather than adopt the helpers. +- **bare path redirects on perceived language.** Google tells you not to build + this, and it makes `x-default` point at a URL that is not language-neutral. + +Offering either as an option is how the fleet drifts, so neither is offered. + +## Strategies + +`defineLocaleRouting({ strategy })` picks only the **encoding**. -The default locale gets its own `?hl=en` **because the bare path is not reliably -English**. As soon as country is a negotiation signal, the bare path renders -French to a crawl from France, so labelling it `en` would be false half the time. +**`"prefix"`** — `/fr/pricing`. Prefer this for a new site. It is better on +every SEO axis: a path cannot be folded into another document the way a query +parameter can (and the URL Parameters tool that used to override that was +retired in 2022), it survives link-sharing and CMS fields that strip query +strings, it does not combine with campaign parameters into an open-ended URL +space, it puts the target-language keyword in the URL, and analytics group by +pathname for free. -**`"prefix"`** — the default locale keeps the bare path, others get `//…` -(`prefixDefaultLocale` prefixes every locale instead). Stronger for ranking, -since the locale is in the path, at the cost of a routing segment. +`localeProxy` rewrites `/fr/pricing` to `/pricing`, so the app keeps one route +tree and never learns what a locale is. + +**`"query"` (default)** — `/pricing?hl=fr`. Google supports it and does not +recommend it. Use it when restructuring routes is not worth it, knowing it is +the weaker of the two — typically a marketing tree of React components with +inline `t()` calls, where the win from having addresses at all dwarfs the gap +between the two encodings. ## The precedence chain @@ -110,29 +130,46 @@ export const routing = defineLocaleRouting({ baseUrl: "https://acme.example", locales: ["en", "fr", "nl"], defaultLocale: "en", + strategy: "prefix", countryLocales: { FR: "fr", NL: "nl" }, // no BE: ambiguous + hrefLangTags: { en: "en-BE", fr: "fr-BE", nl: "nl-BE" }, // only if content differs }); ``` ```ts -// proxy.ts — forward, never redirect +// proxy.ts — the whole middleware side export function proxy(request: NextRequest) { - const requestHeaders = new Headers(request.headers); - forwardUrlLocale(routing, request.nextUrl, requestHeaders); - return NextResponse.next({ request: { headers: requestHeaders } }); + return localeProxy(routing, request); } ``` +`localeProxy` forwards the pathname and URL-locale headers, rewrites a locale +prefix away, and remembers an explicit choice in the cookie. It never redirects. +Middleware that does more of its own work passes its headers in and keeps +editing the response: + +```ts +const requestHeaders = new Headers(request.headers); +requestHeaders.set("x-tenant", tenant); +const response = localeProxy(routing, request, { requestHeaders }); +response.cookies.set(…); +return response; +``` + ```ts -// lib/i18n/locale.ts +// lib/i18n/locale.ts — narrowed to your locale union, no cast export const resolveLocale = cache( createLocaleResolver(routing, { account: () => getProfile().locale }), ); ``` ```tsx -// app/layout.tsx - +// app/layout.tsx — pathname comes from the header localeProxy set + + + + + ``` The language switcher must be real `` links to `routing.urlForLocale(…)`. diff --git a/packages/nk-dev/bin/nk.js b/packages/nk-dev/bin/nk.js index ea71dba..d3a7611 100755 --- a/packages/nk-dev/bin/nk.js +++ b/packages/nk-dev/bin/nk.js @@ -34,8 +34,11 @@ Commands: mechanical refactors — see the codemod skill. check The CI gate: lint + format verify + knip (when configured) + the agent-guide import gate + the migration seal. - type-check next typegen && tsc --noEmit. Recovers automatically when + type-check [--cold] next typegen && tsc --noEmit. Recovers automatically when generated types are damaged (e.g. a killed dev server). + Drops the incremental cache when the lockfile changed + since it was written (a stale cache passes against old + .d.ts); --cold always drops it. clean Remove regenerable build artifacts: Next's generated types and TypeScript incremental caches. test [...] vitest run (extra args passed through). @@ -75,7 +78,7 @@ switch (cmd) { check(); break; case "type-check": - typeCheck(); + typeCheck(rest); break; case "clean": clean(); diff --git a/packages/nk-dev/lib/artifacts.js b/packages/nk-dev/lib/artifacts.js index b87cded..16f9100 100644 --- a/packages/nk-dev/lib/artifacts.js +++ b/packages/nk-dev/lib/artifacts.js @@ -1,4 +1,4 @@ -import { existsSync, readdirSync, rmSync } from "node:fs"; +import { existsSync, readdirSync, rmSync, statSync } from "node:fs"; import { join } from "node:path"; /** @@ -45,6 +45,38 @@ export function listGeneratedArtifacts(cwd = process.cwd()) { return [...present, ...buildInfo]; } +/** + * Files whose change means the dependency tree changed. The lockfile is the + * precise signal; `package.json` covers a hand edit not yet installed. + */ +const DEPENDENCY_MANIFESTS = [ + "bun.lock", + "bun.lockb", + "package-lock.json", + "package.json", +]; + +/** + * TypeScript incremental caches older than the dependency manifests. + * + * `tsc --incremental` does not reliably notice a dependency's `.d.ts` + * changing, so after an upgrade a type-check can pass against the cached + * program while a cold run fails — the exact failure a dependency bump exists + * to catch. Keying invalidation on the lockfile catches that case without + * giving up the cache on every other run. + */ +export function staleBuildInfo(cwd = process.cwd()) { + const manifests = DEPENDENCY_MANIFESTS.map((name) => join(cwd, name)).filter( + (file) => existsSync(file), + ); + if (manifests.length === 0) return []; + const newest = Math.max(...manifests.map((file) => statSync(file).mtimeMs)); + return listGeneratedArtifacts(cwd) + .filter((entry) => entry.owner === "tsc") + .filter((entry) => statSync(join(cwd, entry.path)).mtimeMs < newest) + .map((entry) => entry.path); +} + /** * Remove generated artifacts and return the paths actually deleted. Missing * paths are skipped rather than reported, so this is idempotent. diff --git a/packages/nk-dev/lib/doctor.js b/packages/nk-dev/lib/doctor.js index 82670eb..4c5fbd8 100644 --- a/packages/nk-dev/lib/doctor.js +++ b/packages/nk-dev/lib/doctor.js @@ -1,4 +1,4 @@ -import { existsSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { existsSync, readFileSync, readdirSync, rmSync, writeFileSync } from "node:fs"; import { resolve } from "node:path"; import { authShadowFindings } from "./auth-shadow.js"; import { SUPERSEDED_DEPS } from "./drift.js"; @@ -252,30 +252,82 @@ export function findings(cwd) { } } - // 8. .prettierignore is now dead weight (nk no longer formats SQL). - const prettierIgnore = resolve(cwd, ".prettierignore"); - if (existsSync(prettierIgnore)) { + // 8. Prettier leftovers are dead weight (nk no longer runs Prettier): + // .prettierignore, any .prettierrc*, and a "prettier" key in package.json. + for (const file of prettierFiles(cwd)) { out.push({ - id: "prettierignore", + id: `prettier:${file}`, + level: "warn", + message: `${file} is unused (nk no longer runs Prettier) — remove it`, + fix: (dir) => { + rmSync(resolve(dir, file)); + return `removed ${file}`; + }, + }); + } + if (pkg.prettier !== undefined) { + out.push({ + id: "prettier:package.json", level: "warn", message: - ".prettierignore is unused (nk no longer runs Prettier) — remove it", + 'package.json has a "prettier" key (nk no longer runs Prettier) — remove it', fix: (dir) => { - rmSync(resolve(dir, ".prettierignore")); - return "removed .prettierignore"; + const p = resolve(dir, "package.json"); + const j = readJson(p); + delete j.prettier; + writeJson(p, j); + return 'removed "prettier" from package.json'; }, }); } - // 9. The migration chain is sealed, and its unmodelled DDL is declared. + // 9. A `ci` script exists and runs the house gate. Its full contents are + // the site's call (migrations, i18n, email catalogs, build…), so this only + // warns and never writes — but the dep-upgrade flow and pre-push both + // assume `bun run ci` is the one command that proves a change. + const ci = scripts["ci"]; + if (ci === undefined) { + out.push({ + id: "script:ci", + level: "warn", + message: + 'missing `ci` script — the one command that proves a change (e.g. "nk check && nk type-check && nk test")', + }); + } else { + const missing = ["nk check", "nk type-check"].filter( + (cmd) => !ci.includes(cmd) && !ci.includes(`bun run ${cmd.slice(3)}`), + ); + if (missing.length > 0) { + out.push({ + id: "script:ci", + level: "warn", + message: `\`ci\` script does not run ${missing.map((c) => `\`${c}\``).join(" or ")} — the gate should run both`, + }); + } + } + + // 10. The migration chain is sealed, and its unmodelled DDL is declared. out.push(...migrationFindings(cwd)); - // 10. No page/route under app/auth/ shadows a Better Auth endpoint. + // 11. No page/route under app/auth/ shadows a Better Auth endpoint. out.push(...authShadowFindings(cwd)); return out; } +/** Prettier config files present in `cwd` (relative names). */ +function prettierFiles(cwd) { + let names; + try { + names = readdirSync(cwd); + } catch { + return []; + } + return names.filter( + (name) => name === ".prettierignore" || name.startsWith(".prettierrc"), + ); +} + /** * Findings over a `drizzle/` chain. Silent on repos without one. * @@ -332,8 +384,8 @@ function migrationFindings(cwd) { /** * `nk doctor [--fix]` — report drift from the canonical nk-dev model (scripts, * dependencies, oxlint/tsconfig extends, the CLAUDE.md guide import, stale knip - * ignores, forbidden schema-applying drizzle-kit scripts, a dead - * .prettierignore, an unsealed or unmodelled-DDL-carrying migration chain, a + * ignores, forbidden schema-applying drizzle-kit scripts, Prettier leftovers, + * a missing or thin `ci` script, an unsealed or unmodelled-DDL-carrying migration chain, a * page under app/auth/ shadowing a Better Auth endpoint). * With `--fix`, apply every auto-fixable finding, then remind * to reinstall. diff --git a/packages/nk-dev/lib/passthrough.js b/packages/nk-dev/lib/passthrough.js index 7d83fed..666e17f 100644 --- a/packages/nk-dev/lib/passthrough.js +++ b/packages/nk-dev/lib/passthrough.js @@ -1,10 +1,15 @@ import { checkAgentGuideImport } from "./agent-guide.js"; -import { cleanGeneratedArtifacts, onlyGeneratedTypeErrors } from "./artifacts.js"; +import { + cleanGeneratedArtifacts, + onlyGeneratedTypeErrors, + staleBuildInfo, +} from "./artifacts.js"; import { toolDrift } from "./drift.js"; import { FORMATTER } from "./formatter.js"; import { hasKnipConfig, runKnip } from "./knip.js"; import { checkSeal } from "./migrations.js"; import { run, runCapture, writeThrough } from "./run.js"; +import { readdirSync, rmSync } from "node:fs"; /** `nk lint [...]` — oxlint, with extra args passed through (e.g. `--fix`). */ export function lint(extraArgs = []) { @@ -75,8 +80,27 @@ function warnToolDrift() { * output suppresses semantic diagnostics for the whole program, so real `src/` * errors are hidden behind it. Recovering surfaces them and still exits * non-zero — it never turns a failing check into a passing one. + * + * Starts cold when the dependency tree moved. `tsc --incremental` does not + * reliably re-check a program when a dependency's `.d.ts` changes, so a + * `.tsbuildinfo` older than the lockfile is a green light that means nothing; + * it is dropped (with a note) before the run. `--cold` drops it + * unconditionally. */ -export function typeCheck() { +export function typeCheck(extraArgs = []) { + const cold = extraArgs.includes("--cold"); + const stale = cold + ? cleanBuildInfoOnly() + : staleBuildInfo().map((file) => { + rmSync(file, { force: true }); + return file; + }); + if (stale.length > 0) { + console.error( + `nk type-check: ${cold ? "--cold" : "dependencies changed since the last run"} — removed ${stale.join(", ")}; checking from scratch.`, + ); + } + const typegen = run("next", ["typegen"]); if (typegen !== 0) process.exit(typegen); @@ -99,6 +123,17 @@ export function typeCheck() { process.exit(run("tsc", ["--noEmit"])); } +/** Delete every `*.tsbuildinfo` in cwd and return the names removed. */ +function cleanBuildInfoOnly() { + const removed = []; + for (const name of readdirSync(process.cwd())) { + if (!name.endsWith(".tsbuildinfo")) continue; + rmSync(name, { force: true }); + removed.push(name); + } + return removed; +} + /** * `nk clean` — remove build artifacts that tools regenerate from source * (Next's generated types, TypeScript incremental caches). Safe by diff --git a/packages/nk-dev/test/artifacts.test.js b/packages/nk-dev/test/artifacts.test.js index 60e8623..618c0c8 100644 --- a/packages/nk-dev/test/artifacts.test.js +++ b/packages/nk-dev/test/artifacts.test.js @@ -1,4 +1,11 @@ -import { existsSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { + existsSync, + mkdirSync, + mkdtempSync, + rmSync, + utimesSync, + writeFileSync, +} from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { afterEach, beforeEach, describe, expect, it } from "vitest"; @@ -7,6 +14,7 @@ import { errorFiles, listGeneratedArtifacts, onlyGeneratedTypeErrors, + staleBuildInfo, } from "../lib/artifacts.js"; const PLAIN = @@ -153,3 +161,22 @@ describe("listGeneratedArtifacts / cleanGeneratedArtifacts", () => { expect(cleanGeneratedArtifacts(dir)).toEqual([]); }); }); + +describe("staleBuildInfo", () => { + it("reports a tsbuildinfo older than the lockfile, and nothing when it is newer", () => { + const dir = mkdtempSync(join(tmpdir(), "nk-stale-")); + try { + writeFileSync(join(dir, "package.json"), "{}"); + writeFileSync(join(dir, "bun.lock"), ""); + writeFileSync(join(dir, "tsconfig.tsbuildinfo"), ""); + const old = new Date(Date.now() - 60_000); + utimesSync(join(dir, "tsconfig.tsbuildinfo"), old, old); + expect(staleBuildInfo(dir)).toEqual(["tsconfig.tsbuildinfo"]); + const later = new Date(Date.now() + 60_000); + utimesSync(join(dir, "tsconfig.tsbuildinfo"), later, later); + expect(staleBuildInfo(dir)).toEqual([]); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); +}); diff --git a/packages/nk-dev/test/doctor-prettier-ci.test.js b/packages/nk-dev/test/doctor-prettier-ci.test.js new file mode 100644 index 0000000..331be39 --- /dev/null +++ b/packages/nk-dev/test/doctor-prettier-ci.test.js @@ -0,0 +1,65 @@ +import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { findings } from "../lib/doctor.js"; + +describe("nk doctor: Prettier leftovers and the ci script", () => { + let dir; + beforeEach(() => { + dir = mkdtempSync(join(tmpdir(), "nk-doctor-")); + }); + afterEach(() => { + rmSync(dir, { recursive: true, force: true }); + }); + + const writePkg = (extra) => + writeFileSync( + join(dir, "package.json"), + JSON.stringify({ name: "site", scripts: {}, ...extra }), + ); + const readPkg = () => JSON.parse(readFileSync(join(dir, "package.json"), "utf8")); + const find = (id) => findings(dir).find((f) => f.id === id); + + it("flags a `prettier` key in package.json and deletes it on --fix", () => { + writePkg({ prettier: { semi: false } }); + const f = find("prettier:package.json"); + expect(f?.level).toBe("warn"); + f.fix(dir); + expect(readPkg().prettier).toBeUndefined(); + expect(find("prettier:package.json")).toBeUndefined(); + }); + + it("flags .prettierrc* and .prettierignore files and removes them on --fix", () => { + writePkg({}); + writeFileSync(join(dir, ".prettierrc.json"), "{}"); + writeFileSync(join(dir, ".prettierignore"), "*.sql\n"); + const rc = find("prettier:.prettierrc.json"); + const ignore = find("prettier:.prettierignore"); + expect(rc?.level).toBe("warn"); + expect(ignore?.level).toBe("warn"); + rc.fix(dir); + ignore.fix(dir); + expect(existsSync(join(dir, ".prettierrc.json"))).toBe(false); + expect(existsSync(join(dir, ".prettierignore"))).toBe(false); + }); + + it("warns when the ci script is missing, and does not offer a fix", () => { + writePkg({}); + const f = find("script:ci"); + expect(f?.level).toBe("warn"); + expect(f.fix).toBeUndefined(); + }); + + it("warns when ci skips the type-check", () => { + writePkg({ scripts: { ci: "nk check && vitest run" } }); + expect(find("script:ci")?.message).toContain("nk type-check"); + }); + + it("accepts ci that runs the gate directly or through scripts", () => { + writePkg({ scripts: { ci: "nk check && nk type-check && vitest run" } }); + expect(find("script:ci")).toBeUndefined(); + writePkg({ scripts: { ci: "bun run type-check && bun run check" } }); + expect(find("script:ci")).toBeUndefined(); + }); +}); diff --git a/packages/nk-i18n/README.md b/packages/nk-i18n/README.md index d20a172..02db5cd 100644 --- a/packages/nk-i18n/README.md +++ b/packages/nk-i18n/README.md @@ -115,11 +115,10 @@ export const resolveLocale = cache(async (): Promise => { ## Locale URL routing One definition of how a locale is encoded in a URL, shared by the code that -**serves** a language and the code that **advertises** it to search engines. When -those two drift — the classic being middleware that redirects away the very -`?hl=` URLs hreflang points at — the site tells Google the French page lives at -an address that doesn't serve French, and Google drops the language. Nothing -catches that, because neither half can see the other. +**serves** a language and the code that **advertises** it to search engines. +When those drift — the classic being middleware that redirects away the very +URLs hreflang points at — the site tells Google the French page lives at an +address that doesn't serve French, and Google drops the language. ```ts // lib/i18n/routing.ts @@ -129,54 +128,89 @@ export const routing = defineLocaleRouting({ baseUrl: "https://example.com", locales: ["en", "fr", "nl"], defaultLocale: "en", - // strategy: "query" (default) → /pricing?hl=fr, bare path is x-default - // strategy: "prefix" → /fr/pricing, default locale stays bare + strategy: "prefix", // or "query" (default) countryLocales: { FR: "fr", NL: "nl" }, // omit BE: geography can't decide + hrefLangTags: { en: "en-BE", fr: "fr-BE" }, // only if content differs by country }); ``` -**The rule: a URL that names a locale serves that locale, with a 200, to -everybody.** Never redirect it. +### The cluster shape is fixed + +Whichever strategy you pick: **every locale gets its own address, the default +included, and the bare path belongs to no locale** — it negotiates, and it is +`x-default`. + +``` +x-default → /pricing negotiates; language varies by visitor +en → /en/pricing or /pricing?hl=en +fr → /fr/pricing or /fr/pricing?hl=fr +``` + +That isn't configurable, on purpose. A bare path that *is* the default locale +serves English to anyone following a bare internal link, and a bare path that +redirects on perceived language is what Google tells you not to build. Both are +available as options in most i18n libraries and both are traps. + +`strategy` picks only the encoding. Prefer `"prefix"`: a path can't be folded +into another document the way a query parameter can, it survives link-sharing +that strips query strings, and it puts the target-language keyword in the URL. +Use `"query"` when restructuring routes isn't worth it. + +### Middleware ```ts -// proxy.ts — forward, never redirect -import { forwardUrlLocale } from "@ingram-tech/nk-i18n/next"; +// proxy.ts +import { localeProxy } from "@ingram-tech/nk-i18n/next"; export function proxy(request: NextRequest) { - const requestHeaders = new Headers(request.headers); - forwardUrlLocale(routing, request.nextUrl, requestHeaders); - return NextResponse.next({ request: { headers: requestHeaders } }); + return localeProxy(routing, request); } ``` +That forwards the pathname and URL-locale headers, rewrites `/fr/about` to +`/about` so the app keeps one route tree, and remembers an explicit choice in +the cookie. **It never redirects.** Middleware that does more of its own work +passes its headers in and keeps editing the response: + ```ts -// lib/i18n/locale.ts -import { createLocaleResolver } from "@ingram-tech/nk-i18n/next"; +const requestHeaders = new Headers(request.headers); +requestHeaders.set("x-tenant", tenant); +const response = localeProxy(routing, request, { requestHeaders }); +response.cookies.set(…); +return response; +``` +### Resolving + +```ts export const resolveLocale = cache( createLocaleResolver(routing, { account: () => getProfile().locale }), ); ``` +Returns your locale union, not `string`, so no guard or cast at the call site. The precedence is fixed and not configurable: -1. **the URL** (`?hl=fr`) 2. account setting 3. cookie 4. `Accept-Language` +1. **the URL** 2. account setting 3. cookie 4. `Accept-Language` 5. country 6. `defaultLocale` The URL beating the account setting is the load-bearing part: a shared link must show the recipient the language it names, or every localized link the site ships -is a lie. Suppliers are lazy, so a `?hl=` request never touches the database. +is a lie. Suppliers are lazy, so a localized address never touches the database. -For hreflang, hand the same object to nk-seo — `hreflangConfigFor` sets -`currentLocale` from the **URL**, so canonicals follow the address rather than -whatever language negotiation rendered: +### hreflang -```tsx -// app/layout.tsx -import { hreflangConfigFor } from "@ingram-tech/nk-i18n/next"; -import { HreflangLinks } from "@ingram-tech/nk-seo/components"; +Hand the same object to nk-seo. `hreflangConfigFor` sets `currentLocale` from +the **URL**, so canonicals follow the address rather than whatever language +negotiation rendered, and it passes `hrefLangTags` through so there is no second +config object to drift: -; +```tsx + + + + + ``` The language switcher must be real `` @@ -197,6 +231,6 @@ Prove the site serves what it advertises with `assertHreflangCluster` from `LocaleDefinition` / `LocaleRouting` / `LocaleSignals` types. - `@ingram-tech/nk-i18n/client` (`"use client"`): `LocaleProvider`, `useLocale`, `useT`. -- `@ingram-tech/nk-i18n/next` (server, needs `next`): `forwardUrlLocale`, - `getUrlLocale`, `createLocaleResolver`, `hreflangConfigFor`, - `LOCALE_URL_HEADER`. +- `@ingram-tech/nk-i18n/next` (server, needs `next`): `localeProxy`, + `forwardRequestContext`, `getUrlLocale`, `createLocaleResolver`, + `hreflangConfigFor`, `LOCALE_URL_HEADER`, `PATHNAME_HEADER`. diff --git a/packages/nk-i18n/src/next.test.ts b/packages/nk-i18n/src/next.test.ts new file mode 100644 index 0000000..0c13386 --- /dev/null +++ b/packages/nk-i18n/src/next.test.ts @@ -0,0 +1,120 @@ +import { NextRequest } from "next/server"; +import { describe, expect, it } from "vitest"; +import { LOCALE_URL_HEADER, localeProxy, PATHNAME_HEADER } from "./next.js"; +import { defineLocaleRouting } from "./routing.js"; + +const prefixed = defineLocaleRouting({ + baseUrl: "https://acme.test", + locales: ["en", "fr", "nl"], + defaultLocale: "en", + strategy: "prefix", +}); + +const query = defineLocaleRouting({ + baseUrl: "https://acme.test", + locales: ["en", "fr", "nl"], + defaultLocale: "en", +}); + +const request = (url: string, headers: Record = {}) => + new NextRequest(new URL(url, "https://acme.test"), { headers }); + +/** The request headers the middleware forwarded, as the app would see them. */ +const forwarded = (response: Response, name: string) => + response.headers.get("x-middleware-override-headers")?.includes(name) + ? response.headers.get(`x-middleware-request-${name}`) + : null; + +describe("localeProxy: prefix strategy", () => { + it("rewrites a localized path to the bare route and forwards the locale", () => { + const response = localeProxy(prefixed, request("/fr/about")); + expect(response.headers.get("x-middleware-rewrite")).toBe( + "https://acme.test/about", + ); + expect(forwarded(response, LOCALE_URL_HEADER)).toBe("fr"); + expect(forwarded(response, PATHNAME_HEADER)).toBe("/fr/about"); + }); + + it("keeps the query string across the rewrite", () => { + const response = localeProxy(prefixed, request("/fr/search?q=vat")); + expect(response.headers.get("x-middleware-rewrite")).toBe( + "https://acme.test/search?q=vat", + ); + }); + + it("rewrites a bare locale root to /", () => { + expect( + localeProxy(prefixed, request("/fr")).headers.get("x-middleware-rewrite"), + ).toBe("https://acme.test/"); + }); + + it("does not rewrite or claim a locale on the bare path", () => { + const response = localeProxy(prefixed, request("/about")); + expect(response.headers.get("x-middleware-rewrite")).toBeNull(); + expect(forwarded(response, LOCALE_URL_HEADER)).toBeNull(); + }); + + it("never redirects: every advertised address answers in place", () => { + for (const path of ["/en/about", "/fr/about", "/nl/about", "/about"]) { + expect(localeProxy(prefixed, request(path)).status).toBe(200); + } + }); +}); + +describe("localeProxy: query strategy", () => { + it("forwards the locale without rewriting", () => { + const response = localeProxy(query, request("/about?hl=fr")); + expect(response.headers.get("x-middleware-rewrite")).toBeNull(); + expect(forwarded(response, LOCALE_URL_HEADER)).toBe("fr"); + }); + + it("claims no locale on the bare path", () => { + expect( + forwarded(localeProxy(query, request("/about")), LOCALE_URL_HEADER), + ).toBeNull(); + }); +}); + +describe("localeProxy: cookie and header hygiene", () => { + it("remembers an explicit choice for later bare-path visits", () => { + const response = localeProxy(query, request("/about?hl=nl")); + expect(response.cookies.get("locale")?.value).toBe("nl"); + }); + + it("uses the cookie name from routing, not a second source of truth", () => { + const named = defineLocaleRouting({ + baseUrl: "https://acme.test", + locales: ["en", "fr"], + defaultLocale: "en", + cookieName: "lang", + }); + expect( + localeProxy(named, request("/about?hl=fr")).cookies.get("lang")?.value, + ).toBe("fr"); + }); + + it("writes no cookie when the URL named no locale", () => { + expect( + localeProxy(query, request("/about")).cookies.get("locale"), + ).toBeUndefined(); + }); + + it("strips a client-supplied locale header instead of trusting it", () => { + // The header is ours to mint; a client that sends its own must not reach + // the app, or it can pick the language of a page it does not address. + const response = localeProxy( + query, + request("/about", { [LOCALE_URL_HEADER]: "fr" }), + ); + expect(forwarded(response, LOCALE_URL_HEADER)).toBeNull(); + }); + + it("keeps headers the caller added of its own", () => { + const requestHeaders = new Headers({ "x-tenant": "acme" }); + const response = localeProxy(query, request("/about?hl=fr"), { + requestHeaders, + }); + expect(forwarded(response, "x-tenant")).toBe("acme"); + expect(forwarded(response, LOCALE_URL_HEADER)).toBe("fr"); + }); +}); diff --git a/packages/nk-i18n/src/next.ts b/packages/nk-i18n/src/next.ts index 506a63f..7690dfc 100644 --- a/packages/nk-i18n/src/next.ts +++ b/packages/nk-i18n/src/next.ts @@ -1,15 +1,15 @@ /** - * Next.js wiring for {@link LocaleRouting}: the middleware side that reads the - * locale a URL names, and the server-component side that resolves the locale - * for a request and builds a matching hreflang config. + * Next.js wiring for {@link LocaleRouting}: one middleware helper, the + * server-component resolver, and the matching hreflang config. * - * The rule this module exists to enforce: a URL that names a locale SERVES that - * locale, with a 200. It is never redirected away. Redirecting `?hl=fr` to the - * bare path is the bug that makes every hreflang annotation on the site point at - * a URL which does not serve the language it claims, and Google responds by - * dropping the non-default languages entirely. + * The rule this module enforces: a URL that names a locale SERVES that locale, + * with a 200. It is never redirected away. Redirecting `?hl=fr` (or `/fr/…`) to + * the bare path makes every hreflang annotation on the site point at a URL that + * does not serve the language it claims, and Google drops the non-default + * languages entirely. */ import { cookies, headers } from "next/headers"; +import { type NextRequest, NextResponse } from "next/server"; import type { LocaleRouting, LocaleSupplier } from "./routing.js"; import { resolveLocaleFromSuppliers } from "./routing.js"; @@ -20,32 +20,36 @@ import { resolveLocaleFromSuppliers } from "./routing.js"; */ export const LOCALE_URL_HEADER = "x-nk-url-locale"; +/** + * Request header carrying the pathname, which `@ingram-tech/nk-seo`'s + * `` reads. Set alongside the locale header rather than by hand, + * because two conventions wired separately is how one gets forgotten. + */ +export const PATHNAME_HEADER = "x-pathname"; + /** Vercel's geo header, the default source for the country signal. */ const VERCEL_COUNTRY_HEADER = "x-vercel-ip-country"; +/** Cookie lifetime for a remembered language choice: one year. */ +const COOKIE_MAX_AGE = 60 * 60 * 24 * 365; + /** - * Middleware: read the locale `url` names and forward it on `requestHeaders`. - * Returns it too, for callers that want to branch. + * Set the request headers server components need: the pathname, and the locale + * the URL names (or nothing, on the bare negotiating path). * - * Set-or-delete, never pass through: the header is ours to mint, so a client - * that sends one of its own must not reach the app. + * Set-or-delete, never pass through: these headers are ours to mint, so a + * client that sends one of its own must not reach the app. * - * This deliberately does NOT redirect. Under the `"query"` strategy the bare - * path is a negotiating entry point and `?hl=xx` addresses are the indexable - * per-locale ones; both must return 200. - * - * export function proxy(request: NextRequest) { - * const requestHeaders = new Headers(request.headers); - * forwardUrlLocale(routing, request.nextUrl, requestHeaders); - * return NextResponse.next({ request: { headers: requestHeaders } }); - * } + * Returns the locale the URL named, for callers that want to branch. Most + * middleware should call {@link localeProxy} instead, which wraps this. */ -export function forwardUrlLocale( - routing: LocaleRouting, - url: URL, +export function forwardRequestContext( + routing: LocaleRouting, + request: NextRequest, requestHeaders: Headers, -): string | undefined { - const locale = routing.localeFromUrl(url); +): L | undefined { + requestHeaders.set(PATHNAME_HEADER, request.nextUrl.pathname); + const locale = routing.localeFromUrl(request.nextUrl); if (locale) { requestHeaders.set(LOCALE_URL_HEADER, locale); } else { @@ -54,27 +58,88 @@ export function forwardUrlLocale( return locale; } +export interface LocaleProxyOptions { + /** + * Headers to forward to the app, if the middleware has its own to add. A + * fresh copy of the request's headers is used when omitted; pass your own + * when you also set things like a tenant header. + */ + requestHeaders?: Headers; +} + +/** + * The whole middleware side of locale routing, in one call. + * + * - forwards the pathname and URL-locale headers; + * - under `"prefix"`, rewrites `/fr/about` to `/about` so the app keeps one + * route tree and never learns what a locale is; + * - remembers an explicit choice in the cookie, for the visitor's later visits + * to a bare path. That is a write, not a read: the URL already decided THIS + * request, since it outranks the cookie. + * + * It never redirects. The bare path negotiates and every `/fr/…` or `?hl=fr` + * address serves its language directly, so there is nothing to consolidate. + * + * export function proxy(request: NextRequest) { + * return localeProxy(routing, request); + * } + * + * Middleware that does more of its own work passes its headers in and keeps + * editing the response: + * + * const requestHeaders = new Headers(request.headers); + * requestHeaders.set("x-tenant", tenant); + * const response = localeProxy(routing, request, { requestHeaders }); + * response.cookies.set(…); + * return response; + */ +export function localeProxy( + routing: LocaleRouting, + request: NextRequest, + options: LocaleProxyOptions = {}, +): NextResponse { + const requestHeaders = options.requestHeaders ?? new Headers(request.headers); + const locale = forwardRequestContext(routing, request, requestHeaders); + + const stripped = routing.stripLocale(request.nextUrl.pathname); + const init = { request: { headers: requestHeaders } }; + + const response = + stripped === request.nextUrl.pathname + ? NextResponse.next(init) + : NextResponse.rewrite( + new URL(`${stripped}${request.nextUrl.search}`, request.nextUrl), + init, + ); + + if (locale) { + response.cookies.set(routing.cookieName, locale, { + path: "/", + maxAge: COOKIE_MAX_AGE, + sameSite: "lax", + }); + } + return response; +} + /** - * The locale the current URL names, or `undefined` when it names none — the - * bare negotiating path under the `"query"` strategy. + * The locale the current URL names, or `undefined` on the bare negotiating path. * * This, not the negotiated locale, is what a canonical tag must follow. A - * canonical is a statement about an address; `/pricing` canonicalizes to + * canonical is a statement about an address: `/pricing` canonicalizes to * `/pricing` even while it renders French for a French visitor. */ -export async function getUrlLocale( - routing: LocaleRouting, -): Promise { +export async function getUrlLocale( + routing: LocaleRouting, +): Promise { const value = (await headers()).get(LOCALE_URL_HEADER); - return routing.isLocale(value) && value !== null ? value : undefined; + return routing.isLocale(value) ? value : undefined; } export interface LocaleResolverOptions { - /** Remembered-choice cookie name. Default `"locale"`. */ - cookieName?: string; /** * The signed-in user's stored preference. Only called when the URL did not - * name a locale, so a `?hl=` request costs no database round trip. + * name a locale, so a localized address costs no database round trip. */ account?: LocaleSupplier; /** @@ -86,24 +151,25 @@ export interface LocaleResolverOptions { } /** - * Build the request-scoped locale resolver. Wrap the result in React's `cache()` - * if you call it more than once per render. + * Build the request-scoped locale resolver, narrowed to the site's locale + * union. Wrap the result in React's `cache()` if you call it more than once per + * render. * * export const resolveLocale = cache( * createLocaleResolver(routing, { account: () => getProfile().locale }), * ); */ -export function createLocaleResolver( - routing: LocaleRouting, +export function createLocaleResolver( + routing: LocaleRouting, options: LocaleResolverOptions = {}, -): () => Promise { - const { cookieName = "locale", account, country } = options; +): () => Promise { + const { account, country } = options; return () => resolveLocaleFromSuppliers(routing, { url: async () => (await headers()).get(LOCALE_URL_HEADER), account, - cookie: async () => (await cookies()).get(cookieName)?.value, + cookie: async () => (await cookies()).get(routing.cookieName)?.value, acceptLanguage: async () => (await headers()).get("accept-language"), country: country ?? (async () => (await headers()).get(VERCEL_COUNTRY_HEADER)), @@ -113,30 +179,31 @@ export function createLocaleResolver( /** * The hreflang config for the page being rendered, with `currentLocale` set from * the URL rather than from negotiation. Spread it into `` (from - * `@ingram-tech/nk-seo/components`) or `hreflangAlternates`: + * `@ingram-tech/nk-seo/components`); the pathname comes from the header + * {@link localeProxy} already set, so there is nothing else to wire. * - * + * * * Going through here is what keeps the advertised URLs and the served URLs the * same strings, and what keeps canonicals following the address instead of the * rendered language. */ -export async function hreflangConfigFor(routing: LocaleRouting): Promise<{ +export async function hreflangConfigFor( + routing: LocaleRouting, +): Promise<{ baseUrl: string; locales: readonly string[]; - defaultLocale: string; - strategy: "query" | "prefix"; + strategy: LocaleRouting["strategy"]; param: string; - prefixDefaultLocale: boolean; + hrefLangTags: Readonly>> | undefined; currentLocale: string | undefined; }> { return { baseUrl: routing.baseUrl, locales: routing.locales, - defaultLocale: routing.defaultLocale, strategy: routing.strategy, param: routing.param, - prefixDefaultLocale: routing.prefixDefaultLocale, + hrefLangTags: routing.hrefLangTags, currentLocale: await getUrlLocale(routing), }; } diff --git a/packages/nk-i18n/src/routing.test.ts b/packages/nk-i18n/src/routing.test.ts index 354e53c..e3dc39d 100644 --- a/packages/nk-i18n/src/routing.test.ts +++ b/packages/nk-i18n/src/routing.test.ts @@ -67,16 +67,54 @@ describe("defineLocaleRouting: prefix strategy", () => { strategy: "prefix", }); - it("keeps the default locale on the bare path and prefixes the rest", () => { - expect(prefixed.urlForLocale("/about", "en")).toBe("https://acme.test/about"); + it("gives every locale a prefix, the default included", () => { + // The bare path belongs to no locale, so `en` does not get to own it. + expect(prefixed.urlForLocale("/about", "en")).toBe( + "https://acme.test/en/about", + ); expect(prefixed.urlForLocale("/about", "fr")).toBe( "https://acme.test/fr/about", ); }); + it("treats a bare path as naming no locale, so negotiation decides", () => { + // This is the whole point: returning the default locale here would make it + // the URL signal, which outranks the cookie, so a visitor who chose French + // would snap back to English on the first bare internal link they click. + expect(prefixed.localeFromUrl("https://acme.test/about")).toBeUndefined(); + expect(prefixed.localeFromUrl("https://acme.test/")).toBeUndefined(); + }); + it("reads the locale back out of the pathname", () => { expect(prefixed.localeFromUrl("https://acme.test/fr/about")).toBe("fr"); - expect(prefixed.localeFromUrl("https://acme.test/about")).toBe("en"); + expect(prefixed.localeFromUrl("https://acme.test/en/about")).toBe("en"); + expect(prefixed.localeFromUrl("https://acme.test/fr")).toBe("fr"); + }); + + it("round-trips every locale through its own address", () => { + for (const locale of prefixed.locales) { + expect(prefixed.localeFromUrl(prefixed.urlForLocale("/x", locale))).toBe( + locale, + ); + } + }); + + it("strips the prefix for the app-facing rewrite", () => { + expect(prefixed.stripLocale("/fr/about")).toBe("/about"); + expect(prefixed.stripLocale("/fr")).toBe("/"); + expect(prefixed.stripLocale("/about")).toBe("/about"); + // A path merely starting with the letters must not be mistaken for one. + expect(prefixed.stripLocale("/french-press")).toBe("/french-press"); + }); + + it("does not double-prefix an already-prefixed path", () => { + expect(prefixed.urlForLocale("/fr/about", "nl" as "fr")).toBe( + "https://acme.test/nl/about", + ); + }); + + it("is identity under the query strategy", () => { + expect(routing.stripLocale("/about")).toBe("/about"); }); }); @@ -173,3 +211,77 @@ describe("resolveLocaleFromSuppliers", () => { expect(locale).toBe("nl"); }); }); + +describe("typing", () => { + // Sites used to write their own guard and cast the resolver's result; the + // first consumer of this API had `(await resolve()) as Locale` in it, which + // is the tell that the types were doing no work. + const typed = defineLocaleRouting({ + baseUrl: "https://acme.test", + locales: ["en", "fr"], + defaultLocale: "en", + }); + + it("narrows an unknown value to the site's locale union", () => { + const raw: unknown = "fr"; + if (typed.isLocale(raw)) { + const locale: "en" | "fr" = raw; + expect(locale).toBe("fr"); + } else { + throw new Error("expected the guard to narrow"); + } + }); + + it("returns the union from resolve, not a bare string", () => { + const locale: "en" | "fr" = typed.resolve({ cookie: "fr" }); + expect(locale).toBe("fr"); + }); + + it("narrows localeFromUrl too", () => { + const named: "en" | "fr" | undefined = typed.localeFromUrl( + "https://acme.test/x?hl=fr", + ); + expect(named).toBe("fr"); + }); +}); + +describe("hreflang tags and html lang", () => { + const regional = defineLocaleRouting({ + baseUrl: "https://acme.test", + locales: ["en", "fr", "nl"], + defaultLocale: "en", + hrefLangTags: { en: "en-BE", fr: "fr-BE", nl: "nl-BE" }, + }); + + it("uses the regional tag for when one is set", () => { + expect(regional.htmlLang("fr")).toBe("fr-BE"); + }); + + it("falls back to the plain locale when no tag is set", () => { + expect(routing.htmlLang("fr")).toBe("fr"); + }); + + it("carries the tags on the routing object, so no second config is needed", () => { + // A site with regional tags used to build one object for serving and + // another for hreflang, which is precisely the drift this prevents. + expect(regional.hrefLangTags).toEqual({ + en: "en-BE", + fr: "fr-BE", + nl: "nl-BE", + }); + }); +}); + +describe("cookie name", () => { + it("lives on routing, so middleware and resolver cannot disagree", () => { + expect(routing.cookieName).toBe("locale"); + expect( + defineLocaleRouting({ + baseUrl: "https://acme.test", + locales: ["en"], + defaultLocale: "en", + cookieName: "lang", + }).cookieName, + ).toBe("lang"); + }); +}); diff --git a/packages/nk-i18n/src/routing.ts b/packages/nk-i18n/src/routing.ts index 991d40d..1d84306 100644 --- a/packages/nk-i18n/src/routing.ts +++ b/packages/nk-i18n/src/routing.ts @@ -14,51 +14,64 @@ import { negotiateAcceptLanguage } from "./negotiate.js"; * * A {@link LocaleRouting} is deliberately shaped so it can be handed straight * to `hreflangAlternates` as its config: one object owns the locale list, the - * default, the strategy and the param name, so the advertised URL and the - * served URL are the same string by construction. + * default, the strategy, the param name and the hreflang tags, so the + * advertised URL and the served URL are the same string by construction. */ /** - * How a locale is encoded in a URL. + * How a locale is encoded in a URL. This is the ONLY thing that varies between + * sites; the cluster's shape does not (see {@link LocaleRouting}). * - * - `"query"`: every locale gets `?=`, and the bare path is a - * negotiating entry point that belongs to no locale (it is `x-default`). - * - `"prefix"`: the default locale lives at the bare path and the rest get - * `//…`, unless {@link LocaleRoutingConfig.prefixDefaultLocale}. + * - `"query"`: `?=`. No routing work, and the option Google + * supports but does not recommend — parameters can be folded as duplicates + * and there has been no URL Parameters tool to override that since 2022. + * - `"prefix"`: `//…`. Better on every SEO axis (a path cannot be + * folded into another document, survives link-sharing that strips query + * strings, and puts the target-language keyword in the URL), at the cost of + * a route segment or a middleware rewrite. + * + * Prefer `"prefix"` for a new site. Use `"query"` when restructuring routes is + * not worth it, knowing it is the weaker of the two. */ export type LocaleStrategy = "query" | "prefix"; -export interface LocaleRoutingConfig { +export interface LocaleRoutingConfig { /** Absolute site origin, e.g. "https://acme.example". */ baseUrl: string; /** Every supported locale, e.g. `["en", "fr", "nl"]`. */ - locales: readonly string[]; - /** - * The locale served when no signal says otherwise. Under `"query"` it is - * NOT the owner of the bare path: the bare path negotiates and `x-default` - * points at it, while the default locale gets its own `?=` address - * like every other locale. - */ - defaultLocale: string; - /** Default `"query"`. */ + locales: readonly L[]; + /** The locale served when no signal says otherwise. */ + defaultLocale: L; + /** Default `"query"`. See {@link LocaleStrategy}; prefer `"prefix"`. */ strategy?: LocaleStrategy; /** Query-param name for the `"query"` strategy. Default `"hl"`. */ param?: string; - /** `"prefix"` only: prefix the default locale too (`/en/about`). */ - prefixDefaultLocale?: boolean; + /** Remembered-choice cookie. Default `"locale"`. */ + cookieName?: string; /** * ISO-3166 alpha-2 country → locale, for the last-resort country signal. * Omit a country whose language is genuinely ambiguous (Belgium is the * obvious one: geography tells you nothing about whether a visitor reads - * French or Dutch) so it falls through to {@link defaultLocale} instead of - * guessing. Countries absent from the map are ignored. + * French or Dutch) so it falls through instead of guessing. Countries + * absent from the map are ignored. */ - countryLocales?: Readonly>; + countryLocales?: Readonly>; + /** + * Optional locale → hreflang tag, e.g. `{ en: "en-BE", fr: "fr-BE" }`. Lives + * here rather than on the SEO config so a site with regional tags does not + * have to build a second object — that second object is exactly the drift + * this package exists to prevent. Also the value to put in ``, + * via {@link LocaleRouting.htmlLang}. + * + * Only use region tags when the content genuinely differs by country. They + * fragment the cluster and cut you out of neighbouring markets otherwise. + */ + hrefLangTags?: Readonly>>; } /** * The signals a locale can be decided from, in no particular order — the order - * is {@link resolveLocaleFromSignals}'s to own, not the caller's. + * is {@link LOCALE_PRECEDENCE}'s to own, not the caller's. */ export interface LocaleSignals { /** The locale the URL itself names (the `?hl=` value, or a path prefix). */ @@ -73,25 +86,57 @@ export interface LocaleSignals { country?: string | null | undefined; } -export interface LocaleRouting extends Required< - Omit -> { - countryLocales: Readonly>; - /** Narrow an arbitrary value (cookie, header, DB column) to a locale. */ - isLocale: (value: unknown) => boolean; +/** + * The routing definition. Hand it to BOTH your locale resolver and your + * hreflang config — a `LocaleRouting` is a valid `HreflangConfig`, so the URL + * you advertise and the URL you serve cannot drift. + * + * **The cluster shape is fixed and not configurable**, whichever strategy you + * pick: + * + * - every locale has its own address (`?hl=en` / `/en/…`), the default + * included; + * - the bare path belongs to NO locale. It negotiates, and it is `x-default`. + * + * The two shapes this deliberately does not offer are the ones that go wrong. + * A bare path that IS the default locale serves English to a French visitor who + * followed a bare internal link — and every site that starts with a cookie + * switcher has bare internal links. A bare path that redirects on perceived + * language is what Google tells you not to build, and makes `x-default` point + * at a URL that is not language-neutral. Offering either as an option is how + * the fleet drifts, so neither is offered. + */ +export interface LocaleRouting { + baseUrl: string; + locales: readonly L[]; + defaultLocale: L; + strategy: LocaleStrategy; + param: string; + cookieName: string; + countryLocales: Readonly>; + hrefLangTags?: Readonly>>; + /** Type guard, so sites don't each write their own. */ + isLocale: (value: unknown) => value is L; /** - * The locale this URL *names*, or `undefined` when it names none (the bare - * negotiating path under `"query"`). This is the value canonical tags must - * follow: a canonical is a statement about an address, not about whichever - * language negotiation happened to render. + * The locale this URL *names*, or `undefined` for the bare path, which names + * none. This is the value canonical tags must follow: a canonical is a + * statement about an address, not about whichever language negotiation + * happened to render. */ - localeFromUrl: (url: URL | string) => string | undefined; + localeFromUrl: (url: URL | string) => L | undefined; /** The absolute address that always serves `locale`. */ - urlForLocale: (pathname: string, locale: string) => string; - /** The absolute bare address — `x-default` under `"query"`. */ + urlForLocale: (pathname: string, locale: L) => string; + /** The absolute bare address — the negotiating entry point, `x-default`. */ bareUrl: (pathname: string) => string; + /** + * The app-facing pathname with any locale prefix removed, for the middleware + * rewrite. Identity under `"query"`. + */ + stripLocale: (pathname: string) => string; + /** The `` value: the regional tag if one is set, else the locale. */ + htmlLang: (locale: L) => string; /** Apply the fixed precedence to a set of signals. */ - resolve: (signals: LocaleSignals) => string; + resolve: (signals: LocaleSignals) => L; } /** Resolve `path` against `baseUrl`, refusing anything that escapes the origin. */ @@ -109,10 +154,10 @@ const absolute = (path: string, baseUrl: string): string => { /** * The one order every Ingram site decides a locale in: * - * 1. the URL (`?hl=fr`) — an address that names a language always wins, - * including over a signed-in user's stored preference. A shared link must - * show the recipient the language it names, or the link is a lie and the - * hreflang annotation pointing at it is too. + * 1. the URL (`?hl=fr`, `/fr/…`) — an address that names a language always + * wins, including over a signed-in user's stored preference. A shared link + * must show the recipient the language it names, or the link is a lie and + * the hreflang annotation pointing at it is too. * 2. the account's stored preference * 3. the remembered-choice cookie * 4. `Accept-Language` @@ -134,8 +179,8 @@ export const LOCALE_PRECEDENCE = [ export type LocaleSignal = (typeof LOCALE_PRECEDENCE)[number]; -type RoutingSlice = Pick< - LocaleRouting, +type RoutingSlice = Pick< + LocaleRouting, "locales" | "defaultLocale" | "countryLocales" | "isLocale" >; @@ -144,11 +189,11 @@ type RoutingSlice = Pick< * code and only need narrowing; `acceptLanguage` is a header to negotiate and * `country` is an ISO code to look up. */ -const normalize = ( +const normalize = ( signal: LocaleSignal, raw: string | null | undefined, - routing: RoutingSlice, -): string | undefined => { + routing: RoutingSlice, +): L | undefined => { if (raw === null || raw === undefined || raw === "") return undefined; if (signal === "acceptLanguage") { const negotiated = negotiateAcceptLanguage(raw, routing.locales); @@ -162,10 +207,10 @@ const normalize = ( }; /** Apply {@link LOCALE_PRECEDENCE} to already-gathered signal values. */ -export function resolveLocaleFromSignals( - routing: RoutingSlice, +export function resolveLocaleFromSignals( + routing: RoutingSlice, signals: LocaleSignals, -): string { +): L { for (const signal of LOCALE_PRECEDENCE) { const locale = normalize(signal, signals[signal], routing); if (locale) return locale; @@ -187,10 +232,10 @@ export type LocaleSuppliers = Partial>; * first that yields a locale. Suppliers later in the chain are never called, so * a URL that names its language costs no database round trip. */ -export async function resolveLocaleFromSuppliers( - routing: RoutingSlice, +export async function resolveLocaleFromSuppliers( + routing: RoutingSlice, suppliers: LocaleSuppliers, -): Promise { +): Promise { for (const signal of LOCALE_PRECEDENCE) { const supplier = suppliers[signal]; if (!supplier) continue; @@ -200,20 +245,19 @@ export async function resolveLocaleFromSuppliers( return routing.defaultLocale; } -/** - * Build the routing definition. Hand the result to BOTH your locale resolver - * and your hreflang config — a `LocaleRouting` is a valid `HreflangConfig`, so - * the URL you advertise and the URL you serve cannot drift. - */ -export function defineLocaleRouting(config: LocaleRoutingConfig): LocaleRouting { +/** Build the routing definition. See {@link LocaleRouting}. */ +export function defineLocaleRouting( + config: LocaleRoutingConfig, +): LocaleRouting { const { baseUrl, locales, defaultLocale, strategy = "query", param = "hl", - prefixDefaultLocale = false, - countryLocales = {}, + cookieName = "locale", + countryLocales = {} as Readonly>, + hrefLangTags, } = config; if (!locales.includes(defaultLocale)) { @@ -222,48 +266,56 @@ export function defineLocaleRouting(config: LocaleRoutingConfig): LocaleRouting ); } - const isLocale = (value: unknown): boolean => - typeof value === "string" && locales.includes(value); + const isLocale = (value: unknown): value is L => + typeof value === "string" && (locales as readonly string[]).includes(value); const bareUrl = (pathname: string): string => absolute(pathname, baseUrl); - const urlForLocale = (pathname: string, locale: string): string => { + /** The locale segment at the head of `pathname`, if any. */ + const prefixOf = (pathname: string): L | undefined => + locales.find( + (locale) => pathname === `/${locale}` || pathname.startsWith(`/${locale}/`), + ); + + const stripLocale = (pathname: string): string => { + if (strategy !== "prefix") return pathname; + const locale = prefixOf(pathname); + return locale ? pathname.slice(locale.length + 1) || "/" : pathname; + }; + + const urlForLocale = (pathname: string, locale: L): string => { + const base = stripLocale(pathname); + // Every locale gets its own address, the default included: the bare path + // is the negotiating entry point and belongs to none of them. if (strategy === "prefix") { - if (locale === defaultLocale && !prefixDefaultLocale) - return bareUrl(pathname); - return absolute(`/${locale}${pathname === "/" ? "" : pathname}`, baseUrl); + return absolute(`/${locale}${base === "/" ? "" : base}`, baseUrl); } - const bare = bareUrl(pathname); + const bare = bareUrl(base); return `${bare}${bare.includes("?") ? "&" : "?"}${param}=${locale}`; }; - const localeFromUrl = (url: URL | string): string | undefined => { + const localeFromUrl = (url: URL | string): L | undefined => { const parsed = typeof url === "string" ? new URL(url, baseUrl) : url; - if (strategy === "prefix") { - const found = locales.find( - (locale) => - parsed.pathname === `/${locale}` || - parsed.pathname.startsWith(`/${locale}/`), - ); - if (found) return found; - return prefixDefaultLocale ? undefined : defaultLocale; - } + if (strategy === "prefix") return prefixOf(parsed.pathname); const value = parsed.searchParams.get(param); - return isLocale(value) && value !== null ? value : undefined; + return isLocale(value) ? value : undefined; }; - const routing: LocaleRouting = { + const routing: LocaleRouting = { baseUrl, locales, defaultLocale, strategy, param, - prefixDefaultLocale, + cookieName, countryLocales, + hrefLangTags, isLocale, localeFromUrl, urlForLocale, bareUrl, + stripLocale, + htmlLang: (locale) => hrefLangTags?.[locale] ?? locale, resolve: (signals) => resolveLocaleFromSignals(routing, signals), }; return routing; diff --git a/packages/nk-seo/README.md b/packages/nk-seo/README.md index 6085e5f..77eae2e 100644 --- a/packages/nk-seo/README.md +++ b/packages/nk-seo/README.md @@ -316,21 +316,12 @@ import { HreflangLinks } from "@ingram-tech/nk-seo/components"; /> ``` -Sites that prefix **every** locale — where `/about` redirects to `/en/about` -and no bare path exists — pass `prefixDefaultLocale`, which prefixes the default -locale too and points `x-default` at that prefixed URL. Without it the bare path -is emitted for both `en` and `x-default`, annotating URLs that redirect (Google -wants every hreflang target to answer 200 directly). - -```tsx - -``` +The cluster's **shape** is fixed and not configurable: every locale gets its own +address, the default included, and the bare path belongs to no locale — it is +the negotiating entry point that `x-default` names. `strategy` picks only the +encoding. `@ingram-tech/nk-i18n`'s `defineLocaleRouting` produces a valid config +for this, and `hreflangConfigFor(routing)` fills in `currentLocale` and the +pathname header correctly; prefer that over assembling a config by hand. Pass `pathname` explicitly if you don't use the `x-pathname` header. When neither is available the component **throws** instead of guessing — a silent diff --git a/packages/nk-seo/src/alternates.test.ts b/packages/nk-seo/src/alternates.test.ts index 69259ff..00daae4 100644 --- a/packages/nk-seo/src/alternates.test.ts +++ b/packages/nk-seo/src/alternates.test.ts @@ -25,21 +25,9 @@ describe("hreflangAlternates", () => { expect(links[0]?.href).toBe("https://acme.test/?lang=en"); }); - it("prefix strategy: keeps the default locale bare and prefixes the rest", () => { - const { links } = hreflangAlternates( - { baseUrl, locales: ["en", "fr"], strategy: "prefix", defaultLocale: "en" }, - "/about", - ); - expect(links).toEqual([ - { hrefLang: "en", href: "https://acme.test/about" }, - { hrefLang: "fr", href: "https://acme.test/fr/about" }, - { hrefLang: "x-default", href: "https://acme.test/about" }, - ]); - }); - it("prefix strategy: the root path gets bare locale prefixes", () => { const { links } = hreflangAlternates( - { baseUrl, locales: ["en", "fr"], strategy: "prefix", defaultLocale: "en" }, + { baseUrl, locales: ["en", "fr"], strategy: "prefix" }, "/", ); expect(links[1]?.href).toBe("https://acme.test/fr"); @@ -63,7 +51,6 @@ describe("hreflangAlternates", () => { baseUrl: "https://acme.test/", locales: ["en", "fr"], strategy: "prefix", - defaultLocale: "en", }, "/about", ); @@ -71,100 +58,12 @@ describe("hreflangAlternates", () => { expect(links[1]?.href).toBe("https://acme.test/fr/about"); }); - it("prefix strategy: strips an existing locale prefix instead of double-prefixing", () => { - // Middleware's x-pathname carries the real (prefixed) path on a localized - // route; blindly prepending produced /fr/fr/about + /en/fr/about. - const { canonical, links } = hreflangAlternates( - { baseUrl, locales: ["en", "fr"], strategy: "prefix", defaultLocale: "en" }, - "/fr/about", - ); - expect(links).toEqual([ - { hrefLang: "en", href: "https://acme.test/about" }, - { hrefLang: "fr", href: "https://acme.test/fr/about" }, - { hrefLang: "x-default", href: "https://acme.test/about" }, - ]); - // …and the canonical self-references the French variant being rendered. - expect(canonical).toBe("https://acme.test/fr/about"); - }); - - it("prefixDefaultLocale: prefixes every locale and points x-default at the default one", () => { - // Sites whose negotiation redirects bare paths have no unprefixed - // variant: emitting one would annotate a URL that 3xx-redirects. - const { canonical, links } = hreflangAlternates( - { - baseUrl, - locales: ["en", "fr"], - strategy: "prefix", - defaultLocale: "en", - prefixDefaultLocale: true, - }, - "/about", - ); - expect(links).toEqual([ - { hrefLang: "en", href: "https://acme.test/en/about" }, - { hrefLang: "fr", href: "https://acme.test/fr/about" }, - { hrefLang: "x-default", href: "https://acme.test/en/about" }, - ]); - expect(canonical).toBe("https://acme.test/en/about"); - }); - - it("prefixDefaultLocale: the default locale's own page canonicalizes to its prefixed URL", () => { - const { canonical } = hreflangAlternates( - { - baseUrl, - locales: ["en", "fr"], - strategy: "prefix", - defaultLocale: "en", - prefixDefaultLocale: true, - }, - "/en/about", - ); - expect(canonical).toBe("https://acme.test/en/about"); - }); - - it("prefixDefaultLocale: the root path prefixes without a trailing slash", () => { - const { links } = hreflangAlternates( - { - baseUrl, - locales: ["en", "fr"], - strategy: "prefix", - defaultLocale: "en", - prefixDefaultLocale: true, - }, - "/", - ); - expect(links.map((l) => l.href)).toEqual([ - "https://acme.test/en", - "https://acme.test/fr", - "https://acme.test/en", - ]); - }); - - it("exposes the links keyed by hreflang for Metadata.alternates.languages", () => { - const { languages } = hreflangAlternates( - { baseUrl, locales: ["en", "fr"], strategy: "prefix", defaultLocale: "en" }, - "/about", - ); - expect(languages).toEqual({ - en: "https://acme.test/about", - fr: "https://acme.test/fr/about", - "x-default": "https://acme.test/about", - }); - }); - - it("prefix strategy: requires defaultLocale", () => { - expect(() => - hreflangAlternates({ baseUrl, locales: ["en"], strategy: "prefix" }, "/p"), - ).toThrow(/defaultLocale/); - }); - it("query strategy: canonical self-references the variant when currentLocale is passed", () => { // A variant canonicalizing to another URL makes Google drop the cluster. const { canonical } = hreflangAlternates( { baseUrl, locales: ["en", "fr"], - defaultLocale: "en", currentLocale: "fr", }, "/about", @@ -192,8 +91,79 @@ describe("hreflangAlternates", () => { }); }); +describe("hreflangAlternates: prefix strategy", () => { + const config = { + baseUrl, + locales: ["en", "fr", "nl"], + strategy: "prefix" as const, + }; + + it("gives every locale its own prefix, the default included", () => { + // The bare path belongs to no locale, so `en` does NOT get it. + const { links } = hreflangAlternates(config, "/about"); + expect(links).toEqual([ + { hrefLang: "en", href: "https://acme.test/en/about" }, + { hrefLang: "fr", href: "https://acme.test/fr/about" }, + { hrefLang: "nl", href: "https://acme.test/nl/about" }, + { hrefLang: "x-default", href: "https://acme.test/about" }, + ]); + }); + + it("points x-default at the bare negotiating path", () => { + const { languages } = hreflangAlternates(config, "/about"); + expect(languages["x-default"]).toBe("https://acme.test/about"); + }); + + it("strips an existing locale prefix instead of double-prefixing", () => { + const { links, canonical } = hreflangAlternates(config, "/fr/about"); + expect(links[1]?.href).toBe("https://acme.test/fr/about"); + expect(links[0]?.href).toBe("https://acme.test/en/about"); + // The locale is detected from the path, so the canonical self-references. + expect(canonical).toBe("https://acme.test/fr/about"); + }); + + it("prefixes the root path without a trailing slash", () => { + const { links } = hreflangAlternates(config, "/"); + expect(links[0]?.href).toBe("https://acme.test/en"); + expect(links[3]?.href).toBe("https://acme.test/"); + }); + + it("canonicalizes a bare path to itself, naming no locale", () => { + expect(hreflangAlternates(config, "/about").canonical).toBe( + "https://acme.test/about", + ); + }); + + it("emits the same cluster shape as the query strategy", () => { + // The encoding differs; the shape must not. Both give every locale its own + // address and reserve the bare path for x-default. + const asQuery = hreflangAlternates( + { baseUrl, locales: config.locales }, + "/about", + ); + const asPrefix = hreflangAlternates(config, "/about"); + expect(asPrefix.links.map((l) => l.hrefLang)).toEqual( + asQuery.links.map((l) => l.hrefLang), + ); + expect(asPrefix.languages["x-default"]).toBe(asQuery.languages["x-default"]); + }); + + it("exposes the links keyed by hreflang for Metadata.alternates.languages", () => { + const { languages } = hreflangAlternates( + { ...config, hrefLangTags: { en: "en-BE", fr: "fr-BE", nl: "nl-BE" } }, + "/about", + ); + expect(languages).toEqual({ + "en-BE": "https://acme.test/en/about", + "fr-BE": "https://acme.test/fr/about", + "nl-BE": "https://acme.test/nl/about", + "x-default": "https://acme.test/about", + }); + }); +}); + describe("hreflangAlternates: canonical follows the address, not the language", () => { - const config = { baseUrl, locales: ["en", "fr", "nl"], defaultLocale: "en" }; + const config = { baseUrl, locales: ["en", "fr", "nl"] }; it("query strategy: the default locale canonicalizes to its own param URL", () => { // The bare path is the negotiating entry point and belongs to no locale, @@ -229,27 +199,6 @@ describe("hreflangAlternates: canonical follows the address, not the language", expect(languages.en).toBe("https://acme.test/pricing?hl=en"); }); - it("prefix strategy: the default locale still canonicalizes to the bare path", () => { - const { canonical } = hreflangAlternates( - { ...config, strategy: "prefix", currentLocale: "en" }, - "/pricing", - ); - expect(canonical).toBe("https://acme.test/pricing"); - }); - - it("prefix strategy with prefixDefaultLocale: canonical is the prefixed URL", () => { - const { canonical } = hreflangAlternates( - { - ...config, - strategy: "prefix", - prefixDefaultLocale: true, - currentLocale: "en", - }, - "/pricing", - ); - expect(canonical).toBe("https://acme.test/en/pricing"); - }); - it("every advertised URL round-trips to itself as canonical", () => { const { links } = hreflangAlternates(config, "/pricing"); for (const [index, locale] of config.locales.entries()) { diff --git a/packages/nk-seo/src/alternates.ts b/packages/nk-seo/src/alternates.ts index 3177526..7de612b 100644 --- a/packages/nk-seo/src/alternates.ts +++ b/packages/nk-seo/src/alternates.ts @@ -12,42 +12,35 @@ export interface HreflangConfig { locales: readonly string[]; /** * How locale is encoded in the alternate URLs: - * - `"query"` (default): `${baseUrl}${path}?${param}=${locale}` for every locale. - * - `"prefix"`: the default locale stays at the bare path; others get - * `/${locale}${path}` (matches a localized-rewrite setup). Set - * {@link HreflangConfig.prefixDefaultLocale} for sites that prefix every - * locale instead. + * - `"query"` (default): `${baseUrl}${path}?${param}=${locale}` + * - `"prefix"`: `/${locale}${path}` + * + * The cluster's SHAPE is the same either way and is not configurable: every + * locale gets its own address, the default included, and the bare path + * belongs to no locale — it is the negotiating entry point that `x-default` + * names. `@ingram-tech/nk-i18n`'s `defineLocaleRouting` produces a valid + * config for this; prefer passing that over assembling one by hand. */ strategy?: "query" | "prefix"; /** Query-param name for the `"query"` strategy. Default `"hl"`. */ param?: string; - /** Default locale. Required for `"prefix"`; it is what `x-default` resolves to. */ - defaultLocale?: string; - /** - * `"prefix"` only: prefix the default locale too (`/en/about`, never - * `/about`), and point `x-default` at that prefixed URL. For sites whose - * locale negotiation redirects every bare path — emitting the bare path as - * an alternate would annotate a URL that 3xx-redirects, which is the bug the - * mandatory `defaultLocale` exists to prevent. - */ - prefixDefaultLocale?: boolean; /** * The locale **the URL names**, which is not always the locale that rendered. * It determines the self-referencing canonical, and a canonical is a claim - * about an address: under `"query"`, `/pricing` canonicalizes to `/pricing` - * even while content negotiation renders it in French for a French visitor, - * because `/pricing` is the negotiating entry point and belongs to no locale. - * Passing the negotiated locale here instead makes the bare path claim to be - * the French URL, and the real French URL then looks like a duplicate. + * about an address: `/pricing` canonicalizes to `/pricing` even while content + * negotiation renders it in French for a French visitor, because `/pricing` + * belongs to no locale. Passing the negotiated locale here instead makes the + * bare path claim to be the French URL, and the real French URL then looks + * like a duplicate of it. * - * Leave it unset on a negotiating bare path. Under `"prefix"` it is detected - * from the pathname; under `"query"` the server cannot see the query string, - * so pass it — `hreflangConfigFor(routing)` from - * "@ingram-tech/nk-i18n/next" fills it in correctly. + * Leave it unset on the bare path. Under `"prefix"` it is detected from the + * pathname; under `"query"` the server cannot see the query string, so pass + * it — `hreflangConfigFor(routing)` from "@ingram-tech/nk-i18n/next" fills it + * in correctly. */ currentLocale?: string; /** Optional locale → hreflang tag map, e.g. `{ en: "en-BE", fr: "fr-BE" }`. */ - hrefLangTags?: Record; + hrefLangTags?: Readonly>>; } export interface HreflangLink { @@ -77,27 +70,11 @@ export function hreflangAlternates( config: HreflangConfig, pathname: string, ): HreflangAlternates { - const { - strategy = "query", - param = "hl", - defaultLocale, - prefixDefaultLocale, - hrefLangTags, - } = config; - if (strategy === "prefix" && !defaultLocale) { - // Without it canonical/x-default point at a bare path that is no - // locale's URL — a silent SEO bug. (With `prefixDefaultLocale` no bare - // path is emitted at all, but x-default still has to resolve to *some* - // locale, so the requirement stands.) - throw new Error( - "hreflangAlternates: `defaultLocale` is required for the prefix strategy.", - ); - } + const { strategy = "query", param = "hl", hrefLangTags } = config; - // Prefix strategy: accept both the bare and the locale-prefixed form of the - // path (middleware's `x-pathname` carries the latter on a real localized - // route — blindly prepending would emit /fr/fr/about) and detect the - // current locale from it. + // Accept both the bare and the locale-prefixed form of the path (middleware's + // `x-pathname` carries the latter on a real localized route — blindly + // prepending would emit /fr/fr/about) and detect the current locale from it. let basePath = pathname; let currentLocale = config.currentLocale; if (strategy === "prefix") { @@ -108,54 +85,38 @@ export function hreflangAlternates( break; } } - currentLocale ??= defaultLocale; + // Deliberately no fallback to a default locale: a bare path names none. } - /** The bare (locale-free) URL of the path. */ + /** The bare (locale-free) URL — the negotiating entry point, `x-default`. */ const bareUrl = absoluteUrl(basePath, config.baseUrl); - const prefixedUrl = (locale: string): string => - absoluteUrl(`/${locale}${basePath === "/" ? "" : basePath}`, config.baseUrl); - + /** Every locale has its own address, the default included. */ const hrefFor = (locale: string): string => { if (strategy === "prefix") { - if (locale === defaultLocale && !prefixDefaultLocale) return bareUrl; - return prefixedUrl(locale); + return absoluteUrl( + `/${locale}${basePath === "/" ? "" : basePath}`, + config.baseUrl, + ); } return `${bareUrl}${bareUrl.includes("?") ? "&" : "?"}${param}=${locale}`; }; - /** x-default: the default locale's own URL, prefixed or not. */ - const defaultUrl = - strategy === "prefix" && prefixDefaultLocale && defaultLocale - ? prefixedUrl(defaultLocale) - : bareUrl; - - // Self-referencing canonical: the current variant's own URL. Canonicalizing - // a localized variant to the bare path makes Google treat the variants as - // duplicates and ignore the hreflang annotations entirely. - // - // Whether the default locale has a URL of its own is strategy-dependent, and - // conflating the two is a silent way to delete the default locale from the - // cluster. Under `"prefix"` it shares the bare path (unless every locale is - // prefixed), so its canonical IS the bare path. Under `"query"` every locale - // gets its own `?param=` address and the bare path belongs to none of them: - // it is the negotiating entry point that `x-default` names. So `?hl=en` must - // canonicalize to `?hl=en`, never to the bare path. - const defaultLocaleOwnsBarePath = strategy === "prefix" && !prefixDefaultLocale; + // Self-referencing canonical: the current variant's own URL, or the bare path + // when the URL names no locale. Canonicalizing a localized variant to the + // bare path makes Google treat the variants as duplicates and ignore the + // hreflang annotations entirely. const canonical = - currentLocale && - config.locales.includes(currentLocale) && - !(defaultLocaleOwnsBarePath && currentLocale === defaultLocale) + currentLocale && config.locales.includes(currentLocale) ? hrefFor(currentLocale) - : defaultUrl; + : bareUrl; const links = [ ...config.locales.map((locale) => ({ hrefLang: hrefLangTags?.[locale] ?? locale, href: hrefFor(locale), })), - { hrefLang: "x-default", href: defaultUrl }, + { hrefLang: "x-default", href: bareUrl }, ]; return {