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

Add array index as a final argument to createArraySelector combiner #20

Open
pbadenski opened this issue Sep 15, 2020 · 4 comments · May be fixed by #21
Open

Add array index as a final argument to createArraySelector combiner #20

pbadenski opened this issue Sep 15, 2020 · 4 comments · May be fixed by #21

Comments

@pbadenski
Copy link

pbadenski commented Sep 15, 2020

In order to make this possible:

const mul = createArraySelector(
  numbers,
  (number, index) => [index, number]
);

This is similar to what's already possible with keys for maps and objects.

@pbadenski pbadenski changed the title Include array index as a final argument to createArraySelector callback Add array index as a final argument to createArraySelector combiner Sep 15, 2020
@heyimalex
Copy link
Owner

heyimalex commented Sep 15, 2020

This was intentional; memoization is bound to a "key" and array index doesn't make sense as a key because re-orders trigger length - index total changes. Changing this would opt everyone into that behavior, since it makes the mapping function dependent on the index and the mapping functions need to be pure. Some more discussion about this choice here.

@pbadenski
Copy link
Author

Makes sense. Although it does optimise towards reselect-map in computation use case (which what this library was created for, so that makes sense 😆). I'm plugging in reselect-map for inputArray -> outputArray transformation to ensure output array only changes at particular indices where inputArray also changed and other elements remain unchanged. The behaviour with ordering is actually what I would be expecting.

Hope you can help... What would be the easiest way for me to roll out the custom implementation? Would it be possible to expose a bit more of the API (eg. createMappedSelectorCreator and memoizeMap)?

@heyimalex
Copy link
Owner

Yeah, we can definitely expose those. I think just exporting and then adding to the typescript definitions is all that needs to be done if you want to submit a pr, or I can try and tackle it sometime this week.

@pbadenski
Copy link
Author

cool, I'll have a go at it, thanks!

@pbadenski pbadenski linked a pull request Sep 21, 2020 that will close this issue
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

Successfully merging a pull request may close this issue.

2 participants