[charts] Memoize series series selectors

[JCQuintas]

In the original code I had a function that created the selectors.

The new code uses the arguments directly in the selector, which allows us to simply use the useAllSeriesOfType/useSeriesOfType instead of the generated values in each useSeries type

import { createAllSeriesSelectorOfType } from '../internals/createSeriesSelectorOfType';

const useSelectorSeriesContext = createAllSeriesSelectorOfType('bar');

export function useBarSeriesContext(): UseBarSeriesContextReturnValue {
  return useSelectorSeriesContext();
}

then becomes

import { useAllSeriesOfType } from '../internals/seriesSelectorOfType';

export function useBarSeriesContext(): UseBarSeriesContextReturnValue {
  return useAllSeriesOfType('bar');
}

Realistically this shouldn't affect performance directly, but could improve re-renders for users by memoizing the result of the multi-series output (useSeriesOfType('bar',[1,2,3...]);) when it returns an array.

[mui-bot]

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

Bundle size report

Bundle	Parsed size	Gzip size
@mui/x-data-grid	0B(0.00%)	0B(0.00%)
@mui/x-data-grid-pro	0B(0.00%)	0B(0.00%)
@mui/x-data-grid-premium	0B(0.00%)	0B(0.00%)
@mui/x-charts	▼-133B(-0.04%)	▼-41B(-0.04%)
@mui/x-charts-pro	▼-196B(-0.04%)	▼-52B(-0.04%)
@mui/x-charts-premium	▼-163B(-0.04%)	▼-69B(-0.05%)
@mui/x-date-pickers	0B(0.00%)	0B(0.00%)
@mui/x-date-pickers-pro	0B(0.00%)	0B(0.00%)
@mui/x-tree-view	0B(0.00%)	0B(0.00%)
@mui/x-tree-view-pro	0B(0.00%)	0B(0.00%)

Details of bundle changes

Generated by 🚫 dangerJS against 66637d0

[JCQuintas]

@romgrk would it be possible (realistic) to create "generic" selectors?

This doesn't seem to properly work.

// Ideally the result of
useSelector(store, selectorAllSeriesOfType, 'bar')
// Would be `ChartSeriesDefaultized<'bar'>`
// But is a union of all possible `ChartSeriesDefaultized` variations.

I tried looking at the types, but I guess you would know better before I delve too deep into this 😆

[romgrk]

Possible probably, realistic I don't know, sometimes types become quite complex, and I don't know how to fix this one.

[bernardobelchior]

Could you provide more information? What cases is this memoizing improving? Did you see any meaningful performance improvement?

[JCQuintas]

Could you provide more information? What cases is this memoizing improving? Did you see any meaningful performance improvement?

It was in draft, info should be up now 😆
This is just an improvement on the original

[bernardobelchior]

Looks good to me 👍

Both my comments are suggestions for improvements, but I realized that the code didn't change, so I think it's fine as is and we can consider tackling them as follow-ups.

[bernardobelchior]

I realize this is how it works currently and it would be a breaking change to fix it, but just wanted to start this discussion:

If an empty array is provided shouldn't we return an empty array?

It's a bit weird that if you provide one ID you get one series, but if you provide zero IDs you get all the series.

We already have the optional ids field to mean "return all the series".

[JCQuintas]

added to #17955

[bernardobelchior]

If we don't want this to have a performance impact in production environments we can derive the failed IDs from result and ids inside the if statement.

Likely not a big deal, though

[JCQuintas]

Yeah I would assume this hook is not called with a lot of ids