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

Make disposing unmounted routes more robust #50

Merged
merged 2 commits into from
Jun 28, 2022
Merged

Conversation

zth
Copy link
Owner

@zth zth commented Jun 27, 2022

Closes #38
Closes #39

For #39, I found nothing of value in my investigation. Will continue investigating and re-open if needed.

In short, one feature of the router is that it will "dispose" any route that is not rendered anymore. This means the router will tell the Relay store that any queries used by the current route is no longer active, and can be GCed if appropriate. The feature itself is especially important for larger apps where the Relay store otherwise can become quite fat and memory intensive as you browse around.

Previously, this disposing was done via a regular useEffect that would trigger when a route render function was unmounted. However, a render function unmounting doesn't necessarily mean that the route has unmounted. For example, it could be temporarily unmounted because something suspended.

This PR changes the mechanism that disposes unmounted routes from using a regular useEffect to actually pushing an event from the router when a route unmounts, and then checking by computed routeKey (which is unique per route + params) what's supposed to be disposed.

@zth zth requested a review from Kingdutch June 27, 2022 19:34
Copy link
Collaborator

@Kingdutch Kingdutch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a great addition :D

router/RelayRouter__Internal__DeclarationsSupport.res Outdated Show resolved Hide resolved
Co-authored-by: Alexander Varwijk <Kingdutch@users.noreply.github.com>
@zth zth merged commit 0387d38 into main Jun 28, 2022
@zth zth deleted the more-robust-disposing branch June 28, 2022 06:24
Kingdutch added a commit that referenced this pull request Jun 28, 2022
This reverts commit b5c312f, reversing
changes made to c8c0f27.

This removes changes to the plugin and does not yet add the route
preLoading code. It leaves in place those things that were later used
for work done in PRs #49 and #50.
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

Successfully merging this pull request may close these issues.

Router dedupe prepare blocks Relay store invalidations? Router queries disposed early?
2 participants