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

unbreak LiveState unsub when references.size === 0 #4832

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

subtleGradient
Copy link

fixes #4830 by ensuring that there is only a single path for evicting stuff, ensuring that maybeResolverSubscription gets called correctly

@captbaritone
Copy link
Contributor

Thank you for this. Great catch. Would you mind adding a test case here: https://github.com/facebook/relay/blob/f4c37a333575341b78979106084068cb6b27e1cf/packages/relay-runtime/store/__tests__/resolvers/ResolverGC-test.js so that we don't regress this again?

@subtleGradient
Copy link
Author

@captbaritone added the test.
I couldn't figure out how to get the __generated__ stuff to build,
so I reused an existing fragment name, not sure if that matters or not

manually verified that the test fails before the fix and passes after the fix

@subtleGradient
Copy link
Author

bummed this didn't make it into 18.2
What still needs to happen before this can land? @captbaritone

@captbaritone
Copy link
Contributor

Sorry for the poor communication. We are in a code freeze currently so not landing any high risk changes that are not necessary, so I wanted to wait to land this one. I'm out next week, but can try to get this landed when I return.

@facebook-github-bot
Copy link
Contributor

@captbaritone has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

@live LiveState unsubscribe cleanup function never gets called
3 participants