Skip to content

Commit b41ac06

Browse files
committed
fix(plugin-chart-echarts): guard loadLocale on the time section, dispose test chart
Review feedback: the keys-length guard was weaker than the crash invariant. The failure mode is echarts' time formatter reading time.month from a locale registered without a time section, so check for that section directly; a partial bundle now falls back to the builtin locale too. Also dispose the SSR chart instance the rendering test creates so it doesn't leak across the jest worker. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent d5e59d4 commit b41ac06

2 files changed

Lines changed: 27 additions & 19 deletions

File tree

superset-frontend/plugins/plugin-chart-echarts/src/components/echartsLocale.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,11 @@ export const loadLocale = async (
9494
// Depending on the module system, the locale object is either the
9595
// module's default export or the module namespace itself.
9696
const localeObj = mod.default ?? (mod as unknown as EChartsLocaleOption);
97-
if (localeObj && Object.keys(localeObj).length > 0) {
97+
// registerLocale replaces any builtin entry under the same key, and
98+
// `time` is the section whose absence crashes every time-axis chart, so
99+
// a locale without it (an empty or partial bundle) must not be
100+
// registered. Fall back to the builtin locale instead.
101+
if (localeObj?.time) {
98102
return localeObj;
99103
}
100104
} catch {

superset-frontend/plugins/plugin-chart-echarts/test/components/echartsLocale.test.ts

Lines changed: 22 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -57,22 +57,26 @@ test('registering the loaded EN locale keeps time-axis charts rendering', async
5757
height: 300,
5858
locale: 'EN',
5959
});
60-
chart.setOption(
61-
{
62-
xAxis: { type: 'time' },
63-
yAxis: { type: 'value' },
64-
series: [
65-
{
66-
type: 'line',
67-
data: [
68-
[new Date(1965, 0, 1).getTime(), 100],
69-
[new Date(1985, 0, 1).getTime(), 200],
70-
[new Date(2005, 0, 1).getTime(), 150],
71-
],
72-
},
73-
],
74-
},
75-
{ notMerge: true, lazyUpdate: false },
76-
);
77-
expect(chart.renderToSVGString()).toContain('<svg');
60+
try {
61+
chart.setOption(
62+
{
63+
xAxis: { type: 'time' },
64+
yAxis: { type: 'value' },
65+
series: [
66+
{
67+
type: 'line',
68+
data: [
69+
[new Date(1965, 0, 1).getTime(), 100],
70+
[new Date(1985, 0, 1).getTime(), 200],
71+
[new Date(2005, 0, 1).getTime(), 150],
72+
],
73+
},
74+
],
75+
},
76+
{ notMerge: true, lazyUpdate: false },
77+
);
78+
expect(chart.renderToSVGString()).toContain('<svg');
79+
} finally {
80+
chart.dispose();
81+
}
7882
});

0 commit comments

Comments
 (0)