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

Why will Suspense for data fetching require a cache? #16954

Closed
ghengeveld opened this issue Sep 29, 2019 · 12 comments
Closed

Why will Suspense for data fetching require a cache? #16954

ghengeveld opened this issue Sep 29, 2019 · 12 comments
Labels
Resolution: Stale Automatically closed due to inactivity

Comments

@ghengeveld
Copy link
Contributor

ghengeveld commented Sep 29, 2019

I'm integrating Suspense with React Async, and I've been reading up on how it's supposed to work. All the resources I found talk about needing react-cache or another cache mechanism for Suspense to work properly. However, none of the resources explain WHY Suspense needs a cache. Could someone explain that? I've been able to make it work without a cache. Am I missing something?

@Freak613
Copy link

Freak613 commented Oct 8, 2019

React will destroy local state whenever you throw anything from component:
https://codesandbox.io/s/inspiring-lumiere-70r3z

And caching needs to track your requests outside of destroyed component, somewhere that is stateful and won't be corrupted.

@ghengeveld
Copy link
Contributor Author

Won't useRef work? I'm still struggling to understand. It works fine without one. I've come to see the cache as a means to synchronously resolve the data in order to avoid unnecessary rerenders. But personally I don't really care about that, it feels like a premature optimization.

@robertvansteen
Copy link

If your suspended component unmounts as a result of a higher up in the tree you will lose your local state and refs. That’s why you need the data in something that lives outside your components: the cache.

@stale
Copy link

stale bot commented Jan 9, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution.

@stale stale bot added the Resolution: Stale Automatically closed due to inactivity label Jan 9, 2020
@brainkim
Copy link
Contributor

Would like to see this issue replied to before it’s marked as stale and locked, maybe with an update by the original author with an update for this issue.

@stale stale bot removed the Resolution: Stale Automatically closed due to inactivity label Jan 10, 2020
@stale
Copy link

stale bot commented Apr 9, 2020

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any additional information, please include with in your comment!

@stale stale bot added the Resolution: Stale Automatically closed due to inactivity label Apr 9, 2020
@gaearon
Copy link
Collaborator

gaearon commented Apr 9, 2020

Sorry I didn’t see this issue earlier! This has nothing to do with optimisations.

Suspense requires a cache because when a tree suspends, we want to be able to throw that render attempt away. The just-created components’ state or refs are preserved. (Details may vary based on the situation, but at least it’s not preserved for the components that suspended.) If you keep data inside state or refs, it would have to be above the place that may suspend while mounting. Because if that data/Promise gets thrown away, then you’re going to have an infinite loop of render attempts, each time throwing a different Promise.

In that sense, yes, keeping it in a ref is a form of a “cache”. But this ref would have to be higher than any usage of that data, which presumably is close to the root. So you can’t just put it inside your Hook that suspends. That’s a rather unintuitive but an important limitation.

Another way to look at it is that data has a lifetime associated with it. But rendering has no lifetime. It’s just a render attempt. You need to attach data to something that lives on permanently. That “something” is a cache. It could be local state of a component sufficiently high up the tree. Or it could be an object outside of the component tree. In both cases you then need to decide how and when to invalidate the cache. And how to do this without breaking encapsulation between deep children and the cache owner.

Our current thinking is that this concept of a cache and how it invalidates is tied to the idea of navigation. That broadly speaking, we can navigate “forward” and “back”, and the intent matters. When we navigate forward, we don’t want to show potentially stale cached data from the cache. Instead, we want to refetch. (To enable faster transitions, however, we’ll want to embed data needed by top-level views of the next screen into responses for the previous screen.) When we go back, we want to show data from the cache. Unless there was a mutation, in which case we do refetch the data when it happens.

I know this is rather hand wavy at this point but just wanted to explain where we’re going and how we’re thinking about these problems.

@stale stale bot removed the Resolution: Stale Automatically closed due to inactivity label Apr 9, 2020
@ghengeveld
Copy link
Contributor Author

Thanks for the extensive explanation, this is exactly what I was after. Thanks also for clarifying why a ref typically won't work for this purpose.

@lazeebee
Copy link

@gaearon Can you explain further why react can't implement that the state and refs of the suspended component aren't thrown away?

@jezzgoodwin
Copy link

@gaearon Thanks for the explanation. I wondered if you guys have thought about keeping the state alive for suspended components? It feels like it would make working with Suspense easier.

@stale
Copy link

stale bot commented Sep 20, 2020

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

@stale stale bot added the Resolution: Stale Automatically closed due to inactivity label Sep 20, 2020
@stale
Copy link

stale bot commented Oct 4, 2020

Closing this issue after a prolonged period of inactivity. If this issue is still present in the latest release, please create a new issue with up-to-date information. Thank you!

@stale stale bot closed this as completed Oct 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution: Stale Automatically closed due to inactivity
Projects
None yet
Development

No branches or pull requests

7 participants