i18n: fixed locale cluster shape, one middleware call - #44
Merged
Conversation
…ettier leftovers and a missing ci script Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012aTVvXFCsmSt8Lv83YM458
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) <noreply@anthropic.com>
Copilot started reviewing on behalf of
Jerome Leclanche (jleclanche)
August 28, 2026 20:58
View session
There was a problem hiding this comment.
Pull request overview
Updates @ingram-tech/nk-i18n and @ingram-tech/nk-seo to enforce a single, non-configurable locale cluster shape (bare path is always negotiating x-default; every locale—including the default—has its own address) and to reduce middleware wiring to a single helper (localeProxy) that forwards headers, rewrites prefix URLs, and persists explicit choices to the cookie.
Changes:
- Remove
prefixDefaultLocale/defaultLocaleSEO shape options; align prefix/query strategies to differ only by encoding while keeping the same cluster shape. - Add
localeProxy()+forwardRequestContext()Next.js helpers and expandLocaleRouting(cookie name, hreflang tags, stripLocale, htmlLang) with stronger locale-union typing throughout routing/resolution. - Improve nk-dev ergonomics: cold/stale type-check behavior, expanded doctor checks (Prettier leftovers + CI script presence/content), and new tests/docs/changesets to match.
Reviewed changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/nk-seo/src/alternates.ts | Removes configurable cluster-shape fields; makes prefix strategy always generate per-locale prefixed URLs and x-default as bare. |
| packages/nk-seo/src/alternates.test.ts | Replaces old prefix-default-locale test matrix with coverage for the fixed cluster shape and canonical behavior. |
| packages/nk-seo/README.md | Updates docs to reflect fixed cluster shape and recommends using nk-i18n routing helpers for config. |
| packages/nk-i18n/src/routing.ts | Makes routing generic over locale union; adds cookieName/hrefLangTags/stripLocale/htmlLang; fixes prefix strategy to treat bare path as naming no locale. |
| packages/nk-i18n/src/routing.test.ts | Updates prefix-strategy expectations and adds typing assertions + new helpers coverage (stripLocale, htmlLang, cookieName). |
| packages/nk-i18n/src/next.ts | Introduces localeProxy + forwardRequestContext; wires pathname + url-locale headers together; aligns resolver to routing.cookieName. |
| packages/nk-i18n/src/next.test.ts | Adds middleware-level tests for rewriting, query preservation, cookie persistence, and header spoofing protection. |
| packages/nk-i18n/README.md | Updates public guidance: fixed cluster shape, new middleware helper, and html/hreflang wiring. |
| packages/nk-dev/test/doctor-prettier-ci.test.js | Adds tests for new doctor findings (Prettier leftovers + CI script checks). |
| packages/nk-dev/test/artifacts.test.js | Adds tests for stale .tsbuildinfo detection logic. |
| packages/nk-dev/lib/passthrough.js | Enhances nk type-check to drop stale incremental caches (or --cold) before running. |
| packages/nk-dev/lib/doctor.js | Expands drift detection to include Prettier config leftovers and CI script checks; adds helper for Prettier file discovery. |
| packages/nk-dev/lib/artifacts.js | Adds staleBuildInfo() to detect .tsbuildinfo older than dependency manifests. |
| packages/nk-dev/bin/nk.js | Extends type-check CLI to accept --cold and documents stale-cache dropping behavior. |
| docs/i18n-routing.md | Updates conceptual docs to emphasize fixed cluster shape and localeProxy usage. |
| .changeset/nk-dev-cold-type-check.md | Changeset for nk-dev type-check cold/stale behavior + expanded doctor coverage. |
| .changeset/locale-cluster-shape.md | Changeset documenting the deliberate breaking change and migration guidance for cluster-shape + middleware API. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| param: string; | ||
| cookieName: string; | ||
| countryLocales: Readonly<Record<string, L>>; | ||
| hrefLangTags?: Readonly<Partial<Record<string, string>>>; |
Comment on lines
195
to
199
| locales: readonly string[]; | ||
| defaultLocale: string; | ||
| strategy: "query" | "prefix"; | ||
| strategy: LocaleRouting<L>["strategy"]; | ||
| param: string; | ||
| prefixDefaultLocale: boolean; | ||
| hrefLangTags: Readonly<Partial<Record<string, string>>> | undefined; | ||
| currentLocale: string | undefined; |
Comment on lines
+1
to
+5
| 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"; |
Comment on lines
+98
to
+101
| if (stale.length > 0) { | ||
| console.error( | ||
| `nk type-check: ${cold ? "--cold" : "dependencies changed since the last run"} — removed ${stale.join(", ")}; checking from scratch.`, | ||
| ); |
| @@ -1,4 +1,4 @@ | |||
| import { existsSync, readFileSync, rmSync, writeFileSync } from "node:fs"; | |||
| import { existsSync, readFileSync, readdirSync, rmSync, writeFileSync } from "node:fs"; | |||
Jerome Leclanche (jleclanche)
added a commit
that referenced
this pull request
Aug 29, 2026
Addresses the two substantive review comments on #44: both widened L back to string. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Acts on fleet feedback from a site that adopted the prefix strategy and had to
fork the wiring rather than use the helpers. That's the failure this package
exists to prevent, so the fix is aimed at removing the fork, not at adding
options.
The bug
Under
"prefix",localeFromUrlreturneddefaultLocalefor a bare path. Thatis the URL signal, which outranks the cookie, so a visitor who chose French
snapped back to English on the first bare internal link they clicked. Every site
that starts with a cookie switcher has bare internal links.
The fix is not another option
prefixDefaultLocaleis removed and nothing replaces it. The cluster'sshape stops being configurable:
x-default.strategynow picks only the encoding. The two shapes this excludes are thetwo that go wrong: a bare path that IS the default locale (the bug above), and a
bare path that redirects on perceived language (what Google tells you not to
build, and it makes
x-defaultpoint at a URL that isn't language-neutral).Offering either is how a fleet drifts.
The rest
localeProxy(routing, request)— the whole middleware side in one call.Forwards headers, rewrites
/fr/about→/about, remembers the choice in thecookie, never redirects. Replaces
forwardUrlLocaleplus thestrip/rewrite/cookie/consolidate code every prefix site hand-wrote. Middleware
that does more passes
requestHeadersin and keeps editing the response.forwardRequestContextsets nk-seo'sx-pathnameand the locale headertogether, closing the seam where two conventions were wired separately.
isLocaleis a real type guard;resolve,localeFromUrlandcreateLocaleResolverreturnL. The first consumer ofthe old API had
(await resolve()) as Localein it, which was the tell.hrefLangTagsandcookieNameon routing. A site with regional tags nolonger builds a second config object — precisely the drift this package
exists to prevent.
routing.htmlLang(locale)gives<html lang>.nk-seo's
HreflangConfigdropsdefaultLocaleandprefixDefaultLocale;x-defaultis always the bare path, so neither is needed. The canonicalcomputation loses its special case entirely.
Tests
New coverage for the prefix cluster shape,
localeProxy(rewrite, querypreservation, cookie, and that a client-supplied locale header is stripped
rather than trusted), and type-level assertions that the union survives.
🤖 Generated with Claude Code