search

mweststrate / use-st8

Single-function alternative for React.useState
MIT License
234 stars 7 forks source link

Return value is not referentially stable #13

Closed merisbahti closed 3 years ago

merisbahti commented 3 years ago

The return value here: https://github.com/mweststrate/use-st8/blob/743172aff4a97eb63e311a9985484948236db373/src/index.ts#L17

Is not referentially stable.

Should it be wrapped in a memo perhaps?

mweststrate commented 3 years ago

yeah looks like it! not memo but useState or a ref, otherwise it is still not stable :)

On Fri, Feb 12, 2021 at 5:33 PM Meris Bahtijaragic notifications@github.com wrote:

The return value here:

https://github.com/mweststrate/use-st8/blob/743172aff4a97eb63e311a9985484948236db373/src/index.ts#L17

Is not referentially stable.

Should it be wrapped in a memo perhaps?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/mweststrate/use-st8/issues/13, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAN4NBCH7ABFE3MXF7S6KHTS6VRAJANCNFSM4XRBZFCQ .

mweststrate commented 3 years ago

Not guaranteed I mean

On Fri, Feb 12, 2021 at 5:48 PM Michel Weststrate mweststrate@gmail.com wrote:

yeah looks like it! not memo but useState or a ref, otherwise it is still not stable :)

On Fri, Feb 12, 2021 at 5:33 PM Meris Bahtijaragic < notifications@github.com> wrote:

The return value here:

https://github.com/mweststrate/use-st8/blob/743172aff4a97eb63e311a9985484948236db373/src/index.ts#L17

Is not referentially stable.

Should it be wrapped in a memo perhaps?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/mweststrate/use-st8/issues/13, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAN4NBCH7ABFE3MXF7S6KHTS6VRAJANCNFSM4XRBZFCQ .

merisbahti commented 3 years ago

So how would the ref solution work to make it stable? I'm curious.

Would we put the states current value in the ref, then in the function return the refs current value?

Wonder if that's CM compatible!

merisbahti commented 3 years ago

I just realised that if this value was referentially stable, it would cause bugs.

imagine the simple scenario where we have a Parent, that initializes state const st8 = useSt8(5), and a child which takes the st8 as a prop.

If the st8 value is updated (via parent or child) and the child is wrapped in React.memo - the child would opt out from rendering, because its st8 prop is still the same reference.

Closing this as that would break this lib :)