[core] Replace reselect with local memoizers in x-internals#22721
[core] Replace reselect with local memoizers in x-internals#22721sai6855 wants to merge 2 commits into
Conversation
Replace the reselect runtime dependency with small local implementations of the two memoizers actually used: - lruMemoize: least-recently-used cache (maxSize 1 by default) with configurable equalityCheck and resultEqualityCheck, matching the reselect API subset used by the data grid and the store. - weakMapMemoize: argument-keyed cache tree holding object arguments weakly, used to memoize selectors on their arguments. createSelector keeps the exact same memoization strategy as before: the combiner is memoized on its input values (latest call, Object.is), the selector itself is memoized on its arguments through the weak cache, and resultEqualityCheck overrides keep working for the chart tooltip selectors. reselect stays in package.json as it is still used for type-only imports. This removes ~8kB minified (~0.8-1kB gzip) from every bundle that includes the x-internals store: all data grid, charts and tree view packages. Combined full-barrel gzip size drops by ~6.8kB as measured with pnpm size:snapshot.
sai6855
added
scope: data grid
sai6855
changed the title
Deploy previewhttps://deploy-preview-22721--material-ui-x.netlify.app/ Bundle size
Check out the code infra dashboard for more information about this PR. |
|
@romgrk Before I make this PR ready for review, I'd like to check the direction with you. The idea: @mui/x-internals only uses a small subset of reselect (lruMemoize with maxSize: 1 and the default weakMapMemoize for args). This PR replaces that subset with ~150 lines of local code with the same memoization semantics — combiner memoized on input values (Object.is, latest call), selector memoized on its arguments through a WeakMap cache tree, resultEqualityCheck still supported for the chart tooltip selectors. Public API of createSelector/createSelectorMemoized is unchanged and the existing grid/charts/tree-view test suites pass. Result: one less runtime dependency, and ~0.8–1kB gzip off every package that bundles the store (data grid, charts, tree view — all tiers), ~6.9kB total measured with pnpm size:snapshot. Does this seem like a direction worth taking? |
|
I'm in favor of this change, but duplicated version of the store code exist here and in the Base UI repository. I'm trying to merge those versions so whatever changes we do here, we also need to do on the BUI repository. I'd open a parallel PR to validate that the changes pass tests on both sides before merging anything.
Ideally we'd get rid of it completely. |
There was a problem hiding this comment.
One thing I like about reselect is that it has a fast version of lruMemoize optimized for maxSize: 1, which is our default (and maybe only) value for that option. I guess we'd lose some of the size benefits by doing the same here, I'd be curious to know how much it costs.
See https://github.com/reduxjs/reselect/blob/master/src/lruMemoize.ts
Great
Got it, let me refine PR and do changes in both repos
I'll check |
There was a problem hiding this comment.
Result: one less runtime dependency, and ~0.8–1kB gzip off every package that bundles the store (data grid, charts, tree view — all tiers)
Although considering the popularity of Redux Toolkit, some of our users might see a net loss because they'd be shipping both our code and reselect's code.
2 participants
Replace the reselect runtime dependency with small local implementations of the two memoizers actually used:
createSelector keeps the exact same memoization strategy as before: the combiner is memoized on its input values (latest call, Object.is), the selector itself is memoized on its arguments through the weak cache, and resultEqualityCheck overrides keep working for the chart tooltip selectors.
reselect stays in package.json as it is still used for type-only imports.
This removes ~8kB minified (~0.8-1kB gzip) from every bundle that includes the x-internals store: all data grid, charts and tree view packages. Combined full-barrel gzip size drops by ~6.8kB as measured with pnpm size:snapshot.