From 7e74222c5cd1c4d24b4bb889163d5396e088dab2 Mon Sep 17 00:00:00 2001 From: Jerome Leclanche Date: Fri, 28 Aug 2026 20:22:46 +0200 Subject: [PATCH 1/2] nk-dev: type-check goes cold when the lockfile moved; doctor flags Prettier leftovers and a missing ci script Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_012aTVvXFCsmSt8Lv83YM458 --- .changeset/nk-dev-cold-type-check.md | 12 +++ packages/nk-dev/bin/nk.js | 7 +- packages/nk-dev/lib/artifacts.js | 34 ++++++++- packages/nk-dev/lib/doctor.js | 76 ++++++++++++++++--- packages/nk-dev/lib/passthrough.js | 39 +++++++++- packages/nk-dev/test/artifacts.test.js | 29 ++++++- .../nk-dev/test/doctor-prettier-ci.test.js | 65 ++++++++++++++++ 7 files changed, 244 insertions(+), 18 deletions(-) create mode 100644 .changeset/nk-dev-cold-type-check.md create mode 100644 packages/nk-dev/test/doctor-prettier-ci.test.js 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/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(); + }); +}); From 254c08802e539e18b792f6f1303c9f9a9d8ea1a3 Mon Sep 17 00:00:00 2001 From: Jerome Leclanche Date: Fri, 28 Aug 2026 22:57:02 +0200 Subject: [PATCH 2/2] i18n: make the locale cluster shape fixed, and middleware one call MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Feedback from a fleet site that adopted the prefix strategy: it had to fork the wiring. Under prefix, localeFromUrl returned defaultLocale for a bare path, and 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. Every site that starts with a cookie switcher has bare internal links. The fix is not another option. The cluster's shape stops being configurable: every locale has its own address, the default included, and the bare path belongs to none of them. prefixDefaultLocale is removed with no replacement. The two shapes now excluded are the two that go wrong, and offering either is how a fleet drifts. localeProxy is the whole middleware side, replacing the strip/rewrite/cookie code each prefix site was hand-writing, and it sets nk-seo's x-pathname alongside the locale header so the two can't be wired separately. Routing is generic over the locale union, so sites stop writing their own guards and casts, and hrefLangTags/cookieName move onto routing so a site with regional tags no longer builds the second config object this package exists to prevent. Co-Authored-By: Claude Opus 5 (1M context) --- .changeset/locale-cluster-shape.md | 44 +++++ docs/i18n-routing.md | 87 +++++++--- packages/nk-i18n/README.md | 92 +++++++---- packages/nk-i18n/src/next.test.ts | 120 ++++++++++++++ packages/nk-i18n/src/next.ts | 169 ++++++++++++++------ packages/nk-i18n/src/routing.test.ts | 118 +++++++++++++- packages/nk-i18n/src/routing.ts | 212 +++++++++++++++---------- packages/nk-seo/README.md | 21 +-- packages/nk-seo/src/alternates.test.ts | 197 +++++++++-------------- packages/nk-seo/src/alternates.ts | 111 +++++-------- 10 files changed, 769 insertions(+), 402 deletions(-) create mode 100644 .changeset/locale-cluster-shape.md create mode 100644 packages/nk-i18n/src/next.test.ts 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/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-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 {