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

Bug: useMemo with Suspense #20877

Closed
guaijie opened this issue Feb 25, 2021 · 6 comments
Closed

Bug: useMemo with Suspense #20877

guaijie opened this issue Feb 25, 2021 · 6 comments
Labels
Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug

Comments

@guaijie
Copy link

guaijie commented Feb 25, 2021

React version: 17.0.1

Steps To Reproduce

  1. useing useMemo cache a function cacheFn
  2. calling cacheFn require resource
  3. using Suspense fetching resource

Link to code example: codesandbox

The current behavior

const UserInfo: FunctionComponent<{ id: number }> = ({ id }) => {
  const fetch = useMemo(() => suspenseFetch(fetchUser, id), [id]);
  const user: User = fetch();
  return (
    <div>
      <h1>{user.name}</h1>
    </div>
  );
};

'useMemo' is called repeatedly when when 'fetch' is called

The expected behavior

'useMemo' is called only when the dependencies changed

@guaijie guaijie added the Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug label Feb 25, 2021
@guaijie guaijie changed the title Bug: Bug: useMemo with Suspense Feb 25, 2021
@vkurchatkin
Copy link

In order of Suspense to work you have to store resources externally, i.e. you can't create a resource and put it into a ref or state.

This is even more true for useMemo. Quoting the docs:

You may rely on useMemo as a performance optimization, not as a semantic guarantee

Meaning that if your code doesn't work for some reason, sticking useMemo is never a valid solution. It should only be used to make code that already works work faster.

@gaearon
Copy link
Collaborator

gaearon commented Feb 25, 2021

Indeed, Suspense is meant to only be backed by a cache. Component state or useMemo is not enough. See #17526 (comment) for more detailed requirements.

@gaearon gaearon closed this as completed Feb 25, 2021
@guaijie
Copy link
Author

guaijie commented Feb 26, 2021

Indeed, Suspense is meant to only be backed by a cache. Component state or useMemo is not enough. See #17526 (comment) for more detailed requirements.

It is reasonable and logical for me to do so
I don't think there's anything wrong with me doing that.
In fact, I submitted it as a bug in the hope that this writing would be implemented

@guaijie
Copy link
Author

guaijie commented Feb 26, 2021

In order of Suspense to work you have to store resources externally, i.e. you can't create a resource and put it into a ref or state.

This is even more true for useMemo. Quoting the docs:

You may rely on useMemo as a performance optimization, not as a semantic guarantee

Meaning that if your code doesn't work for some reason, sticking useMemo is never a valid solution. It should only be used to make code that already works work faster.

I know, but it is also reasonable and logical for me to use UseMemo or UseCallback to cache resources.
That's why I brought it up as a bug, right.

@guaijie
Copy link
Author

guaijie commented Feb 26, 2021

Indeed, Suspense is meant to only be backed by a cache. Component state or useMemo is not enough. See #17526 (comment) for more detailed requirements.

Maybe you should provide a hook so that I can store the resource in the component instead of having to store it in the parent component and pass it to the child component as a prop

@shengxingwang
Copy link

Can you provide a hook to implement the above

Indeed, Suspense is meant to only be backed by a cache. Component state or useMemo is not enough. See #17526 (comment) for more detailed requirements.

Can you provide a hook to implement the above

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug
Projects
None yet
Development

No branches or pull requests

4 participants