Skip to content

Commit e8f82b7

Browse files
committed
[FIX] Charts: Ensure Chart js extension are loaded on chart creation
When calling the method chartToImage, the chartJs extensions might not be loaded as we load them conditionally when mounting a chart. Hence, calling `ChartToImage` when there are no visible charts in the viewport or if we just instantiate a model without loading a component `Spreadsheet`, the extensions will be missing. Right now, the plugins/extensions are not fundamental to convert a chart to an image but this becomes problematic with the arrival of future charts (for instance Funnel). This commit adds a check to ensure that the extensisons are loaded and unloads them after use as they can cause crashes when using the global `ChartJs` variable elsewhere (see #6076). Task: 5214007 X-original-commit: d146422
1 parent b7280f7 commit e8f82b7

File tree

2 files changed

+50
-0
lines changed

2 files changed

+50
-0
lines changed
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
import { Chart } from "chart.js";
2+
import { Registry } from "../../../registry";
3+
4+
export const chartJsExtensionRegistry = new Registry<{
5+
register: (chart: typeof Chart) => void;
6+
unregister: (chart: typeof Chart) => void;
7+
}>();
8+
9+
const getChartJs = () => (globalThis as any).Chart;
10+
11+
export function areChartJSExtensionsLoaded() {
12+
return !!getChartJs().registry.plugins.get("chartShowValuesPlugin");
13+
}
14+
15+
export function registerChartJSExtensions() {
16+
if (!getChartJs() || areChartJSExtensionsLoaded()) {
17+
return;
18+
}
19+
for (const registryItem of chartJsExtensionRegistry.getAll()) {
20+
registryItem.register(getChartJs());
21+
}
22+
}
23+
24+
export function unregisterChartJsExtensions() {
25+
if (!getChartJs()) {
26+
return;
27+
}
28+
for (const registryItem of chartJsExtensionRegistry.getAll()) {
29+
registryItem.unregister(getChartJs());
30+
}
31+
}

packages/o-spreadsheet-engine/src/helpers/figures/charts/chart_ui_common.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,11 @@ import {
77
} from "../../../types/chart";
88
import { Figure } from "../../../types/figure";
99
import { deepCopy } from "../../misc";
10+
import {
11+
areChartJSExtensionsLoaded,
12+
registerChartJSExtensions,
13+
unregisterChartJsExtensions,
14+
} from "./chart_js_extension";
1015
import { drawGaugeChart } from "./gauge_chart_rendering";
1116
import { drawScoreChart } from "./scorecard_chart";
1217
import { getScorecardConfiguration } from "./scorecard_chart_config_builder";
@@ -35,6 +40,10 @@ export async function chartToImageUrl(
3540
const canvas = createRenderingSurface(figure.width, figure.height);
3641
let imageUrl: string | undefined;
3742
if ("chartJsConfig" in runtime) {
43+
const extensionsLoaded = areChartJSExtensionsLoaded();
44+
if (!extensionsLoaded) {
45+
registerChartJSExtensions();
46+
}
3847
const config = deepCopy(runtime.chartJsConfig);
3948
config.plugins = [backgroundColorChartJSPlugin];
4049
const chart = new (globalThis as any).Chart(canvas, config as ChartConfiguration);
@@ -43,6 +52,9 @@ export async function chartToImageUrl(
4352
} finally {
4453
chart.destroy();
4554
}
55+
if (!extensionsLoaded) {
56+
unregisterChartJsExtensions();
57+
}
4658
}
4759
// TODO: make a registry of chart types to their rendering functions
4860
else {
@@ -70,6 +82,10 @@ export async function chartToImageFile(
7082
const canvas = createRenderingSurface(figure.width, figure.height);
7183
let chartBlob: Blob | null = null;
7284
if ("chartJsConfig" in runtime) {
85+
const extensionsLoaded = areChartJSExtensionsLoaded();
86+
if (!extensionsLoaded) {
87+
registerChartJSExtensions();
88+
}
7389
const config = deepCopy(runtime.chartJsConfig);
7490
config.plugins = [backgroundColorChartJSPlugin];
7591
const chart = new (globalThis as any).Chart(canvas, config as ChartConfiguration);
@@ -78,6 +94,9 @@ export async function chartToImageFile(
7894
} finally {
7995
chart.destroy();
8096
}
97+
if (!extensionsLoaded) {
98+
unregisterChartJsExtensions();
99+
}
81100
} else {
82101
if (!globalThis.OffscreenCanvas)
83102
throw new Error(

0 commit comments

Comments
 (0)