add i18n support#6796
Conversation
Merging this PR will not alter performance
Comparing Footnotes
|
Greptile SummaryThis PR adds
Confidence Score: 5/5Safe to merge; no functional regressions identified in the new i18n subsystem or the core framework changes. The previously flagged issues (context manager leak in _EventScope.aenter, RFC 7231 q=0 exclusion in negotiate_locale, unhandled catalog-load rejection in i18n.js, missing set_locale in the lazy-loader mapping, and add_command outside the try block in reflex.py) are all addressed in this version. The event-scope integration, implicit dependency tracking, and client-side catalog compilation are logically sound. The only remaining findings are minor style and structural observations that do not affect runtime behaviour. Files Needing Attention: No files require special attention. reflex/i18n.py has a structural note about eager I18nState import, but the practical impact is minimal.
|
| Filename | Overview |
|---|---|
| packages/reflex-base/src/reflex_base/event/processor/scope.py | New per-event ambient context hook. The mid-loop BaseException guard in __aenter__ correctly prevents contextvar leaks when a later provider raises. |
| packages/reflex-base/src/reflex_base/event/processor/base_state_processor.py | Wraps both handler execution and delta resolution in event_scope; background tasks get their own second scope after the lock is dropped. Flow is correct. |
| packages/reflex-i18n/src/reflex_i18n/runtime.py | Server-side gettext + Babel formatting. RFC 7231 q=0 exclusion, ContextVar locale management, lazy translation caching, and implicit dependency registration are all correctly implemented. |
| packages/reflex-i18n/src/reflex_i18n/state.py | I18nState with cookie-backed locale, locale negotiation, and event scope provider. The _locale_scope correctly re-reads the cookie for each event phase so a locale change by a handler is visible during delta resolution. |
| packages/reflex-i18n/src/reflex_i18n/_web/i18n.js | Client runtime: I18nProvider with cookie + navigator.languages locale negotiation. Promise rejection on catalog load is caught; Secure flag on cookie is conditioned on HTTPS. Race condition on rapid locale changes is handled via a cancelled flag. |
| packages/reflex-i18n/src/reflex_i18n/catalog.py | Compiles .po catalogs to JS modules. Plural expression is whitelist-validated before embedding. json.dumps used for all string values, preventing injection. |
| packages/reflex-i18n/src/reflex_i18n/vars.py | Implements rx.t() — requires literal strings at call time for catalog extraction; properly validates plural/count pairing. |
| packages/reflex-i18n/src/reflex_i18n/format.py | Client-side locale-aware number/date formatting vars using Intl API via useFormat hook. Formatter cache capped at 100 entries to prevent unbounded growth. |
| packages/reflex-i18n/src/reflex_i18n/plugin.py | I18nPlugin registers state and ships the JS runtime. Lazy state import in post_init avoids registering I18nState for apps that only load the CLI entry point. |
| reflex/init.py | Adds i18n lazy-loader mapping including set_locale and t; correct. |
| reflex/i18n.py | Re-exports all reflex_i18n public API. Eagerly imports I18nState (which registers it as a substate), meaning accessing any rx.* i18n attribute triggers state registration — partially defeating the lazy-load intent in reflex_i18n/init.py. |
| reflex/reflex.py | Plugin CLI command loading now validates that the resolved object is a click.Command before calling add_command, preventing a TypeError from aborting the whole CLI startup. |
| packages/reflex-base/src/reflex_base/vars/dep_tracking.py | Adds implicit dependency registration for LOAD_GLOBAL and LOAD_DEREF opcodes. Unhashable-object guard via try/except TypeError is correct. |
| reflex/state.py | Splits reload_state_module to also purge dangling dependency edges pointing at removed states, fixing a hot-reload crash in _mark_dirty_computed_vars. |
| packages/reflex-i18n/src/reflex_i18n/cli.py | Implements reflex i18n extract/init/check commands. Stats and merge logic are correct; check_command does not write to disk. |
Reviews (10): Last reviewed commit: "Merge remote-tracking branch 'upstream/m..." | Re-trigger Greptile
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.
Remove development pins before publishing reflex-i18n
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
closes #2339