[charts] Let the useXxxSeries support array of ids and document them

[JCQuintas]

• added docs to all public hooks
• Improve types of useXxxSeries hooks and add more use-cases

[mui-bot]

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

Generated by 🚫 dangerJS against 07ae309

[codspeed-hq]

CodSpeed Performance Report

Merging #15545 will not alter performance

_{Comparing JCQuintas:remove-unstable-useseries (07ae309) with master (e71e831)}

Summary

✅ 6 untouched benchmarks

[alexfauquette]

I'm wondering if the current hook is the best DX. If we provide something like

useBarSeries(['seriesId1', 'seriesId2']) // Returns the config of the series
useBarSeries() // Returns all bar series in the correct order

It might be easier to use

[JCQuintas]

Do you see a lot of value in returning an array for allowing multiple items to be returned or only allow one return per call?

const a = useSeries('a')
const b = useSeries('b')

vs

const [a,b] useSeries('a', 'b')

[alexfauquette]

The array has the advantage to be easier to understand and to allow adding extra arguments later. But I'm not sure if passing an array like useBarSeries(['seriesId1', 'seriesId2']) could cause perf issue because at each render it would creat a new array 🤔

Maybe we should wait for the end of the plugin migration to see if one DX allows easier perf otpimisation than the other

[JCQuintas]

The array has the advantage to be easier to understand and to allow adding extra arguments later. But I'm not sure if passing an array like useBarSeries(['seriesId1', 'seriesId2']) could cause perf issue because at each render it would creat a new array 🤔

Maybe we should wait for the end of the plugin migration to see if one DX allows easier perf otpimisation than the other

Optimisation-wise we can just ...args into the dep array. Though we have to disable the eslint error for exaustive deps 🙃

[github-actions]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[alexfauquette]

I think that's how we standardize the docs naming for x/y axes

Suggested change

	* Get all the X axes.
	* Get all the x-axes.

[alexfauquette]

Did you consider using selectors directly for the meoization?

export const selectorScatterSeries = createSelector(
  [selectorChartSeriesProcessed],
  (series) => series.scatter)
);


export const selectorScatterSeriesWithIds = createSelector(
  [
    selectorScatterSeries,
    (_, ids: SeriesId | SeriesId[]
  ],
  (scatterSeries, ids) => {
    if(typeof ids === 'object'){
        return ids.map(id => scatterSeries.series[id] ?? null)
    }
    return scatterSeries.series[ids] ?? null
  }
);

Then

useScatterSeries(ids?:  SeriesId | SeriesId[]){
  const store = useStore()

  return useSelector(store, ids===undefined ? selectorScatterSeries : selectorScatterSeriesWithIds, ids);
}

As long as the ids stay the same, you should benefit from selector memoization

[JCQuintas]

It was quite hard to create a place where I could mock the return values, so in the end I'm testing them with the real chart components 😓

[alexfauquette]

I've allowed my self to update your PR title

[JCQuintas]

I've allowed my self to update your PR title

Good, because it said pubic hooks before 😳

[JCQuintas]

@alexfauquette tried to simplify the logic and keep it in a single place, so we can use createSeriesSelectorsOfType('pie'), etc on the actual hooks.

[alexfauquette]

I'm not 100% sure, but your idea is to call something like

useScatterSeries('id1', 'id2')

But then the following lines will transform 'id1', 'id2' into ['id1', 'id2'] and so generate a new array which break the memoization.

export function useScatterSeries(...seriesIds: SeriesId[]): any {
  return selectorSeries(seriesIds);
}

For me the only way to let user (or react compiler) memorize the ids is to pass directly an array

[github-actions]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[JCQuintas]

Should we divide the logic for useAllScatterSeries/useScatterSeriesAll from the rest? The return values are wildly different 🤔

[JCQuintas]

I would add it to another PR though 😅

[alexfauquette]

I agree. Finding a good name for this hook is the main difficulty. Maybe useScatterSeriesContext or useScatterSeriesState even if it does not exactly translate what it is

[alexfauquette]

🎉