[charts] Readonly array inputs

[JCQuintas]

This might be a bit overarching, but currently sometimes when we use [] as const it can't be used as the input for our chart components because readonly arrays can't be downgraded to mutable arrays.

This PR tries to fix that by making all the array inputs readonly

Changelog

• Charts array inputs are now readonly. Allowing externally defined as const to be used as a prop value of the React component.

  const xAxis = [{ position: 'bottom' }] as const
  <BarChart xAxis={xAxis} />

[mui-bot]

Deploy preview: https://deploy-preview-16632--material-ui-x.netlify.app/

Generated by 🚫 dangerJS against b120768

[codspeed-hq]

CodSpeed Performance Report

Merging #16632 will not alter performance

_{Comparing JCQuintas:readonly-arrays (b120768) with master (3ed991b)}

Summary

✅ 6 untouched benchmarks

[bernardobelchior]

Good change 👍 Left a couple questions

[bernardobelchior]

This comment applies to this whole file: why do we need to create a new array here? Isn't paramsZoomData a mutable array?

[JCQuintas]

const RO_MUT: readonly [] = [] // ok
const MUT_RO: [] = [] as readonly [] // not ok

[alexfauquette]

What about also moving the store array to readonly?

We do not mutate the store content so it should be fine and avoid those [...xxx]

diff --git a/packages/x-charts-pro/src/internals/plugins/useChartProZoom/useChartProZoom.ts b/packages/x-charts-pro/src/internals/plugins/useChartProZoom/useChartProZoom.ts
index c5d13c2ca..33a17098c 100644
--- a/packages/x-charts-pro/src/internals/plugins/useChartProZoom/useChartProZoom.ts
+++ b/packages/x-charts-pro/src/internals/plugins/useChartProZoom/useChartProZoom.ts
@@ -25,7 +25,7 @@ import {
 } from './useChartProZoom.utils';
 
 // It is helpful to avoid the need to provide the possibly auto-generated id for each axis.
