build(deps): replace ex_cldr with localize and localize_web - #5630
Conversation
✅ Deploy Preview for teslamate ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Thanks Matthias — this is a really well-prepared migration, and getting rid of the compile-time locale download, the Nix cldr pin and the CI locale cache is exactly the win the PR description promises. I ran nix run .#update-nix-hashes and pushed the new mixFodDeps hash to the branch, so that follow-up is done.
I did a deep review with 8 parallel Agents (including reading the localize/localize_web sources and verifying the locale resolution empirically with mix run). One real regression and a few smaller things:
Blocker
Valid-but-unsupported languages resolve to Catalan instead of falling back to English. Localize.validate_locale/1 restricts to supported_locales via LanguageTag.best_match/3 with the default distance threshold, which always returns a result — the first supported locale as a last resort. Verified with the app config loaded:
Accept-Language: ru-RU,ru;q=0.9,en;q=0.8→ locale:ca, Gettext"ca"(theen;q=0.8never wins, because theru-RUtag already "validates" to:ca)?locale=pt,?locale=pl→:caas wellLocalize.Plug.PutSessionthen persists"ca", so the user is stuck in Catalan across requests
On main, Cldr.Plug.AcceptLanguage declined the no-match (hence the old no_match_log_level: :debug) and we fell through to English. The new test only covers ?locale=xx, which fails parsing (invalid ISO subtag) — a different code path — so the suite stays green. Could you add tests with a real unsupported language (ru, pt) and restore the English fallback (strict matching / reordering alone only fixes the last-resort case)?
Should fix
- Locale negotiation is now recomputed on every request and LiveView mount, uncached. The old chain read a precomputed session value (
Process.put-cheap); nowgettext_locale_id/2runs a full 19-candidateLanguageTag.best_match(parse + canonicalize + likely-subtags per candidate) in the plug on every request, plus twice more per LiveView page load (dead render + connected mount). Measured ~70 µs per call on Apple Silicon — on the Raspberry Pi 3 B+ boxes a large share of our installs run on, that's realistically ~2–3 ms per negotiation, so ~6–10 ms per page load warm, and a cold Accept-Language tag (~2.5 ms even on the Mac) can hit 50–100 ms there. The fix is cheap and restores the old shape: store the resolved Gettext locale in the session (like the deletedPutSessiondid) and makeon_mounta plainGettext.put_locale/2again. <html lang>is no longer valid BCP-47 for Chinese:Gettext.get_locale/1returns the POSIX name, so we renderlang="zh_Hans". Screen readers,:lang()CSS and translate detection won't match it — andlocale_test.exscurrently asserts the invalid value, locking it in. The plug already has the proper tag:Localize.Plug.PutLocale.get_locale(@conn)renders as"zh-Hans".- Two locale lists that can drift: the settings dropdown now derives from
Gettext.known_locales/1while the plug gate is the hand-copied list inconfig.exs. If Weblate addspriv/gettext/pland nobody edits the config, "Polish" shows up in the dropdown and selecting it lands on… Catalan (same mechanism as above), with no compile-time or test signal. A test asserting the two sets match would close this cheaply. (Your comment aboutzh_Hanscollapsing to:zhis accurate, I checked — so the static list itself is fine, it just needs the guard.)
Nits
router.ex:default: :enduplicatesconfig :localize, default_locale: :enand opts back into eagerPlug.init-time validation that the library's:__localize_default__sentinel deliberately avoids;param: "locale"andas: :stringare the library defaults. All three lines can go.init_assigns.ex: the{:error, _}branch hardcodes"en"(Gettext already falls back to its configured default if you just do nothing) and never setsLocalize.put_locale/1, so the two locale registries disagree on that path. It is reachable once per pre-upgrade session on a LiveView reconnect — harmless, but worth simplifying.car_live/index.html.heexstill passes"locale" => @localeinto the nestedlive_render— dead since the controller-side removal, nothing reads it anymore..gitignorestill has/priv/cldr/; nothing writes that directory now (and upgraded checkouts keep a stale multi-MBpriv/cldr/locales).CHANGELOG.md: the entry is missing your- @swiffercredit, and the one-time UI-language reset (session key change) deserves a short note block under[unreleased]like we did for 4.1.x, so we don't collect "my language reset itself" issues.
None of this diminishes the migration itself — the approach and the build-plumbing cleanup are exactly right. 👍
🤖 Review drafted with Claude Code (Fable 5 high) — sponsored by Claude for Open Source
b66c870 to
23a902a
Compare
|
I'll implement the fixes |
…ocale (#5630) Localize's default matching never fails for a valid language tag and best-matches unrelated languages onto the first supported locale, so Accept-Language: ru came back Catalan and was persisted in the session. Replace the built-in PutLocale sources with strict ones (distance 79 keeps CLDR's related-language fallbacks, rejects the unrelated bucket) that fall through per Accept-Language entry to the configured default. Store the resolved Gettext locale under the pre-migration "gettext_locale" session key: existing sessions keep their language across the upgrade, the LiveView on_mount hook is a plain Gettext.put_locale/2 again, and the per-request 19-candidate tag matching drops to a map lookup on the warm path. Also: emit BCP 47 <html lang> (zh-Hans, not zh_Hans), drop the redundant PutLocale options and Localize.Plug.PutSession, remove the dead "locale" live_render session key and the stale /priv/cldr/ gitignore entry, and add tests for unsupported locales, secondary Accept-Language preferences, legacy sessions, and config/Gettext sync. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Resolved differently: the implementation now stores the Gettext locale under the pre-migration |
JakobLichterfeld
left a comment
There was a problem hiding this comment.
Follow-up on my review above: I pushed 959d92c addressing the findings — strict source functions for the negotiation (restores the English fallback, with tests for ru, pt and the ru,de;q=0.9 fallthrough), session-cached Gettext locale so on_mount is a plain put_locale/2 again, proper BCP-47 <html lang> (zh-Hans/zh-Hant covered), the config↔priv/gettext sync test, and the nits. The session key stays "gettext_locale", so pre-migration sessions keep their language and no changelog note is needed.
I then re-ran the deep review in a new session (8 parallel agents, findings verified by executing the compiled plug) on the fixed branch. Two edge cases in my own fix, worth closing before merge:
- Non-binary
?localeparam crashes with a 500.strict_match/1only hasnilandis_binaryclauses, soGET /?locale[]=de(Plug decodes it as["de"]) raisesFunctionClauseErroron every:browserroute — trivially reachable by scanners. The oldCldr.validate_locale/2returned{:error, _}and fell through. Fix:defp strict_match(_other), do: nil. ?locale=undrenders the UI in Catalan.best_match/3short-circuitsundto the first supported locale at score 80 — above the strict threshold of 79 — but the{:ok, cldr_id, _score}clause discards the score and accepts it, then persistsgettext_locale=ca. The one gap in the strict matching; needs a score check (or rejectingundup front) plus a test.
One design point to settle deliberately: the release ships no CLDR locale data except en/und, allow_runtime_locale_download is false, and otp_app: :teslamate points Localize's cache dir at a path nothing in the Docker/Nix build populates. Negotiation needs no per-locale data, so everything works — but the first Localize.Number/date formatting call anyone adds will raise LocaleNotFoundInCacheError in production, for non-English users only. If formatting stays a non-goal, let's document that and re-add a .gitignore entry for /priv/localize/ (a mix localize.download_locales run currently leaves committable blobs in priv/).
Smaller things, fine as follow-ups:
put_session/3runs unconditionally, so every response re-signs and re-sends the session cookie; compare-then-write (like Localize's ownPutSession) avoids that.on_mount's catch-all silently falls back to English on the connected mount when the session is missing (reverse proxies stripping cookies on the websocket upgrade — we've had support threads on that), and a pre-migration session carrying"gettext_locale" => nil(the oldPutSessionwrotegettext_locale_nameverbatim) matches the pattern andGettext.put_locale/2raises onnil. Awhen is_binary(locale)guard covers both.@strict_distancederives fromLocalize.LanguageTag.default_distance() - 1, a@doc falseinternal that tracks CLDR data — a dep bump can silently move the matching window. I'd pin the literal and add anAccept-Language: nntest assertingnb(verified working today), so the moduledoc's promise is enforced.session_key/0andgettext_locales/0have no callers —init_assigns.exandlocale_test.exshardcode the literal / re-derive the mapping. Either wire them through so the sync test exercises the plug's actual maps, or drop them.
Also verified clean: zh-CN → zh-Hans and zh-Hant-TW → zh-Hant via Accept-Language, no dangling Cldr/SKIP_LOCALE_DOWNLOAD references, Nix/CI/docs consistent.
🤖 Review drafted with Claude Code (Fable 5 high) — sponsored by Claude for Open Source
There was a problem hiding this comment.
Re-reviewed the six follow-up commits (same setup: parallel agents + empirical verification against the compiled plug, full locale/settings suite green against Postgres). All review findings are addressed: non-binary ?locale params fall through instead of 500ing, und is rejected by the score guard, legacy nil session locales no longer crash mounts, nn → nb is pinned by a test, the session key and locale map are wired to their consumers, and the formatting non-goal is documented with the cache dir ignored.
What's left is minor:
- The
persist_localecompare-then-write doesn't save anything yet:fetch_settingsright after it does an unconditionalput_session(:settings, …), so the session is always dirty and every response still re-signs the cookie. The same one-line compare-then-write there unlocks the win (and stops re-serializing the whole settings struct into the cookie per request). - The pinned
@strict_distance 79guards against localize raising its default distance, but inverts if an upgrade ever lowers it to ≤ 79 (the dep's strict-error path is guarded bydistance < default, and the fallback echoes the threshold as the score, so the guard passes). Ourrutest would go red in CI, so it's caught — but a compile-time@strict_distance < default_distance()assertion fails at the file that owns the invariant. The comment also overstates: CLDR scores some genuinely related pairs (sgs↔lt) at exactly 80, so rejecting the 80 bucket is a trade-off, not a free lunch. on_mountaccepts any binary session locale unvalidated, unlike the dead-render path — a stale value from an old release (e.g. a removed locale) renders English while the settings dropdown matches no option. A membership check againstgettext_locales()makes both paths agree.- The nil-guard test should use
session_key()(renaming the key would silently make it test the wrong branch) and also assert the positive case.
🤖 Review drafted with Claude Code (Fable 5 high) — sponsored by Claude for Open Source
There was a problem hiding this comment.
Re-reviewed the follow-up commits (same setup: parallel review agents + empirical verification against the compiled plug, full web-layer suite green against Postgres — 119 tests, 0 failures). All findings from my review are addressed: non-binary ?locale params fall through instead of 500ing, und is rejected by the score guard, legacy nil session locales no longer crash LiveView mounts, nn → nb is pinned by a test, the session key and locale map are wired to their consumers, and the formatting non-goal is documented with the locale cache dir git-ignored.
The re-review surfaced a few smaller things, closed in 1edf81e:
- The
persist_localecompare-then-write saved nothing yet:fetch_settingsright after it did an unconditionalput_session(:settings, …), keeping the session permanently dirty — every response still re-signed and re-sent the cookie.fetch_settingsnow does the same compare-then-write, which also stops re-serializing the settings struct into the cookie on every request. - Pinning
@strict_distance 79guards against localize raising its default matching distance, but would invert if an upgrade ever lowered it to ≤ 79 (the dep's strict-error path is guarded bydistance < default, and the fallback echoes the threshold as the score). A compile-time assertion now fails the build at the file that owns the invariant. The comment also acknowledges that CLDR scores a few genuine relatives (sgs↔lt) at exactly 80, so rejecting that bucket is a deliberate trade-off. on_mountaccepted any binary session locale unvalidated, unlike the dead-render path — a stale value from an old release (e.g. a removed locale) would render English while the settings dropdown matched no option. The guard is now a membership check against the plug's locale map, so both paths agree; covered by new positive and stale-locale tests.- The nil-guard test hardcoded the session key and only asserted the fallback; it now goes through
session_key()and pins the positive branch too.
From my side this is ready to merge after smoke test.
🤖 Review drafted with Claude Code (Fable 5 high) — sponsored by Claude for Open Source
|
Thanks a lot for taking this over, Jakob — work kept me fully occupied and I didn't get back to the review in time. Really appreciate the follow-ups: the strict matching so we don't drop people into Catalan, keeping the old Looks good from my side; happy to leave the rest with you for the smoke test / merge. |
|
You're very welcome. I'll test these images and then do the release. |
Migrate off ex_cldr before support ends in 2027 and drop compile-time locale downloads.
…ocale (#5630) Localize's default matching never fails for a valid language tag and best-matches unrelated languages onto the first supported locale, so Accept-Language: ru came back Catalan and was persisted in the session. Replace the built-in PutLocale sources with strict ones (distance 79 keeps CLDR's related-language fallbacks, rejects the unrelated bucket) that fall through per Accept-Language entry to the configured default. Store the resolved Gettext locale under the pre-migration "gettext_locale" session key: existing sessions keep their language across the upgrade, the LiveView on_mount hook is a plain Gettext.put_locale/2 again, and the per-request 19-candidate tag matching drops to a map lookup on the warm path. Also: emit BCP 47 <html lang> (zh-Hans, not zh_Hans), drop the redundant PutLocale options and Localize.Plug.PutSession, remove the dead "locale" live_render session key and the stale /priv/cldr/ gitignore entry, and add tests for unsupported locales, secondary Accept-Language preferences, legacy sessions, and config/Gettext sync. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
strict_match/1 only had nil and is_binary clauses, so a request like GET /?locale[]=de — which Plug decodes to a list — raised a FunctionClauseError (HTTP 500) on every browser route. Ignore any non-binary value and fall through to the next locale source. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
LanguageTag.best_match/3 short-circuits the desired locale "und" past the distance threshold and returns the first supported locale at the default distance (80), so ?locale=und rendered the UI in Catalan and persisted it in the session. Accept a match only when its score is within the strict distance. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The pre-localize PutSession plug wrote the CLDR tag's gettext_locale_name into the session verbatim, which could be nil. Such a session matched the on_mount pattern and Gettext.put_locale/2 raises on nil, crashing every LiveView mount for that browser until the cookie is replaced. Only accept binary locale values; anything else falls back to the Gettext default. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@strict_distance derived from Localize.LanguageTag.default_distance/0, a @doc false internal tracking CLDR data, so a dependency bump could silently move the matching window. Pin the literal (79 — one below the unrelated-language bucket) and assert both sides of the boundary: Accept-Language nn falls back to nb, ru falls through to the default. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… consumers session_key/0 and gettext_locales/0 had no callers: init_assigns hardcoded the session key and the sync test re-derived the mapping from config, so neither exercised the plug's actual values. Wire both through. Also write the session only when the locale changed, instead of re-signing the cookie on every response. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ache dir The release ships no per-locale CLDR data (negotiation needs none) and runtime download is off, so any future Localize formatting call would raise LocaleNotFoundInCacheError for non-English locales. State that next to the config, and ignore /priv/localize/ so a manual `mix localize.download_locales` cannot leave committable blobs in priv. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- write the settings session only on change: the unconditional put_session in fetch_settings kept the session dirty on every response, re-signing the cookie and defeating the locale plug's compare-then-write - assert at compile time that @strict_distance stays below Localize's default matching distance, where strictness would silently invert, and state the sgs/lt trade-off of the 80-bucket cut in the comment - validate session locales on LiveView mounts against the supported set, keeping the mount path consistent with the dead render and covering legacy values (nil, removed locales) - tests: use session_key() instead of the literal, assert the positive mount path and the stale-locale fallback Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1edf81e to
aec388f
Compare
Why
ex_cldr officially recommends migrating to localize (migration guide):
ex_cldr_*only until 31 December 2027, then no planned supportTeslaMate already paid for the old architecture.
ex_cldrembeds locale data at compile time, so every prod build (and often everymix compile) tried to pull locales from GitHub. That caused recurring headaches:elixir-cldr/cldrtree (SKIP_LOCALE_DOWNLOAD+LOCALES) just to keep Darwin/CI builds reproducible (fix(nix): skip duplicate cldr download to fix build on darwin and improve reproducibility #4763)priv/cldr/localesex_cldr(build(deps): bump ex_cldr from 2.46.0 to 2.47.1 #5166)The application-code change is small. The build plumbing is the real win.
What TeslaMate actually used
We do not use CLDR for numbers, dates, units, lists, currencies, or routes. Dates go through Timex. Translations go through Gettext. Address language is a separate OSM Nominatim setting and is untouched.
ex_cldr+ex_cldr_plugswere only used for:ca da de en es fi fr hu it ja ko nb nl sv th tr uk zh_Hans zh_Hant)Cldr.known_locale_names/0)So we need both successor packages:
localize1.2.0localize_web1.1.0PutLocale,PutSession,put_locale_from_session/2(replacesex_cldr_plugs)We do not take calendrical, localize_sql, localized routes, HTML helpers, MF2 interpolation, or the ICU NIF. Runtime locale download stays off.
What changed
ex_cldr/ex_cldr_plugs/cldr_utils, addlocalize+localize_webTeslaMateWeb.Cldrand the customPutSessionplug (no compile-time backend)Localize.Plug.PutLocalewithfrom: [:query, :session, :accept_language]— query must stay first so the settings UI (?locale=de) beats a stored session. Do not use localize_web’s default order.Localize.Plug.put_locale_from_session/2and still assign the Gettext id ("en","zh_Hans"), not a BCP 47 tagGettext.known_locales/1(no more rejecting parent:zh/ hyphen rewriting)supported_localesis a static CLDR ID list including:"zh-Hans"and:"zh-Hant". Expanding Gettext names like"zh_Hans"would collapse both Chinese variants to:zhhtml langnow usesGettext.get_locale/1(the old:localesession key was never written)SKIP_LOCALE_DOWNLOAD/LOCALES, Nixelixir-cldr/cldrfetch, devenvLOCALES, CIpriv/cldr/localescacheBehaviour preserved
Accept-Language, elseen?locale=is present"localize_locale") and fall back to Accept-LanguageTests
New
test/teslamate_web/locale_test.exscovers defaulten, Accept-Languagede-DE, query beating the header, unknown?locale=xx, distinctzh_Hans/zh_Hant, and session persistence. Existing settings UI-language redirect test is kept.Follow-up
mixFodDepsinnix/flake-modules/package.nixis stale. Runnix run .#update-nix-hashesbefore merging (not done here; Nix was not available).Out of scope
Replacing Timex, localizing routes,
Localize.HTMLhelpers, MF2 Gettext interpolation, OSM address-language list.