[REV] Charts: Do not rely on history for the chartRuntimes

Since [1], the commands "UNDO" and "REDO" no longer invalidate the chart
evaluation as it was supposedly handled by the plugin history. However,
the ChartEvaluation plugin would lazy create the chart runtime and
update the history outside of the dispatch context (the changes are
therefore not recorded).
After some consideration, we consider that the storage cost of the
objects `ChartRuntime` is greater than having to invalidate/recompute it
at each UNDO/REDO. We therefore revert the change.

[1] #2408

closes #3150

Task: 3578417
X-original-commit: 80bd3b3
Signed-off-by: Lucas Lefèvre (lul) <[email protected]>
Signed-off-by: Rémi Rahir (rar) <[email protected]>

src/plugins/ui_core_views/evaluation_chart.ts

@@ -1,6 +1,6 @@
 import { BACKGROUND_CHART_COLOR } from "../../constants";
 import { chartRuntimeFactory, chartToImage } from "../../helpers/figures/charts";
-import { Color, ExcelWorkbookData, FigureData, Immutable, Range, UID } from "../../types";
+import { Color, ExcelWorkbookData, FigureData, Range, UID } from "../../types";
 import { ChartRuntime, ExcelChartDefinition } from "../../types/chart/chart";
 import {
   CoreViewCommand,
@@ -10,12 +10,12 @@ import {
 import { UIPlugin } from "../ui_plugin";
 
 interface EvaluationChartState {
-  readonly charts: Immutable<Record<UID, ChartRuntime | undefined>>;
+  charts: Record<UID, ChartRuntime | undefined>;
 }
 export class EvaluationChartPlugin extends UIPlugin<EvaluationChartState> {
   static getters = ["getChartRuntime", "getBackgroundOfSingleCellChart"] as const;
 
-  readonly charts: Immutable<Record<UID, ChartRuntime | undefined>> = {};
+  charts: Record<UID, ChartRuntime | undefined> = {};
 
   private createRuntimeChart = chartRuntimeFactory(this.getters);
 
@@ -26,23 +26,21 @@ export class EvaluationChartPlugin extends UIPlugin<EvaluationChartState> {
       cmd.type === "EVALUATE_CELLS" ||
       cmd.type === "UPDATE_CELL"
     ) {
-      if (cmd.type !== "UNDO" && cmd.type !== "REDO") {
-        for (const chartId in this.charts) {
-          this.history.update("charts", chartId, undefined);
-        }
+      for (const chartId in this.charts) {
+        this.charts[chartId] = undefined;
       }
     }
 
     switch (cmd.type) {
       case "UPDATE_CHART":
       case "CREATE_CHART":
       case "DELETE_FIGURE":
-        this.history.update("charts", cmd.id, undefined);
+        this.charts[cmd.id] = undefined;
         break;
       case "DELETE_SHEET":
         for (let chartId in this.charts) {
           if (!this.getters.isChartDefined(chartId)) {
-            this.history.update("charts", chartId, undefined);
+            this.charts[chartId] = undefined;
           }
         }
         break;
@@ -55,8 +53,7 @@ export class EvaluationChartPlugin extends UIPlugin<EvaluationChartState> {
       if (!chart) {
         throw new Error(`No chart for the given id: ${figureId}`);
       }
-      const runtime = this.createRuntimeChart(chart) as Immutable<ChartRuntime>;
-      this.history.update("charts", figureId, runtime);
+      this.charts[figureId] = this.createRuntimeChart(chart);
     }
     return this.charts[figureId] as ChartRuntime;
   }

tests/figures/chart/chart_plugin.test.ts

@@ -2004,6 +2004,23 @@ describe("Chart evaluation", () => {
         .data![0]
     ).toBe("#REF");
   });
+
+  test("undo/redo invalidates the chart runtime", () => {
+    const chartId = "test";
+    setCellContent(model, "A1", "oui");
+    setCellContent(model, "A2", "non");
+    createChart(model, {}, chartId);
+
+    updateChart(model, chartId, { labelRange: "A1:A2" });
+    const chartRuntime1 = model.getters.getChartRuntime(chartId) as BarChartRuntime;
+    undo(model);
+    const chartRuntime2 = model.getters.getChartRuntime(chartId) as BarChartRuntime;
+    redo(model);
+    const chartRuntime3 = model.getters.getChartRuntime(chartId) as BarChartRuntime;
+    expect(chartRuntime1.chartJsConfig.data?.labels).toEqual(["oui", "non"]);
+    expect(chartRuntime2.chartJsConfig.data?.labels).toEqual([]);
+    expect(chartRuntime3.chartJsConfig.data?.labels).toEqual(["oui", "non"]);
+  });
 });
 
 describe("Cumulative Data line chart", () => {