-function initializeZoomData(options: Record<AxisId, DefaultizedZoomOptions>) {
+function initializeZoomData(options: Record<AxisId, DefaultizedZoomOptions>): readonly ZoomData[] {
   return Object.values(options).map(({ axisId, minStart: start, maxEnd: end }) => ({
     axisId,
     start,
@@ -69,7 +69,7 @@ export const useChartProZoom: ChartPlugin<UseChartProZoomSignature> = ({
         zoom: {
           ...prevState.zoom,
           isInteracting: true,
-          zoomData: [...paramsZoomData],
+          zoomData: paramsZoomData,
         },
       };
     });
@@ -100,7 +100,7 @@ export const useChartProZoom: ChartPlugin<UseChartProZoomSignature> = ({
   );
 
   const setZoomDataCallback = React.useCallback(
-    (zoomData: ZoomData[] | ((prev: ZoomData[]) => ZoomData[])) => {
+    (zoomData: ZoomData[] | ((prev: readonly ZoomData[]) => ZoomData[])) => {
       store.update((prevState) => {
         const newZoomData =
           typeof zoomData === 'function' ? zoomData(prevState.zoom.zoomData) : zoomData;
@@ -114,7 +114,7 @@ export const useChartProZoom: ChartPlugin<UseChartProZoomSignature> = ({
           ...prevState,
           zoom: {
             ...prevState.zoom,
-            zoomData: [...newZoomData],
+            zoomData: newZoomData,
           },
         };
       });
@@ -445,14 +445,14 @@ useChartProZoom.getInitialState = (params) => {
   };
   return {
     zoom: {
-      zoomData: [
+      zoomData:
         // eslint-disable-next-line no-nested-ternary
-        ...(zoomData !== undefined
+        zoomData !== undefined
           ? zoomData
           : initialZoom !== undefined
             ? initialZoom
-            : initializeZoomData(optionsLookup)),
-      ],
+            : initializeZoomData(optionsLookup),
+
       isInteracting: false,
       isControlled: zoomData !== undefined,
     },
diff --git a/packages/x-charts-pro/src/internals/plugins/useChartProZoom/useChartProZoom.types.ts b/packages/x-charts-pro/src/internals/plugins/useChartProZoom/useChartProZoom.types.ts
index 2bd74e320..3f7d8ee2b 100644
--- a/packages/x-charts-pro/src/internals/plugins/useChartProZoom/useChartProZoom.types.ts
+++ b/packages/x-charts-pro/src/internals/plugins/useChartProZoom/useChartProZoom.types.ts
@@ -17,7 +17,7 @@ export interface UseChartProZoomParameters {
    *
    * @param {ZoomData[]} zoomData Updated zoom data.
    */
-  onZoomChange?: (zoomData: ZoomData[] | ((zoomData: ZoomData[]) => ZoomData[])) => void;
+  onZoomChange?: (zoomData: ZoomData[] | ((zoomData: readonly ZoomData[]) => ZoomData[])) => void;
   /**
    * The list of zoom data related to each axis.
    */
@@ -37,7 +37,7 @@ export interface UseChartProZoomState {
     /**
      * Mapping of axis id to the zoom data.
      */
-    zoomData: ZoomData[];
+    zoomData: readonly ZoomData[];
     /**
      * Internal information to know if the user control the state or not.
      */
@@ -51,7 +51,7 @@ export interface UseChartProZoomPublicApi {
    * @param {ZoomData[] | ((prev: ZoomData[]) => ZoomData[])} value  The new value. Can either be the new zoom data, or an updater function.
    * @returns {void}
    */
-  setZoomData: (value: ZoomData[] | ((prev: ZoomData[]) => ZoomData[])) => void;
+  setZoomData: (value: ZoomData[] | ((prev: readonly ZoomData[]) => ZoomData[])) => void;
 }
 
 export interface UseChartProZoomInstance extends UseChartProZoomPublicApi {}
diff --git a/packages/x-charts/src/internals/plugins/featurePlugins/useChartCartesianAxis/useChartCartesianAxis.types.ts b/packages/x-charts/src/internals/plugins/featurePlugins/useChartCartesianAxis/useChartCartesianAxis.types.ts
index b4a746582..73ab1eeff 100644
--- a/packages/x-charts/src/internals/plugins/featurePlugins/useChartCartesianAxis/useChartCartesianAxis.types.ts
+++ b/packages/x-charts/src/internals/plugins/featurePlugins/useChartCartesianAxis/useChartCartesianAxis.types.ts
@@ -85,7 +85,7 @@ export interface UseChartCartesianAxisState {
    */
   zoom?: {
     isInteracting: boolean;
-    zoomData: ZoomData[];
+    zoomData: readonly ZoomData[];
   };
   cartesianAxis: {
     x: AxisConfig<ScaleName, any, ChartsXAxisProps>[];

[JCQuintas]

This would cause a breaking changes for controlled users since we would be changing the signature of the setZoomData and onZoomChange to a stricter one.

[JCQuintas]

Current implementation is definitely creating issues though 😆

[alexfauquette]

This would cause a breaking changes for controlled users since we would be changing the signature of the setZoomData and onZoomChange to a stricter one.

Sounds like a fix. zoomData should not be mutated because otherwise it will not trigger the useEffect that update the store.

By the way the `` signature feels weird

- onZoomChange?: (zoomData: ZoomData[] | ((zoomData: ZoomData[]) => ZoomData[])) => void;
+ onZoomChange?: (zoomData: ZoomData[]) => void;

[bernardobelchior]

Shouldn't zoomData here also be readonly? We probably don't want users pushing to this array, right?

[JCQuintas]

Technically, yes, but I didn't want to introduce a breaking change with this 😩

[bernardobelchior]

Makes sense 👍

Isn't now the best time to do it? Or is the performance hit preferable to causing a breaking change? Still trying to understand where we should place the line.

[JCQuintas]

Right now there is no performance hit, I've removed the destructuring changes by making the internal state also immutable.

[bernardobelchior]

Sorry, I should have linked the line I was referring to.

I was referring to line 106 on useChartProZoom. We're now creating a new array where we weren't before.

[JCQuintas]

Ah, true, but that specific case shouldn't really affect us much. zoomData is not meant to be a super long array.