feat(plotly): add per-chart locale support#6428
feat(plotly): add per-chart locale support#6428BABTUNA wants to merge 1 commit intoreflex-dev:mainfrom
Conversation
Greptile SummaryThis PR adds a
Confidence Score: 3/5Not safe to merge without verifying that dist-variant subclasses receive the A P1 finding (missing import in subclass overrides) could cause a silent runtime ReferenceError for all users of the dist variants when locale is set. The existing tests only validate the Python render output, not the browser-side import map, so the bug would not be caught by CI. packages/reflex-components-plotly/src/reflex_components_plotly/plotly.py — specifically the Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["rx.plotly(data=fig, locale='de')"] --> B["Plotly._render()"]
B --> C{self.locale is not None?}
C -- No --> D["Return tag as-is"]
C -- Yes --> E["Resolve config Var\n(self.config or empty dict)"]
E --> F["Inject config prop:\n_rxGetPlotlyLocaleConfig(config, locale, plotlyLocales)"]
F --> G["Browser: _rxResolvePlotlyLocaleData\nlooks up locale in plotlyLocales bundle"]
G --> H{Locale found?}
H -- Yes --> I["Set config.locale = localeData.name\nSet config.locales[name] = localeData"]
H -- No --> J["Set config.locale = String(locale)\n(fallback — no locale dict injected)"]
I --> K["React Plot renders with locale"]
J --> K
subgraph Import side
L["Plotly.add_imports()\n→ plotly.js-locales@3.5.0 (always)"]
M["PlotlyBasic.add_imports()\n→ CREATE_PLOTLY_COMPONENT only\n⚠️ no super() call"]
end
|
| return { | ||
| # For merging plotly data/layout/templates. | ||
| "mergician@v2.0.2": "mergician" | ||
| "mergician@v2.0.2": "mergician", | ||
| # For locale dictionaries injected into plot config.locales. | ||
| "plotly.js-locales@3.5.0": ImportVar( | ||
| tag="plotlyLocales", | ||
| is_default=True, | ||
| ), | ||
| } |
There was a problem hiding this comment.
Locale bundle always included regardless of usage
plotly.js-locales@3.5.0 is unconditionally added to the import map, and add_custom_code() always emits the two helper functions that reference it. This means every app that uses rx.plotly (even with no locale prop) will bundle all 70+ locale definitions and include the helper code, adding unnecessary weight for users who never touch the locale feature.
A conditional approach would require add_imports and add_custom_code to only emit locale-related output when the component is actually configured with a locale, which isn't easy at the class level. At minimum, the trade-off should be noted in the docstring or changelog so users are aware of the bundle size impact.
There was a problem hiding this comment.
agree. see how we handle it in moment.py when the user supplies the tz prop. the import should be added conditionally based on the locale prop being provided
There was a problem hiding this comment.
The pattern from moment.py is clear. Here's how to apply it to the plotly add_imports:
| return { | |
| # For merging plotly data/layout/templates. | |
| "mergician@v2.0.2": "mergician" | |
| "mergician@v2.0.2": "mergician", | |
| # For locale dictionaries injected into plot config.locales. | |
| "plotly.js-locales@3.5.0": ImportVar( | |
| tag="plotlyLocales", | |
| is_default=True, | |
| ), | |
| } | |
| def add_imports(self) -> ImportDict: | |
| """Add imports for the plotly component. | |
| Returns: | |
| The imports for the plotly component. | |
| """ | |
| imports: ImportDict = { | |
| # For merging plotly data/layout/templates. | |
| "mergician@v2.0.2": "mergician", | |
| } | |
| if self.locale is not None: | |
| # For locale dictionaries injected into plot config.locales. | |
| imports["plotly.js-locales@3.5.0"] = ImportVar( | |
| tag="plotlyLocales", | |
| is_default=True, | |
| ) | |
| return imports |
Same idea applies to add_custom_code — it should also gate the locale helper functions behind if self.locale is not None so the helper code is only emitted when a locale is actually used.
Tip: You can customize Greptile's behavior for this repo with .greptile/rules.md and .greptile/config.json.
All Submissions:
Type of change
New Feature Submission:
Changes To Core Features:
Description
Adds first-pass Plotly locale support on
x.plotly and plotly dist variants via a new locale prop.
What changed:
x.plotly(...) and
x.plotly.basic(...) locale flow.
Validation:
Closes #6101