fix(cli): adopt every locale catalog the scaffold's server sends - #322
Merged
Merged
Conversation
The scaffold gated `updateI18n` on the locale *changing*, so it dropped the
catalog that arrives when the audience changes instead. Signing in swaps the
anonymous snapshot for one including admin-only modules at the same locale, so
the update never fired and every admin screen rendered raw keys
("dashboard.home.title") until a hard refresh.
A non-null `messages` payload is itself the server's signal that the client
needs it — the backend already sends `null` when the catalog the client holds
is still good. `updateI18n` calls `addResourceBundle(..., deep, overwrite)`,
so applying it on every visit is additive and idempotent, and `activeLocale`
becomes dead.
The host fixed this for itself in #261 (`host/client_app/i18n.ts`, whose
comment describes this exact scenario); the template was never brought across,
so every app scaffolded since has shipped the bug. Reproduced on a real
deployment: raw keys on the post-login redirect, correct after a hard reload,
2/2 in fresh browser contexts.
The regression test asserts on the generated `app.tsx` and fails against the
previous template.
Closes #321
Claude-Session: https://claude.ai/code/session_01FGEM3MAEJ9iQ2LJLM3FCsT
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Deploying simple-module-python with
|
| Latest commit: |
234c845
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://999e71de.simple-module-python.pages.dev |
| Branch Preview URL: | https://fix-scaffold-i18n-locale-gat.simple-module-python.pages.dev |
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
1 participant
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.
Closes #321.
The bug
The
sm newhost template gatedupdateI18non the locale changing:But the catalog the server sends varies by audience, not only by locale. Signing in swaps the anonymous snapshot for one including admin-only modules at the same locale (
en→en), so the update never fired and the admin catalog was thrown away. Every admin screen then rendered raw keys until a hard refresh.The framework's own host already fixes this in
host/client_app/i18n.ts— its comment describes this exact scenario,dashboard.home.titleexample included. That landed in #261; the template was last touched in #276 for an unrelated change and never got it. Every app scaffolded since has shipped the bug.The fix
Adopt whatever catalog the server sends. A non-null
messagespayload is the signal that the client needs it — the backend already sendsnullwhen the cached catalog is still good (#248).updateI18ncallsaddResourceBundle(..., deep, overwrite), so applying it on every visit is additive and idempotent;activeLocalebecomes dead and is removed.Verification
Reproduced on a real deployment before fixing — five raw keys (
dashboard.home.title,.description,.stats.total_users,.stats.active_users,.stats.modules) on the post-login redirect, all correct after a hard reload, 2/2 in fresh browser contexts. The tell was that every string on that page which renders correctly is a hardcoded literal inHome.tsx, while every string routed throught()renders raw.app.tsx; verified it fails against the previous template and passes with this change.framework/cli/tests/: 289 passed, 2 skipped.ruff format --check+ruff checkclean;scripts/check_file_size.pyOK.Note
This is the second time the scaffold has shipped broken
t()(cf. #83). It may be worth having the template import the samei18n.tshelpers the host uses rather than restating the wiring, so the two cannot drift again — left out of this PR to keep it to the fix.https://claude.ai/code/session_01FGEM3MAEJ9iQ2LJLM3FCsT