Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Customizing key used for memoization #11

Closed
jedwards1211 opened this issue Jan 20, 2018 · 4 comments
Closed

Customizing key used for memoization #11

jedwards1211 opened this issue Jan 20, 2018 · 4 comments

Comments

@jedwards1211
Copy link

jedwards1211 commented Jan 20, 2018

The array/list selectors in this package have a problem: if you splice in some elements at index n, you lose all the memoization for everything that came after n.

Edit: now that I glanced at the code I see you have an optional undocumented equalityCheck argument. In a lot of cases it's probably more straightforward to just use a function that determines the cache key for a given element.

But often it's common to have an array of objects containing a property that can act as a key:

[
  {id: 1, name: 'Andy'},
  {id: 2, name: 'John'},
  {id: 3, name: 'Martha'},
  ...
]

In this case if I could just provide a function that tells createArraySelector to use element.id as the key, then the results for each id would stay memoized no matter how the array was shuffled.

One way to do this would be to provide a keyBy option with the signature (value, originalKey) => newKey:

createArraySelector(
  state => state.users,
  state => state.multiplier,
  (user, multiplier) => user.age * multiplier,
  {keyBy: user => user.id}
)
@heyimalex
Copy link
Owner

In a lot of cases it's probably more straightforward to just use a function that determines the cache key for a given element.

Eh, it's kind of nuanced. I don't have any usage stats to support this, but I imagine that the common case is that the objects in the array are immutable. When that's the case, keying the elements doesn't achieve much, since most elements stay referentially equal across calls.

Not that I'm shutting this down, just discussing. Here's a proof of concept that I think would work.

export const keyedArrayMemoize = (fn, {equalityCheck, keyBy}) =>
  memoizeMap(fn, {
    unique: true,
    equalityCheck,
    mapper(rows, callback) {
      return rows.map(v => callback(keyBy(v), v));
    }
});

@jedwards1211
Copy link
Author

Yeah that's a good point.

I came to this library from code I had written to manually instantiate and apply a rather complex selector to each value in an array, and I get some benefit from applying the previous selector for a given key if the value for that key changed but none of its contents relevant to the selector changed (assuming a way to create a custom selector for each key were provided by this library).

However, after converting that selector to an unmemoized function and using it with reselect-map, I don't seem to have any performance problems, so it's debatable whether it's worth the extra complexity in most cases.

@heyimalex
Copy link
Owner

Yeah, composition with caching is hard in general. I'll leave this issue open in case anyone else is interested in a similar thing, but probably won't move forward since the current code handles the common case well enough.

@heyimalex
Copy link
Owner

Actually I like zero open issues ahahahahahahahahahahahaha

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants