[FIX] chart: avoid useless chart updates

There was an `useEffect` in the `chartJS` component to update the
chartJS object on runtime change. There was 2 problems:

1) useEffect on object compare the references, so if the chart plugin
was ever changed to return copy of runtime it would break
2) the chart plugin re-build every chartRuntime on each command that
could affect a chart. This created a chart update at each UPDATE_CELL
even if the actual runtime didn't change.

This commit replace the `useEffect` dependency with a `deepEquals`

closes #4154

Task: 3697660
X-original-commit: 6eb4353
Signed-off-by: Pierre Rousseau (pro) <pro@odoo.com>

src/components/figures/chart/chartJs/chartjs.ts

@@ -1,5 +1,6 @@
 import { Component, onMounted, useEffect, useRef } from "@odoo/owl";
 import type { Chart, ChartConfiguration } from "chart.js";
+import { deepCopy, deepEquals } from "../../../../helpers";
 import { Figure, SpreadsheetChildEnv } from "../../../../types";
 import { ChartJSRuntime } from "../../../../types/chart/chart";
 import { GaugeChartConfiguration, GaugeChartOptions } from "../../../../types/chart/gauge_chart";
@@ -16,6 +17,7 @@ export class ChartJsComponent extends Component<Props, SpreadsheetChildEnv> {
 
   private canvas = useRef("graphContainer");
   private chart?: Chart;
+  private currentRuntime!: ChartJSRuntime;
 
   get background(): string {
     return this.chartRuntime.background;
@@ -36,12 +38,17 @@ export class ChartJsComponent extends Component<Props, SpreadsheetChildEnv> {
   setup() {
     onMounted(() => {
       const runtime = this.chartRuntime;
-      this.createChart(runtime.chartJsConfig);
+      this.currentRuntime = runtime;
+      // Note: chartJS modify the runtime in place, so it's important to give it a copy
+      this.createChart(deepCopy(runtime.chartJsConfig));
+    });
+    useEffect(() => {
+      const runtime = this.chartRuntime;
+      if (!deepEquals(runtime, this.currentRuntime, "ignoreFunctions")) {
+        this.currentRuntime = runtime;
+        this.updateChartJs(deepCopy(runtime));
+      }
     });
-    useEffect(
-      () => this.updateChartJs(this.chartRuntime),
-      () => [this.chartRuntime]
-    );
   }
 
   private createChart(chartData: ChartConfiguration | GaugeChartConfiguration) {

src/helpers/misc.ts

@@ -350,7 +350,7 @@ export function getAddHeaderStartIndex(position: "before" | "after", base: numbe
 /**
  * Compares two objects.
  */
-export function deepEquals(o1: any, o2: any): boolean {
+export function deepEquals(o1: any, o2: any, ignoreFunctions?: "ignoreFunctions"): boolean {
   if (o1 === o2) return true;
   if ((o1 && !o2) || (o2 && !o1)) return false;
   if (typeof o1 !== typeof o2) return false;
@@ -364,10 +364,12 @@ export function deepEquals(o1: any, o2: any): boolean {
   }
 
   for (const key in o1) {
-    if (typeof o1[key] !== typeof o2[key]) return false;
-    if (typeof o1[key] === "object") {
-      if (!deepEquals(o1[key], o2[key])) return false;
+    const typeOfO1Key = typeof o1[key];
+    if (typeOfO1Key !== typeof o2[key]) return false;
+    if (typeOfO1Key === "object") {
+      if (!deepEquals(o1[key], o2[key], ignoreFunctions)) return false;
     } else {
+      if (ignoreFunctions && typeOfO1Key === "function") return true;
       if (o1[key] !== o2[key]) return false;
     }
   }

tests/figures/chart/charts_component.test.ts

@@ -1118,6 +1118,15 @@ describe("charts", () => {
     await keyDown({ key: "Z", ctrlKey: true });
     expect(getCellContent(model, "D6")).toEqual("");
   });
+
+  test("Chart is not re-rendered if its runtime do not change", async () => {
+    const updateChart = jest.spyOn((window as any).Chart.prototype, "update");
+    createTestChart("basicChart");
+    await nextTick();
+    setCellContent(model, "C3", "value");
+    await nextTick();
+    expect(updateChart).not.toHaveBeenCalled();
+  });
 });
 
 describe("charts with multiple sheets", () => {

tests/helpers/misc_helpers.test.ts

@@ -233,6 +233,13 @@ test.each([
   expect(deepEquals(o2, o1)).toEqual(expectedResult);
 });
 
+test("deepEquals with argument ignoring functions", () => {
+  const o1 = { a: 1, b: () => 2 };
+  const o2 = { a: 1, b: () => 2 };
+  expect(deepEquals(o1, o2)).toEqual(false);
+  expect(deepEquals(o1, o2, "ignoreFunctions")).toEqual(true);
+});
+
 describe("isConsecutive", () => {
   test("consecutive", () => {
     expect(isConsecutive([2, 3, 1])).toBeTruthy();

tests/test_helpers/helpers.ts

@@ -625,7 +625,7 @@ export const mockChart = () => {
     }
     toBase64Image = () => "";
     destroy = () => {};
-    update = () => {};
+    update() {}
     options = mockChartData.options;
     config = mockChartData;
   }