Intl.Locale: apply CLDR alias mappings during canonicalization - #36731
Intl.Locale: apply CLDR alias mappings during canonicalization#36731robobun wants to merge 6 commits into
Conversation
JSC's canonicalizeLocaleIDWithoutNullTerminator calls ICU's uloc_canonicalize,
which is documented as "does NOT map aliased names in any way". As a result
Intl.Locale and Intl.getCanonicalLocales skip CLDR language/region alias
replacement: new Intl.Locale("fre").baseName stayed "fre" instead of "fr",
"en-uk" stayed "en-UK" instead of "en-GB", "sh" stayed "sh" instead of
"sr-Latn", and new Intl.Locale("und-Armn-SU", { language: "ru" }) produced
"ru-Armn-SU" instead of "ru-Armn-AM".
Safari avoids this via Apple's ualoc_canonicalForm SPI (gated behind
USE(APPLE_INTERNAL_SDK)). The CLDR-aware path in open-source ICU is
icu::Locale::createCanonical, which runs ICU's AliasReplacer since ICU 68, but
WebKit compiles with U_SHOW_CPLUSPLUS_API=0 so the ICU C++ API is unreachable
from JSC.
WebKit now forward-declares a weak Bun__canonicalizeLocaleID with the
uloc_canonicalize signature and calls it when present (oven-sh/WebKit#381).
IntlCanonicalize.cpp provides the implementation: on macOS it forwards to
ualoc_canonicalForm (exported from libicucore on every supported macOS
version), and elsewhere it calls icu::Locale::createCanonical. The file is
compiled standalone and without the PCH so wtf/Platform.h never hides the ICU
C++ API before <unicode/locid.h> is seen.
Fixes #14713.
|
Warning Review limit reached
Next review available in: 12 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (6)
Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
No bugs found, but this needs a human look: it's a coordinated cross-repo change (pins WEBKIT_VERSION to an autobuild-preview-pr-381-* tag pending oven-sh/WebKit#381), and the macOS path links against Apple's undocumented ualoc_canonicalForm SPI from libicucore.
What was reviewed:
Bun__canonicalizeLocaleIDbuffer-overflow contract matchesuloc_canonicalize(U_BUFFER_OVERFLOW_ERROR/U_STRING_NOT_TERMINATED_WARNING+ returned length) so JSC'scallBufferProducingFunctionretry loop works.noUnify/noPchSourcesentries follow existing patterns; the file has no PCH dependency so dropping it is safe.- The changed
Intl.getCanonicalLocales(["sh","tl","no"])assertion — new expectations match V8/Safari and ECMA-402. - The preview-tag
WEBKIT_VERSIONwas flagged and confirmed intentional; a maintainer will need to swap it for the merged hash.
Extended reasoning...
Overview
This PR fixes #14713 (Intl.Locale not applying CLDR alias mappings like fre→fr, en-UK→en-GB). It works by having the WebKit fork call a new weak symbol Bun__canonicalizeLocaleID from canonicalizeLocaleIDWithoutNullTerminator, and providing that symbol in a new standalone TU src/jsc/bindings/IntlCanonicalize.cpp. On macOS it forwards to Apple's ualoc_canonicalForm from libicucore; elsewhere it calls icu::Locale::createCanonical from the statically-linked ICU. Build-system changes add the file to noUnify and noPchSources so wtf/Platform.h's U_SHOW_CPLUSPLUS_API=0 doesn't hide the ICU C++ headers. WEBKIT_VERSION is bumped to a preview tag for the paired WebKit PR. Tests add a new Intl.Locale canonicalization describe block and update one existing assertion.
Security risks
None identified. The input is a locale ID string already validated by JSC's isStructurallyValidLanguageTag; output goes into a caller-owned buffer via the standard ICU buffer-overflow protocol. No user-controlled sizes drive allocation here.
Level of scrutiny
High — this is a cross-repo coordinated change touching JSC integration, per-platform ICU ABI assumptions, and the build system. Specifically:
WEBKIT_VERSIONis pinned toautobuild-preview-pr-381-edc9ed33, an ephemeral preview tag. A maintainer needs to land the WebKit PR and swap this for the merged commit hash (standard workflow, but a human should sequence it).- The macOS path declares and calls
ualoc_canonicalForm, an Apple-internal ICU SPI. The PR body documents its presence in the 12.3–26.4 SDK.tbdfiles, but relying on an undocumented symbol is a maintainer-level call. - The non-Darwin path pulls in the ICU C++ API, which WebKit deliberately hides — the noUnify/noPch workaround is sound but unusual enough to want maintainer eyes.
Other factors
The C++ implementation looks correct: it mirrors uloc_canonicalize's signature and buffer contract exactly (returns required length, sets U_BUFFER_OVERFLOW_ERROR when length > nameCapacity, U_STRING_NOT_TERMINATED_WARNING when equal), which is what JSC's callBufferProducingFunction growth loop expects. The existing test that asserted sh/tl stay as-is was updated to the ECMA-402-correct values, with the comment updated to explain why. New tests cover language aliases, region aliases, before/after option-merge canonicalization, extension-keyword preservation, and getCanonicalLocales — a solid variant matrix. Verifier agents examined and dismissed the preview-tag pin as intentional. Given the cross-repo coordination and platform-SPI reliance, deferring to a human is the right call.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
Build #87506 surfaced the real problem: CI's split link archives bun's C++ objects into Build #87541 then caught a macOS 13/14 edge: Build #87654: 194/196 passed, |
|
Updated 1:33 AM PT - Aug 2nd, 2026
❌ @robobun, your commit 5ba61c5 has 2 failures in
🧪 To try this PR locally: bunx bun-pr 36731That installs a local version of the PR into your bun-36731 --bun |
CI's split build archives bun's C++ objects into libbun.a before linking. libJavaScriptCore.a only references Bun__canonicalizeLocaleID weakly, and a weak reference never pulls a member from an archive, so IntlCanonicalize.o was dropped and JSC fell back to uloc_canonicalize. Local full builds pass the .o directly to the linker and were unaffected. Add -Wl,-u / /include: so the linker treats the symbol as a strong undefined and pulls the defining member.
uloc_forLanguageTag("und") yields the empty ICU locale ID. On macOS 13/14
libicucore, both ualoc_canonicalForm("") and uloc_canonicalize("") return
the process default locale (e.g. en_US_POSIX) instead of "", so
new Intl.Locale("und", { language: "sh" }) produced
"sr-Latn-US-u-va-posix" instead of "sr-Latn". Apple fixed this in the
macOS 26 libicucore; short-circuit empty input on Darwin so older releases
match.
Verified on macOS 13 that the default-locale substitution in
ualoc_canonicalForm only fires for the exact empty string:
uloc_forLanguageTag("und-u-ca-gregory") keeps the "und" prefix and
uloc_forLanguageTag("und-x-private") yields "@x=private", neither of which
triggers the quirk. Add assertions so the macOS lane proves it.
2 participants
Fixes #14713.
Note
WEBKIT_VERSIONcurrently points atautobuild-preview-pr-381-edc9ed33, a preview build of oven-sh/WebKit#381. That PR must land first, and this line must be bumped to the resulting merge-commit sha before this Bun PR is merged.Reproduction
Cause
JSC's
canonicalizeLocaleIDWithoutNullTerminatorcalls ICU'suloc_canonicalize, which is documented as "does NOT map aliased names in any way" (ICU-21506). Safari avoids this via Apple'sualoc_canonicalFormSPI, gated behindUSE(APPLE_INTERNAL_SDK)which Bun does not set. The CLDR-aware path in open-source ICU isicu::Locale::createCanonical(runs ICU'sAliasReplacersince ICU 68), but WebKit compiles withU_SHOW_CPLUSPLUS_API=0(wtf/Platform.h), so the ICU C++ API is unreachable from JSC, and undoing that define from inside a unified-source TU breaks on ICU's header include guards.Fix
WebKit now forward-declares a weak
Bun__canonicalizeLocaleIDwith theuloc_canonicalizesignature and calls it fromcanonicalizeLocaleIDWithoutNullTerminatorwhen present (oven-sh/WebKit#381; thejscshell links without the symbol and keeps the old behavior).src/jsc/bindings/IntlCanonicalize.cppprovides the implementation:ualoc_canonicalForm. The symbol is exported fromlibicucoreon every macOS version Bun supports (present in the MacOSX 12.3 / 13.3 / 15.4 / 26.4 SDK.tbdfiles; Bun's minimum is 13.0), and as a plain C function it avoids the C++ ABI hazard of callingicu::Localeagainst the system ICU.icu::Locale::createCanonical. ICU is statically linked on these targets, so the C++ ABI is fixed at build time.The file is compiled standalone (
noUnify) and without the PCH (noPchSources) sowtf/Platform.hnever setsU_SHOW_CPLUSPLUS_API=0before<unicode/locid.h>is seen.Verification
bun bd test test/js/web/intl/intl.test.tspasses all 37 tests; the seven newIntl.Locale canonicalizationtests plus the updatedIntl.getCanonicalLocalesassertion fail on a build withoutIntlCanonicalize.cpp.no test proof · iteration 8 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/web/intl/intl.test.ts