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

Call forceUpdate immediately if diff changes between first useFragment call and first cache.watch call #10212

Merged
merged 3 commits into from
Oct 20, 2022

Conversation

benjamn
Copy link
Member

@benjamn benjamn commented Oct 19, 2022

This useFragment test has been failing locally for me and has even failed on main for otherwise innocuous commits like 2db905f (failing CI job).

I added this Promise logic in #10118 to fix another flakiness issue, but the current state seems worse (more frequent failures) than before.

@benjamn benjamn self-assigned this Oct 19, 2022
@benjamn
Copy link
Member Author

benjamn commented Oct 19, 2022

Alas, that test failure is the same one I was trying to fix in #10118, so I guess we're back to where we started.

@benjamn benjamn marked this pull request as draft October 19, 2022 19:03
@benjamn benjamn force-pushed the attempt-to-fix-flaky-useFragment-test branch from 2b12820 to 8c40bd1 Compare October 19, 2022 19:51
@benjamn benjamn changed the title Attempt to fix flaky useFragment test by removing needless Promise logic Call forceUpdate immediately if diff changes between first useFragment call and first cache.watch Oct 19, 2022
@benjamn benjamn changed the title Call forceUpdate immediately if diff changes between first useFragment call and first cache.watch Call forceUpdate immediately if diff changes between first useFragment call and first cache.watch call Oct 19, 2022
@benjamn benjamn marked this pull request as ready for review October 19, 2022 19:55
@benjamn benjamn added this to the 3.7.x patch releases milestone Oct 19, 2022
This useFragment test has been failing locally for me and has even
failed on main for otherwise innocuous commits like
2db905f.

I added this Promise logic in #10118 to fix another flakiness issue, but
the current state seems worse (more frequent failures) than before.
Because useSyncExternalStore does not call the subscribe function until
an internal useEffect fires, there's a real chance the latest data from
the cache have changed in the meantime, so it's not useless to check for
changes right away and potentially call forceUpdate.

Passing immediate:true to cache.watch ensures the callback will be
called right away, rather than the _next_ time relevant data change.
@benjamn benjamn force-pushed the attempt-to-fix-flaky-useFragment-test branch from 8c40bd1 to b964d7b Compare October 20, 2022 16:04
@benjamn benjamn merged commit 81bc6b4 into main Oct 20, 2022
@benjamn benjamn deleted the attempt-to-fix-flaky-useFragment-test branch October 20, 2022 16:25
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants