Repository navigation
add i18n support - #6796
add i18n support#6796benedikt-bartscher wants to merge 51 commits into
Conversation
Merging this PR will not alter performance1 warning
Comparing 
|
 [Medium impact] The reviewed changes appear safe to merge, and the switcher now updates both browser and server translations. SummaryAdds internationalization through
Reviews (43) · Last reviewed commit: "Tell the server when the cookie-mode swi..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6b9a5f5342
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| "reflex-base >= 0.9.7.dev0", | ||
| "reflex >= 0.9.7.dev0", |
There was a problem hiding this comment.
With the new dispatch-release option, selecting reflex-i18n will create a release and trigger .github/workflows/publish.yml, whose publish job runs scripts/check_min_deps.py --check-dev-pins "$PACKAGE" and explicitly rejects published dependencies containing *.dev pins. Because these two requirements are part of reflex-i18n's published metadata, the publish workflow for any reflex-i18n release will fail at that gate until they are pinned to released versions.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
I'll leave this open for the maintainers
There was a problem hiding this comment.
Still for the maintainers, but with the current picture after merging main (c00c5ff): reflex itself carries the same kind of pin (reflex-base >= 0.9.12.dev0), so a dev pin in-tree is the normal state and the publish gate is what forces the bump. reflex-i18n's constraint is genuinely unreleased — it needs register_event_scope_provider, register_implicit_dependency and ExpandRoutesContext, all added by this PR — so no published floor would be truthful, and I'd rather not invent one. It uses none of the API #7268 raised floors for. check_min_deps.py reflex-i18n passes; --check-dev-pins reports the pin as expected.
|
@masenf could you please take a brief look at this from an architectural design and DX perspective? it's totally okay if you have smth completly different in mind |
masenf
left a comment
masenf
left a comment
There was a problem hiding this comment.
@benedikt-bartscher thanks for adding this and sorry for the review back-up. i did an initial look and i think the added extension points make sense and the API feels nice.
one potential issue i see affects SEO performance of the SSG static versions of the site -- since the translation depends on state, other languages might not be indexable by crawlers.
e68fb65 to
3e540c0
Compare
Resolve conflicts in tests/units/istate/test_proxy.py, tests/units/test_state.py and uv.lock. Mark I18nState.is_user_defined with _override_base_method for main's reserved state name validation, and regenerate pyi_hashes.json.
… package Adding reflex-i18n pushes the package count past the workflow_dispatch input limit, so reflex-release sync renders the free-text package field instead of per-package checkboxes.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
5 issues found across 56 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/reflex-i18n/src/reflex_i18n/nav.py">
<violation number="1" location="packages/reflex-i18n/src/reflex_i18n/nav.py:57">
P2: When `I18nPlugin` uses its default cookie-based mode, this still renders links such as `/de/current-page` even though URL-prefixed routes are not generated, so every switcher link can 404. Require URL routing before creating this component, or make the switcher use the cookie-based `setLocale` behavior when routing is disabled.</violation>
</file>
<file name="packages/reflex-i18n/src/reflex_i18n/runtime.py">
<violation number="1" location="packages/reflex-i18n/src/reflex_i18n/runtime.py:190">
P2: Configured locales containing underscores never negotiate from normal BCP 47 `Accept-Language` tags. Normalize supported locales from `_` to `-` when building both lookup maps, as is already done for the request tag.</violation>
</file>
<file name="packages/reflex-i18n/src/reflex_i18n/_web/i18n.js">
<violation number="1" location="packages/reflex-i18n/src/reflex_i18n/_web/i18n.js:122">
P2: Localized prerendered pages have no route-specific `<html lang>` or `dir` until hydration, reducing accessibility and SEO correctness for crawlers and users. Emit these attributes from the server-rendered document or another SSR-aware route metadata mechanism.</violation>
<violation number="2" location="packages/reflex-i18n/src/reflex_i18n/_web/i18n.js:163">
P2: When an underscore-separated RTL locale is configured, this check marks the document as left-to-right. Split on both supported locale separators, and apply the same normalization in `LocaleRoute`.</violation>
<violation number="3" location="packages/reflex-i18n/src/reflex_i18n/_web/i18n.js:220">
P1: When a configured locale uses an underscore, `getFormatter` passes the tag directly to `Intl`, which throws before formatting any value. Normalize underscores to hyphens before constructing the formatter.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| const key = locale + " " + JSON.stringify(options ?? {}); | ||
| let formatter = cache.get(key); | ||
| if (formatter === undefined) { | ||
| formatter = new Ctor(locale, options); |
There was a problem hiding this comment.
P1: When a configured locale uses an underscore, getFormatter passes the tag directly to Intl, which throws before formatting any value. Normalize underscores to hyphens before constructing the formatter.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/reflex-i18n/src/reflex_i18n/_web/i18n.js, line 220:
<comment>When a configured locale uses an underscore, `getFormatter` passes the tag directly to `Intl`, which throws before formatting any value. Normalize underscores to hyphens before constructing the formatter.</comment>
<file context>
@@ -0,0 +1,410 @@
+ const key = locale + " " + JSON.stringify(options ?? {});
+ let formatter = cache.get(key);
+ if (formatter === undefined) {
+ formatter = new Ctor(locale, options);
+ if (cache.size >= FORMATTER_CACHE_LIMIT) {
+ cache.delete(cache.keys().next().value);
</file context>
| formatter = new Ctor(locale, options); | |
| formatter = new Ctor(locale.replace(/_/g, "-"), options); |
There was a problem hiding this comment.
Fixed at the root in 9ec44ec: I18nConfig now rejects underscore locales outright instead of normalizing them at each use site. Locales double as URL segments, Intl tags and Accept-Language matches, all hyphen-separated, so the underscore spelling has no consistent meaning here.
There was a problem hiding this comment.
All reported issues were addressed across 26 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
FarhanAliRaza
left a comment
FarhanAliRaza
left a comment
There was a problem hiding this comment.
I exercised the translation example in Chrome with cookie and URL routing, compared client and server translations, and ran a production export and the focused tests. I also reran catalog initialization and loading with language-only and regional locale tags.
See the inline comments.
| ? undefined | ||
| : (event) => { | ||
| event.preventDefault(); | ||
| setLocale(locale); |
There was a problem hiding this comment.
[P2] Synchronize the backend locale from the cookie-mode switcher
With default cookie routing, this calls only the provider’s client setter; it never dispatches I18nState.set_locale. In the live en/de example, clicking de changes rx.t("Hello") to Hallo, but a computed var returning gettext("Welcome") stays Welcome, even after another backend event. The same var becomes Willkommen when using rx.i18n.set_locale("de"). Please make the cookie-mode switcher update both the client and server locale so dynamic translations follow the selected language.
| return read_po(f, locale=locale) | ||
| return Catalog(locale=locale) |
There was a problem hiding this comment.
[P2] Convert accepted locale tags before passing them to Babel
I18nConfig accepts hyphenated tags such as pt-BR and zh-Hant, but both calls pass them verbatim to Babel, which expects underscore-separated locale identifiers. uv run reflex i18n init pt-BR fails here with ValueError: expected only letters, got 'pt-br'; reading an existing catalog fails the same way, while de succeeds. Normalize the locale for both Babel calls (as the runtime catalog loader already does), while retaining the configured tag in filenames and URLs.
# Conflicts: # .github/workflows/dispatch_release.yml # packages/reflex-base/src/reflex_base/vars/dep_tracking.py # tests/units/test_state.py
Upstream now deletes the dispatch-time state tree locals before a background handler runs (reflex-dev#7447), while this branch enters the event scope around that handler with the same root state. The background scope is now created before the locals are deleted, and _EventScope drops its root state reference once entered, so a registered provider (e.g. the i18n locale) no longer pins the tree; test_scope.py covers the release. reflex-i18n takes upstream's workspace-wide Python 3.11 floor and the 3.11-3.15 classifiers. uv.lock is upstream's, relocked with uv 0.12.5 (the CI version) to add reflex-i18n and babel; pyi_hashes.json is regenerated.
Build the hydrate snapshot in a locale scope entered after browser storage is applied, convert hyphenated tags for Babel in the catalog CLI, and raise the reflex and reflex-base floors to the dev release that has the APIs reflex-i18n imports.
hydrate_and_load now also resolves the browser storage replay inside the re-derived event scope: since reflex-dev#7572 the replay redelivers cached storage dependents and recomputes those an override assignment invalidated, which otherwise ran under the locale derived before storage was applied. The hydrate scope test expects the redelivered value and covers the reassignment case.
Entering a StateProxy now also enters the event scope derived from the freshly loaded state, so a locale changed while a background task waited applies inside the block. The scope is closed on exit, restoring the task's own.
…e provider The client now negotiates the locale case-insensitively and returns the configured spelling, as the server does. The redirect from an unprefixed URL to its default-locale address moves from I18nProvider into HreflangLinks, so it happens even when the provider is not rendered.
A scope provider that suppresses the block's exception now has the state persisted as on a clean exit and the suppression propagated to the caller instead of the exception being re-raised.
Listing the reflex.cli entry points read every installed package's metadata on each `reflex --help`, adding about 8% to it. `reflex i18n` is now registered like deploy and cloud when reflex-i18n is installed.
I18nProvider gets an on_locale_change trigger bound to I18nState.set_locale, compiled like any event so its wire name follows the active name resolver; the Python set_locale helper opts out of it so the event isn't sent twice.
closes #2339