New Equality function type signature breaks with a lot of equality functions

[ehtb]

Thanks for the great library! Just a heads up and something I think worth considering; adding the index to the equality callback causes a lot of commonly used equality functions to break.

For instance the most popular one, shallowEqual has as a third parameter a custom comparison function: shallowequal(value, other, [customizer], [thisArg]). Same is true for the lodash deep comparison function.

Of course it is not a huge problem to change this in our code, but it breaks a lot of existing code like this: memoizeOne(params, shallowEqual)

[alexreardon]

Oh snap. Perhaps we should role this back. Thoughts?

…

On Sun, 16 Dec 2018 at 9:04 pm, Edo ***@***.***> wrote: Thanks for the great library! Just a heads up and something I think worth considering; adding the index to the equality callback causes a lot of commonly used equality functions to break. For instance the most popular one, shallowEqual has as a third parameter a custom comparison function: shallowequal(value, other, [customizer], [thisArg]). Same is true for the lodash deep comparison function. Of course it is not a huge problem to change this in our code, but perhaps something to consider since it breaks a lot of existing code like this: memoizeOne(params, shallowEqual) — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#47>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACFN7T1r-SSS0X8bXYckICSRnYMia8dsks5u5hrJgaJpZM4ZVH1o> .

[alexreardon]

I was unsure about this one but I had no data to back up my unease

…

On Sun, 16 Dec 2018 at 9:34 pm, Alex Reardon ***@***.***> wrote: Oh snap. Perhaps we should role this back. Thoughts? On Sun, 16 Dec 2018 at 9:04 pm, Edo ***@***.***> wrote: > Thanks for the great library! Just a heads up and something I think worth > considering; adding the index to the equality callback causes a lot of > commonly used equality functions to break. > > For instance the most popular one, shallowEqual has as a third parameter > a custom comparison function: shallowequal(value, other, [customizer], > [thisArg]). Same is true for the lodash deep comparison function. > > Of course it is not a huge problem to change this in our code, but > perhaps something to consider since it breaks a lot of existing code like > this: memoizeOne(params, shallowEqual) > > — > You are receiving this because you are subscribed to this thread. > Reply to this email directly, view it on GitHub > <#47>, or mute the > thread > <https://github.com/notifications/unsubscribe-auth/ACFN7T1r-SSS0X8bXYckICSRnYMia8dsks5u5hrJgaJpZM4ZVH1o> > . >

[alexreardon]

Based on your feedback I think I will role it back. I’ll have more of a think about how to move forward

…

On Sun, 16 Dec 2018 at 9:35 pm, Alex Reardon ***@***.***> wrote: I was unsure about this one but I had no data to back up my unease On Sun, 16 Dec 2018 at 9:34 pm, Alex Reardon ***@***.***> wrote: > Oh snap. Perhaps we should role this back. Thoughts? > On Sun, 16 Dec 2018 at 9:04 pm, Edo ***@***.***> wrote: > >> Thanks for the great library! Just a heads up and something I think >> worth considering; adding the index to the equality callback causes a lot >> of commonly used equality functions to break. >> >> For instance the most popular one, shallowEqual has as a third >> parameter a custom comparison function: shallowequal(value, other, >> [customizer], [thisArg]). Same is true for the lodash deep comparison >> function. >> >> Of course it is not a huge problem to change this in our code, but >> perhaps something to consider since it breaks a lot of existing code like >> this: memoizeOne(params, shallowEqual) >> >> — >> You are receiving this because you are subscribed to this thread. >> Reply to this email directly, view it on GitHub >> <#47>, or mute the >> thread >> <https://github.com/notifications/unsubscribe-auth/ACFN7T1r-SSS0X8bXYckICSRnYMia8dsks5u5hrJgaJpZM4ZVH1o> >> . >> >

[ehtb]

Thanks for the fast reply, makes sense yes.

[nihgwu]

From my perspective, a library should not be aware of the implementation of other libraries

The HOC idea is awesome, but it complicates the usability IMO, I agree we should roll back #42 as it breaks current usage suggested by the README