i18n: locale URL routing, shared between nk-i18n and nk-seo - #40
Merged
Conversation
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. When those drift the site annotates URLs that don't serve what they claim, and the languages leave the index silently. The precedence (URL, account, cookie, Accept-Language, country) is declared once and is not configurable: a shared ?hl=fr link must show the recipient French, or every localized link the site ships is a lie. Suppliers are lazy, so a ?hl= request costs no database round trip. /next wires it to middleware and server components. It never redirects. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
hreflangAlternates treated "default locale" and "bare path" as the same thing, which only holds for the prefix strategy. Under query every locale has its own ?hl= address and the bare path is the negotiating x-default, so ?hl=en canonicalized away to the bare path and deleted the default locale from its own cluster. hreflang is a set of promises about other URLs and nothing local can tell you whether they hold: a site can emit a flawless cluster while middleware redirects every URL in it away. /verify fetches them and fails on redirects, non-200s, canonicals pointing elsewhere, duplicate canonicals, non-reciprocal clusters, and an html lang contradicting its hreflang. A LocaleRouting is a valid HreflangConfig, asserted at compile time so the two packages cannot drift. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot started reviewing on behalf of
Jerome Leclanche (jleclanche)
August 23, 2026 20:04
View session
There was a problem hiding this comment.
Pull request overview
Introduces a shared, single-source locale URL routing model in @ingram-tech/nk-i18n and aligns @ingram-tech/nk-seo hreflang/canonical behavior with it, plus adds a runtime verifier to catch “advertise vs serve” drift (redirecting locale URLs, wrong canonicals, non-reciprocal clusters, etc.).
Changes:
- Add
defineLocaleRouting+ fixedLOCALE_PRECEDENCEand Next.js wiring (/next) to keep locale resolution and URL encoding consistent. - Fix
hreflangAlternatescanonical logic for the"query"strategy (default locale must canonicalize to its own?hl=URL, not the bare path). - Add
@ingram-tech/nk-seo/verifyto fetch and validate advertised hreflang clusters at runtime / in CI.
Reviewed changes
Copilot reviewed 18 out of 19 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/nk-seo/src/verify.ts | Adds runtime hreflang cluster verification and error aggregation utilities. |
| packages/nk-seo/src/verify.test.ts | Adds tests covering redirects, canonicals, reciprocity, and <html lang>. |
| packages/nk-seo/src/alternates.ts | Fixes canonical computation semantics (address-based) for "query" vs "prefix". |
| packages/nk-seo/src/alternates.test.ts | Adds regression tests for canonical behavior across strategies. |
| packages/nk-seo/README.md | Documents address-vs-language canonical rules and new /verify API. |
| packages/nk-seo/package.json | Exposes new ./verify entrypoint; adds dev dependency for integration test. |
| packages/nk-i18n/tsconfig.json | Adjusts module/moduleResolution for Next-related typing/build compatibility. |
| packages/nk-i18n/src/routing.ts | Implements locale URL routing + eager/lazy precedence-based resolvers. |
| packages/nk-i18n/src/routing.test.ts | Tests URL encoding/decoding and precedence ordering + laziness properties. |
| packages/nk-i18n/src/next.ts | Adds Next.js middleware/server-component wiring and hreflang config adapter. |
| packages/nk-i18n/src/index.ts | Exports new routing APIs from package root. |
| packages/nk-i18n/README.md | Documents locale URL routing and correct canonical/hreflang wiring. |
| packages/nk-i18n/package.json | Adds ./next export and declares Next as optional peer. |
| packages/nk-dev/guide.md | Updates package guide to mention locale URL routing in nk-i18n. |
| docs/README.md | Adds new i18n routing doc to docs index. |
| docs/i18n-routing.md | New design/usage document describing routing, canonicals, and verification. |
| bun.lock | Records workspace dependency changes (Next / nk-i18n dev deps). |
| .changeset/locale-url-routing.md | Changeset for nk-i18n minor release (locale URL routing). |
| .changeset/hreflang-canonical-query.md | Changeset for nk-seo minor release (canonical fix + verify). |
Suppressed comments (1)
packages/nk-seo/src/verify.ts:153
- Alternate link parsing has the same
rel === "alternate"issue as canonical parsing:relis token-based, and extra rel tokens will prevent alternates from being detected. Also, alternatehrefs can be relative; comparing them without normalizing to absolute can incorrectly flag a cluster as non-reciprocal.
const advertised = new Set(
tagsNamed(html, "link")
.filter(
(attrs) =>
attrs.rel?.toLowerCase() === "alternate" && attrs.hreflang,
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+132
to
+147
| const canonicals = tagsNamed(html, "link") | ||
| .filter((attrs) => attrs.rel?.toLowerCase() === "canonical") | ||
| .map((attrs) => attrs.href) | ||
| .filter((href): href is string => href !== undefined); | ||
| if (canonicals.length === 0) { | ||
| add("no <link rel=canonical>"); | ||
| } else if (canonicals.length > 1) { | ||
| add( | ||
| `${canonicals.length} <link rel=canonical> tags; Google ignores all of them`, | ||
| ); | ||
| } else if (canonicals[0] !== url) { | ||
| add( | ||
| `canonical points at ${canonicals[0]}, not itself; ` + | ||
| "a variant that canonicalizes elsewhere is discarded", | ||
| ); | ||
| } |
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.
Why
A multilingual site has two halves that must agree: the code that serves a
language, and the code that advertises it to search engines. Nothing made
them agree, so they drifted. The live failure on financica.app: the layout
emitted
hreflangpointing at?hl=fr, and middleware 307'd every?hl=URLback to the bare path. The site told Google the French page lived at an address
that refused to serve French, so French and Dutch were absent from the index
entirely. Search Console showed it only as 137 ordinary "Page with redirect".
nk-i18n
defineLocaleRouting— one definition of how a locale is encoded in a URL,shaped so it can be handed straight to nk-seo's
hreflangAlternates.LOCALE_PRECEDENCE— the order, declared once and not configurable: URL,account, cookie,
Accept-Language, country, default. The URL beating theaccount setting is load-bearing: a shared
?hl=frlink must show therecipient French, or every localized link the site ships is a lie and so is
the annotation pointing at it. Eager and lazy resolvers walk the same list;
a test asserts they agree on every prefix of the chain.
/next—forwardUrlLocale(forwards, never redirects),getUrlLocale,createLocaleResolver,hreflangConfigFor.countryLocalesis a map, so omitting Belgium is how you express "geographycannot decide between French and Dutch here" rather than a buried special case.
nk-seo
hreflangAlternatestreated "default locale" and "barepath" as the same thing, which only holds for the prefix strategy. Under
"query"every locale has its own?hl=address and the bare path is thenegotiating
x-default, so?hl=encanonicalized away to the bare path anddeleted the default locale from its own cluster. Prefix behaviour unchanged.
/verify. hreflang is a set of promises about other URLs and nothinglocal can tell you whether they hold.
assertHreflangClusterfetches everyadvertised URL and fails on a redirect, a non-200, a canonical pointing
elsewhere, a missing or duplicated canonical, a non-reciprocal cluster, or an
<html lang>contradicting itshreflang.The integration is enforced
verify.test.tsasserts at compile time that aLocaleRoutingis a validHreflangConfig. Verified non-vacuous: adding a third value to nk-i18n'sstrategy union fails the build in nk-seo with
Type 'LocaleRouting' is not assignable to type 'HreflangConfig'.Rationale and the do-nots are in
docs/i18n-routing.md.🤖 Generated with Claude Code