[DataGrid] Fix memoized selectors with arguments#15215
[DataGrid] Fix memoized selectors with arguments#15215MBilalShafi merged 12 commits intomui:masterfrom
Conversation
MBilalShafi
added
internal
|
Deploy preview: https://deploy-preview-15215--material-ui-x.netlify.app/ |
MBilalShafi
added
needs cherry-pick
kenanyusuf
left a comment
kenanyusuf
left a comment
There was a problem hiding this comment.
Tested on my working branch using memoized v8 selectors and it is working as expected 👍
kenanyusuf
left a comment
kenanyusuf
left a comment
There was a problem hiding this comment.
@MBilalShafi Spotted an issue with the types for selectors created using createSelectorMemoizedV8:
When calling the selector directly, it seems to require an args arg even if additional args are not used by the selector.
Selector:
export const gridColumnDefinitionsSelectorV8 = createSelectorMemoizedV8(
gridColumnFieldsSelectorV8,
gridColumnLookupSelectorV8,
(allFields, lookup) => allFields.map((field) => lookup[field]),
);(ignore all the selectors being suffixed with v8, it's for testing purposes)
MBilalShafi
requested review from
kenanyusuf and
romgrk
and removed request for
romgrk
kenanyusuf
left a comment
kenanyusuf
left a comment
There was a problem hiding this comment.
Not sure what's going on with Argos 🤔 but the changes LGTM.
Yep this is a flaky one, I didn't dig deep yet though it seems to be somehow related to #14130 |
| '@typescript-eslint/no-unused-vars': [ | ||
| 'error', | ||
| { vars: 'all', args: 'after-used', ignoreRestSiblings: true, argsIgnorePattern: '^_' }, | ||
| ], |
There was a problem hiding this comment.
This change extends the eslint-config-airbnb-typescript's rule previously in action by adding an argsIgnorePattern for allowing _ based unused variables ignored by eslint parser.
It might be useful to add it to the core .eslintrc.js and extend from there.
CC @mui/x @mui/code-infra
There was a problem hiding this comment.
👍 feel free to open a PR to add this to the Core config.
|
Cherry-pick PRs will be created targeting branches: v7.x |
5 participants
As reported by @kenanyusuf, memoized selectors with arguments weren't working properly, this PR aims to fix that.
Extracted from #15200 to separate from the BC.