useBlockSync: Fix race condition when onChange callback changes #24012
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
noahtallen
added
[Type] Bug
|
Size Change: +13 B (0%) Total Size: 1.15 MB
ℹ️ View Unchanged
|
epiqueras
reviewed
Jul 17, 2020
packages/block-editor/src/components/provider/test/use-block-sync.js
Outdated
Show resolved
Hide resolved
|
😬 ayy sorry for the miss. |
noahtallen
force-pushed
the
fix/use-block-sync-race-condition-on-change
branch
from
881c7c2
to
a530aaf
Compare
Addison-Stavlo
approved these changes
Jul 20, 2020
There was a problem hiding this comment.
Awesome! I can confirm this fixes #23967. The changes make sense to me, onChange/onInput should no longer become outdated in the subscribed function using these refs. And thank you for adding the test! 😄
(small nit Q on the test but this looks good!)
youknowriad
added
the
Backport to WP 6.6 Beta/RC
youknowriad
pushed a commit
that referenced
this pull request
Jul 27, 2020
* Fix race condition when onChange callback changed * Add tests to cover the scenario
youknowriad
removed
the
Backport to WP 6.6 Beta/RC
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
In the
useBlockSynchook, if theonChangeoronInputcallbacks changed while the same component instance was being used, the oldonChange/onInputcallbacks were used instead of the updated ones.For more context, in #23292, we started setting a ref with the latest onChange/onInput callbacks, but we unfortunately never consumed that ref. cc @youknowriad. This updates the block-editor subscription such that it uses the
onChangeandonInputrefs. This way, it always uses the latest versions of the callbacks.Specifically, this fixes #23967 (a result of the race condition).
How has this been tested?
Locally, in edit-site.
Screenshots
Before:
After:
Types of changes
Bug fix.
Checklist: