sadpandajoe opened a new pull request, #42317:
URL: https://github.com/apache/superset/pull/42317
### SUMMARY
Every chart with a temporal x-axis (line, area, bar, ...) throws
`TypeError: Cannot read properties of null (reading '0')` while rendering
axis labels.
echarts ships **two** UMD builds per locale:
| file | behaviour |
|---|---|
| `echarts/i18n/langEN.js` | self-registers under echarts' own key,
**exports nothing** |
| `echarts/i18n/langEN-obj.js` | **exports** the locale object |
`Echart.tsx` imported the first one, so `(await loader()).default` was `{}`
— empty, but
truthy, so it passed the `if (localeObj)` guard and got handed to
`registerLocale('EN', {})`.
That call overwrote the complete locale the very same module had just
self-registered,
leaving the registered locale with no `time` section.
echarts' `formatTime` builds its result as a chained `.replace()`, and
replacement
arguments are evaluated **eagerly** — it reads `month[u - 1]` even when the
template is
just `{yyyy}`. With `time` missing, `month` is `null` and every single axis
label throws.
The fix switches all 24 loaders to the `-obj.js` build, and additionally
refuses to register
a locale object that has no `time` section, so a future bad import degrades
to echarts'
built-in locale instead of crashing the chart.
This also stops dragging the full `echarts` bundle in: the `langXX.js` CJS
branch does
`require('echarts/lib/echarts')`, which defeats the tree-shaken
`echarts/core` setup this
plugin uses. The `-obj` factory takes only `exports`. (Improvement expected,
not measured.)
#### Regression origin
`echarts/lib/i18n/lang${locale}` (a real ESM module with a full `export
default`) was
swapped for `echarts/i18n/lang${locale}.js` in #42055, and that path was
then carried
forward into the static `LOCALE_LOADERS` map in #42154. The defect is
independent of the
echarts major version — 5.6.0 and 6.1.0 both ship the same two i18n flavours.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!-- N/A - the "before" is a chart that fails to render at all -->
### TESTING INSTRUCTIONS
1. Open any chart with a temporal x-axis (e.g. a line chart from the sample
data).
2. Before this change the chart fails to render and the console shows
`Cannot read properties of null (reading '0')` from echarts' `formatTime`.
After this change the chart renders with formatted time axis labels.
3. Repeat with a non-English locale to confirm localized labels still work.
Automated:
```
cd superset-frontend
npm run test -- plugins/plugin-chart-echarts
```
A new test in `test/components/Echart.test.tsx` asserts the object handed to
`registerLocale` actually carries a 12-entry `time.month`, which fails
against the
previous import path.
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [x] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
