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

Use Callback #54

Open
codingedgar opened this issue Dec 5, 2021 · 7 comments
Open

Use Callback #54

codingedgar opened this issue Dec 5, 2021 · 7 comments

Comments

@codingedgar
Copy link

Hi! Thanks a lot for this amazing package ✨

I cannot find an example of React.useCallback I see it was deprecated on what seems to be v5.x what is the correct way to "useCallback" in a hook?

Sorry if it is a silly question I'm learning purescript

@pete-murphy
Copy link
Member

Seems like you could just use useMemo?

  fn' <- Hooks.useMemo deps \_ -> fn

would be the same as a hypothetical

  fn' <- Hooks.useCallback deps fn

right?

@codingedgar
Copy link
Author

Wow idk why the issue description was cut.

What i meant to say was: i see useCallback existed until v5 and now it’s gone, why? Is there a better way to do useCallback in PureScript?

@codingedgar
Copy link
Author

Seems like you could just use useMemo?

  fn' <- Hooks.useMemo deps \_ -> fn

would be the same as a hypothetical

  fn' <- Hooks.useCallback deps fn

right?

It seems about right, but the same applies in regular js, yet useCallback is exposed, why remove it? Coming from Reactjs is a bit weird not to have the same API.

I guess it's because it is "unnecessary" or something similar in PureScript?

@megamaddu
Copy link
Member

Yeah, it's a combination of me naming some functions badly in previous versions plus it being redundant. Leaving it in would have caused confusing errors for those upgrading. I'm not opposed to adding it now, but also.. it's literally useMemo with the wrapper fn applied for you 😅

@codingedgar
Copy link
Author

I was wondering more if the whole pattern was unnecessary or something, meaning declaring functions inside the body of a component does not cause React to re-render sub-components diffing a new function all the time, etc, but if it is the same case then having useCallback seems nice, it's something pretty well understood coming from reactjs to react-basic-hooks.

Yeah, it's a combination of me naming some functions badly in previous versions plus it being redundant. Leaving it in would have caused confusing errors for those upgrading. I'm not opposed to adding it now, but also.. it's literally useMemo with the wrapper fn applied for you 😅

That's fine, considering PS commonality of having a bunch of ergonomic signatures (like fn, fn_, fn') I don't see why it would be bad to have useCallback as a shorthand, but I'm so noob in PS that maybe useCallback is not as widely used/necessary in PS as it is in JS?

But it's fine, I was mostly wondering about the former, I would "vote" for having it, but if it is trouble or confusing in PS, that's fine too, no need to make a change just for that.

@megamaddu
Copy link
Member

That's true, I don't mind adding it.

For a little more background, I think PS devs are less likely to be thinking about reference equality at all, especially for functions, and just eating that performance cost until it actually becomes a problem. On the flip side, it's become fairly expected in JS-land and the more you see it the more you keep using it. In most cases it's a premature optimization. You can reach for these tools in PS if you need them (memo, UnsafeReference, useMemo, and [if added] useCallback), but it isn't as common.

@andys8
Copy link
Member

andys8 commented May 10, 2022

It would be great if there would be useCallback: It is also documented in the react ecosystem and people might look for it. On the other hand it is helpful for people who don't know and are wondering why such a function exists and what problem it solves.

What option would you prefer @megamaddu ?

Adding useCallback implemented in terms of useMemo.

useCallback :: forall deps a. Eq deps => deps -> a -> Hook (UseMemo deps a) a
useCallback deps fn = useMemo deps (const fn)

or actually calling useCallback from the react library and adding a UseCallback hook.

Even though react docs say it's equivalent to, react team could decide to change internals of both functions or remove one of them in a breaking change in the future. And I personally tend to just exposing these functions from the react library and would go with the latter option (calling useCallback from the react library).

image

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

4 participants