First level of cascading memoization is broken when more props are passed

[misha-erm]

Hello,

Not sure it's an issue but want to highlight this small problem we faced recently. So imagine you have two selectors:

const getSchema = createSelector([(state) => state.rawSchema], (rawSchema) => {...})

const getItem = createSelector([getSchema, (state, itemId) => itemId], (schema, itemId) => {...})

as a result whenever getItem is called it also calls getSchema with arguments: state and itemId which leads to invalidation of args memoization. Usually overhead might be unnoticeable because second level of caching still works but for some 'hot' selectors we see some significant numbers.

Here is an example for one of our hottest selectors after reloading the page. And every dispatch generates few thousands of dependency recomputations

Right now I can see 3 ways how it can be fixed in userland:

1. just inline selector everywhere to make sure it's always called with required params. Leaves responsibility on consumer
2. add some utility like const acceptStateOnly = (selector) => (state) => selector(state) and wrap such hot selectors at place where they are defined
3. customize argsMemoize to care about first argument only

I don't really like them as they are pretty fragile so I wonder if it's something that can be improved on library side, e.g. by comparing length of incoming arguments with length of saved arguments or by stricter types or by advanced input stability check

Thanks in advance 🙇🏻

P.S. also the issue is so noticeable for us because we used to pass props as object to selectors like useSelector((state) => selector(state, {prop1, prop2})) which basically leads to args cache invalidation on every react render

[markerikson]

That sounds like things are working as expected:

• All arguments get passed to all input selectors
• Any new reference causes the reference comparisons to fail and that level of memoization to be recalculated

So yes, passing a new object reference to a selector is not a good idea. Have you tried passing them as separate arguments instead? selector(state, prop1, prop2).

[misha-erm]

Have you tried passing them as separate arguments instead? selector(state, prop1, prop2).

That's what we are migrating to right now. But this still doesn't solve the original issue. I understand why it works like that but many devs at first were a bit lost when I told them about first level of cache cascade and that prop selectors ideally should be written like

createSelector([(state) => getSchema(state), (state) => getSmthFromState(state), (state, prop1) => prop1], () => {})

instead of passing input selectors directly

createSelector([getSchema, getSmthFromState, (state, prop1) => prop1], () => {})

So what do you think, is it something that should be handled by the library? E.g. one more input dev check, note in docs or a solution?

[markerikson]

I don't think there's anything we should change here.

Relying on function.length is always tricky, and it's not something I'd want to try to make use of in this case.

I think your best bet is to customize argsMemoize as needed for your situation.

[misha-erm]

Got it, thanks for your opinion 🙇🏻

Closing the issue then