feat(plotly): add locale prop and Config.plotly_locale for i18n chart formatting#6193
feat(plotly): add locale prop and Config.plotly_locale for i18n chart formatting#6193pranavmanglik wants to merge 5 commits intoreflex-dev:mainfrom
Conversation
|
Here, if you are using |
Greptile SummaryThis PR adds first-class i18n support to Confidence Score: 3/5Not safe to merge as-is — an unrelated is_default change breaks all Plotly dist subclasses at runtime. The locale feature itself is well-structured (race-condition fixed, path traversal guarded, graceful fallback), but the PR flips createPlotlyComponent's is_default from True to False — breaking PlotlyBasic, PlotlyCartesian, PlotlyGeo, PlotlyGl3d, PlotlyGl2d, PlotlyMapbox, PlotlyFinance, and PlotlyStrict. The two P2 issues (CSP eval concern, unused npm package) don't block merge on their own. reflex/components/plotly/plotly.py — the is_default=False change at CREATE_PLOTLY_COMPONENT must be reviewed and most likely reverted. Important Files Changed
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
reflex/components/plotly/plotly.py
Outdated
| # Render through _RxPlotLocale wrapper which handles locale loading. | ||
| # `tag = "Plot"` above tells Reflex to auto-import Plot from react-plotly.js; | ||
| # we override the rendered element name here so the JSX uses _RxPlotLocale. | ||
| tag = tag.set(name="_RxPlotLocale") |
There was a problem hiding this comment.
_RxPlotLocale hardcodes Plot — all sub-classes broken
_render() is now inherited by every Plotly sub-class (PlotlyBasic, PlotlyCartesian, PlotlyGeo, etc.) and unconditionally rewrites the element name to _RxPlotLocale. But the _RxPlotLocale JS function hardcodes React.createElement(Plot, ...), where Plot is the default import from react-plotly.js (the full, heavy bundle).
Before this PR each sub-class rendered its own dynamic component (BasicPlotlyPlot, CartesianPlotlyPlot, etc.). After this PR they all render through _RxPlotLocale → Plot, ignoring their lightweight dist variants entirely.
The _render() override should be guarded so it only applies when the component is actually the base Plotly class and a locale is set, or the _RxPlotLocale wrapper needs to accept the inner component as a prop so sub-classes can pass their specific dynamic component name:
function _RxPlotLocale({ locale, config, _plotComponent, ...rest }) {
const PlotComponent = _plotComponent || Plot;
...
return React.createElement(PlotComponent, { ...rest, config: mergedConfig });
}
reflex/components/plotly/plotly.py
Outdated
| function _rxLoadLocale(locale) { | ||
| const key = locale.toLowerCase(); | ||
| if (_rxLocaleCache[key]) return Promise.resolve(_rxLocaleCache[key]); | ||
| const url = `/node_modules/plotly.js-locales/${key}.js`; |
There was a problem hiding this comment.
/node_modules/ will 404 in production
The fetch URL is constructed as /node_modules/plotly.js-locales/${key}.js. During reflex run (dev mode) the frontend dev-server may serve node_modules, but after reflex build the production output is a static bundle — there is no /node_modules/ directory served at that path. Every locale fetch will return a 404, the catch branch will fire, and all non-English locales will silently fall back to English in production.
The locale files should either be:
- Copied to the public/static directory at build time and served from there, or
- Imported at bundle time (e.g., as JSON) so Vite/webpack can include them in the bundle.
Using a CDN URL (e.g. https://cdn.jsdelivr.net/npm/plotly.js-locales@3.3.1/${key}.js) is another option but adds an external network dependency.
| from reflex.config import get_config | ||
|
|
||
| # Apply global plotly_locale from rxconfig.py if no per-chart locale given. | ||
| if not props.get("locale"): |
There was a problem hiding this comment.
locale="" is treated as "not set", overriding with global config
if not props.get("locale") is falsy for both None (locale not supplied at all) and "" (explicitly passed as empty to opt-out). If Config.plotly_locale = "de" and a developer writes rx.plotly(data=fig, locale="") to force English on a specific chart, the global default would still be applied.
The check should differentiate between "not supplied" and "explicitly empty":
| if not props.get("locale"): | |
| if "locale" not in props: |
| # List of plugin types to disable in the app. | ||
| disable_plugins: list[type[Plugin]] = dataclasses.field(default_factory=list) | ||
|
|
||
| plotly_locale: str = "" |
There was a problem hiding this comment.
plotly_locale
Every other field in BaseConfig has a # <description> comment. This field was added without one, and it's also missing the blank line that separates logical sections from neighbouring fields (consistent with the custom instruction rule on code readability).
| plotly_locale: str = "" | |
| # The default BCP-47 locale code for all rx.plotly charts (e.g. "de", "fr", "zh-CN"). | |
| # Can be overridden per-chart with the locale= prop. Also settable via REFLEX_PLOTLY_LOCALE. | |
| plotly_locale: str = "" | |
Rule Used: Add blank lines between logical sections of code f... (source)
Learnt From
reflex-dev/flexgen#2170
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| """Tests for rx.plotly locale support.""" | ||
| from types import SimpleNamespace | ||
| from unittest.mock import patch |
There was a problem hiding this comment.
SimpleNamespace and patch are imported but never referenced in any of the five tests.
| """Tests for rx.plotly locale support.""" | |
| from types import SimpleNamespace | |
| from unittest.mock import patch | |
| """Tests for rx.plotly locale support.""" | |
| from reflex.components.plotly.plotly import Plotly | |
| from reflex.vars.base import LiteralVar |
| function _rxLoadLocale(locale) { | ||
| const key = locale.toLowerCase(); | ||
| if (_rxLocaleCache[key]) return Promise.resolve(_rxLocaleCache[key]); | ||
| const url = `/node_modules/plotly.js-locales/${key}.js`; | ||
| return fetch(url) | ||
| .then(r => { | ||
| if (!r.ok) throw new Error("HTTP " + r.status); | ||
| return r.text(); | ||
| }) | ||
| .then(code => { | ||
| const mod = { exports: {} }; | ||
| new Function("module", "exports", code)(mod, mod.exports); | ||
| _rxLocaleCache[key] = mod.exports; | ||
| return mod.exports; | ||
| }) | ||
| .catch(e => { | ||
| console.warn( | ||
| "[rx.plotly] Locale \\"" + locale + "\\" could not be loaded: " + e.message + | ||
| ". Check https://www.npmjs.com/package/plotly.js-locales for supported codes." | ||
| ); | ||
| return null; | ||
| }); | ||
| } |
There was a problem hiding this comment.
new Function eval with user-controlled locale
The PR description explicitly supports state-driven locales (locale=AppState.locale). If AppState.locale is derived from user input and is not validated, a crafted value like ../../some/path would change the fetch URL to /node_modules/plotly.js-locales/../../some/path.js. The fetched content is then passed directly to new Function("module", "exports", code) and executed — essentially running arbitrary file content as JavaScript.
Before building the URL, the locale key should be validated against a strict allowlist pattern (e.g., allowing only [a-z]{2,3} optionally followed by -[a-z]{2,4}) and rejected with a console warning if it does not match.
| const _rxLocaleCache = {}; | ||
|
|
||
| function _rxLoadLocale(locale) { | ||
| const key = locale.toLowerCase(); | ||
| if (_rxLocaleCache[key]) return Promise.resolve(_rxLocaleCache[key]); | ||
| const url = `/node_modules/plotly.js-locales/${key}.js`; | ||
| return fetch(url) | ||
| .then(r => { | ||
| if (!r.ok) throw new Error("HTTP " + r.status); | ||
| return r.text(); | ||
| }) | ||
| .then(code => { | ||
| const mod = { exports: {} }; | ||
| new Function("module", "exports", code)(mod, mod.exports); | ||
| _rxLocaleCache[key] = mod.exports; | ||
| return mod.exports; | ||
| }) | ||
| .catch(e => { | ||
| console.warn( | ||
| "[rx.plotly] Locale \\"" + locale + "\\" could not be loaded: " + e.message + | ||
| ". Check https://www.npmjs.com/package/plotly.js-locales for supported codes." | ||
| ); | ||
| return null; | ||
| }); | ||
| } |
There was a problem hiding this comment.
_rxLocaleCache race condition — duplicate in-flight fetches
The cache check if (_rxLocaleCache[key]) only guards against re-fetching after a promise has already resolved. If two _RxPlotLocale components with the same non-English locale mount simultaneously, both call _rxLoadLocale before either resolves, find no cache entry, and issue duplicate network requests.
Storing the Promise in the cache rather than the resolved value fixes this — any subsequent call for the same key returns the same pending promise immediately.
1 participant
feat(plotly): add
localeprop andConfig.plotly_localefor i18n chart formattingAll Submissions:
Type of change
New Feature Submission:
Changes To Core Features:
Summary
Adds a
localeprop torx.plotly(and all plotly subcomponents) so that charts automatically follow user language and regional formatting — number separators, date axis labels, month/weekday names, and modebar button tooltips. Also addsplotly_localetorx.Configas an app-wide default.Problem
rx.plotlyhad no way to configure the Plotly.js locale. All charts rendered withen-USdefaults regardless of the user's language — decimal separators, date axis tick labels, month/weekday names, and modebar button tooltips were always in English with no first-class Reflex API to change this.Developers targeting non-English audiences had to ship their own JS shims to work around this.
Usage
Per-chart
locale=takes precedence overConfig.plotly_locale. If neither is set, Plotly's built-in"en"defaults apply — zero behaviour change for existing apps.What locale affects
Test results
Manually verified with
locale="de"andlocale="zh-CN"usingreflex run. Confirmed German modebar tooltips ("Hineinzoomen", "Graphen als PNG herunterladen") and Chinese tooltips rendering correctly. Default chart (no locale set) behaviour is unchanged.Supported locales
~70 locales from
# feat(plotly): add `locale` prop and `Config.plotly_locale` for i18n chart formattingplotly.js-locales(MIT licensed), including:af,ar,bg,cs,da,de,de-ch,el,es,es-ar,et,fa,fi,fr,fr-ch,he,hi-in,hr,hu,id,it,ja,ko,lt,lv,nl,no,pl,pt-br,pt-pt,ro,ru,sk,sl,sr,sv,th,tr,uk,vi,zh-cn,zh-hk,zh-twand more.All Submissions:
Type of change
New Feature Submission:
Changes To Core Features:
Summary
Adds a
localeprop torx.plotly(and all plotly subcomponents) so that charts automatically follow user language and regional formatting — number separators, date axis labels, month/weekday names, and modebar button tooltips. Also addsplotly_localetorx.Configas an app-wide default.Problem
rx.plotlyhad no way to configure the Plotly.js locale. All charts rendered withen-USdefaults regardless of the user's language — decimal separators, date axis tick labels, month/weekday names, and modebar button tooltips were always in English with no first-class Reflex API to change this.Developers targeting non-English audiences had to ship their own JS shims to work around this.
Usage
Per-chart
locale=takes precedence overConfig.plotly_locale. If neither is set, Plotly's built-in"en"defaults apply — zero behaviour change for existing apps.What locale affects
Implementation
Uses Plotly.js's built-in
config.localesinline injection:This avoids
Plotly.register()and any dynamic JS imports entirely. The locale file is fetched at runtime viafetch()from theplotly.js-localespackage, parsed with anew FunctionCJS sandbox, and passed inline through the config prop. This sidesteps all Vite/bundler CJS→ESM conversion issues.A thin React wrapper
_RxPlotLocale(injected viaadd_custom_code) handles the async fetch and forwards all other props to<Plot>unchanged. Sincetag = "Plot"causes Reflex to auto-importPlotfromreact-plotly.js, the rendered element name is overridden in_render()viatag.set(name="_RxPlotLocale").Unknown locale codes fall back gracefully to
"en"with a console warning:Files changed
reflex/components/plotly/plotly.pylocale: Var[str]prop; add_rxLoadLocale+_RxPlotLocaletoadd_custom_code; addplotly.js-localestolib_dependencies; override rendered tag name in_render; read global config fallback increate()reflex/config.pyplotly_locale: str = ""toBaseConfigtests/components/plotly/test_plotly.pyTest results
Manually verified with
locale="de"andlocale="zh-CN"usingreflex run. Confirmed German modebar tooltips ("Hineinzoomen", "Graphen als PNG herunterladen") and Chinese tooltips rendering correctly. Default chart (no locale set) behaviour is unchanged.Supported locales
~70 locales from
[plotly.js-locales](https://www.npmjs.com/package/plotly.js-locales)(MIT licensed), including:af,ar,bg,cs,da,de,de-ch,el,es,es-ar,et,fa,fi,fr,fr-ch,he,hi-in,hr,hu,id,it,ja,ko,lt,lv,nl,no,pl,pt-br,pt-pt,ro,ru,sk,sl,sr,sv,th,tr,uk,vi,zh-cn,zh-hk,zh-twand more.