search

purescript-react / purescript-react-basic-hooks

An implementation of React hooks on top of purescript-react-basic
https://pursuit.purescript.org/packages/purescript-react-basic-hooks/
Apache License 2.0
200 stars 33 forks source link

Hooks with lazy loading #53

Closed mikesol closed 3 years ago

mikesol commented 3 years ago

The current signature for useState is polymorphic over any state:

useState ::
  forall state.
  state ->
  Hook (UseState state) (state /\ ((state -> state) -> Effect Unit))

However, if the state is a function in react, it tries to lazily evaluate the function: https://reactjs.org/docs/hooks-reference.html#lazy-initial-state. Should this case be encapsulated somehow in the library?

megamaddu commented 3 years ago

Good point, this is probably a bug right now! I'll take a look at it soon

megamaddu commented 3 years ago

Fixed in 7.0.1

mikesol commented 3 years ago

Thanks! I checked out the fix and it looks great, but I'm not sure if it preserves the lazy loading mechanism. Would it be possible to represent this as a fundep from Lazy x to x and otherwise from x to x?

megamaddu commented 3 years ago

Correct. I didn't think it made sense to complicate the types and docs with a third version of useState when you can currently achieve the same thing with useMemo:

initialState <- useMemo unit \_ -> slowInitState props
state /\ setState <- useState initialState

I'm not entirely opposed though, if that doesn't work for some reason. Actually, if I were building it from scratch today I'd probably just force the lazy useState always, but that's an unnecessarily large breaking change now, probably.

mikesol commented 3 years ago

Awesome, makes total sense and thanks for the helpful explanation!

On Thu, 25 Nov 2021, 9.21 Madeline Trotter, @.***> wrote:

Correct. I didn't think it made sense to complicate the types and docs with a third version of useState when you can currently achieve the same thing with useMemo:

initialState <- useMemo unit _ -> slowInitState props state /\ setState <- useState initialState

I'm not entirely opposed though, if that doesn't work for some reason. Actually, if I were building it from scratch today I'd probably just force the lazy useState always, but that's an unnecessarily large breaking change now, probably.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/spicydonuts/purescript-react-basic-hooks/issues/53#issuecomment-978901917, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEAIJWVYTR3JNLST2IBPFTUNXPXZANCNFSM5IKL7KHQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

megamaddu commented 3 years ago

Re: Lazy fundeps, that would most likely be a breaking API change, so probably not worth it unless we're already making v8 for some other reason. Is that right? My fundep knowledge is rusty.

mikesol commented 3 years ago

Yup, it would be a breaking change in a few ways. Polymorphic type signatures would break as they'd need the constraint, and anyone using a Lazy value before would see a different behavior.

On Thu, 25 Nov 2021, 9.25 Madeline Trotter, @.***> wrote:

Re: Lazy fundeps, that would most likely be a breaking API change, so probably not worth it unless we're already making v8 for some other reason. Is that right? My fundep knowledge is rusty.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/spicydonuts/purescript-react-basic-hooks/issues/53#issuecomment-978903902, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAEAIJT7CTHVVGEDMSQDEB3UNXQE5ANCNFSM5IKL7KHQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.