MediaReplaceFlow: Avoid React warning when selecting media #34618
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
Mamaduka
added
[Package] Block editor
|
Size Change: +2.34 kB (0%) Total Size: 1.05 MB
ℹ️ View Unchanged
|
ellatrix
reviewed
Sep 7, 2021
| setMediaURLValue( media.url ); | ||
| onSelect( media ); |
There was a problem hiding this comment.
Could you leave an inline comment? In what case does it unmount the component?
There was a problem hiding this comment.
I think that depends on the parent component and what it does inside the onSelect callback.
The "Post Featured Image" edit component will re-render it when new media data is fetched:
gutenberg/packages/block-library/src/post-featured-image/edit.js
Lines 122 to 133 in 7791d58
There was a problem hiding this comment.
@ellatrix, the cause of the unmounting can be different in each case, and I don't think the inline comment would be enough to provide the necessary information.
There was a problem hiding this comment.
Surely there could be a comment explaining why this order is needed? This change is just really easy to revert without getting an error for the automated tests, so it seems like a good place to comment. Also, why are we not getting an error in the automated tests? :)
There was a problem hiding this comment.
Added inline comment in 55806ce.
Would you please let me know if you think that wording needs changing?
There was a problem hiding this comment.
Hi, @ellatrix
I just wanted to check if it's okay to merge this fix after adding the inline comment.
Thanks
7 tasks
gziolo
approved these changes
Sep 17, 2021
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
The component was calling
setMediaURLValueafter calling theonSelectcallback, which in some cases can unmount the component.Initially reported in #34506
How has this been tested?
DevTools console shouldn't display a "Can't perform a React state update on an unmounted component" warning.
Types of changes
Bugfix
Checklist:
*.native.jsfiles for terms that need renaming or removal).