[React Fresh] Return host nodes for visual feedback #15688
Closed
Conversation
This file contains hidden or 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
|
ReactDOM: size: 0.0%, gzip: -0.0% Details of bundled changes.Comparing: 50b50c2...5b6c10e react-dom
react-art
react-native-renderer
react-test-renderer
react-reconciler
Generated by 🚫 dangerJS |
We will return an array of host nodes that are related to the hot reloaded Fibers. The hot reloading implementation can use it to "flash" updated parts of the screen visually. The array only includes outermost nodes in the updated subtrees. This both helps reduce the overhead from finding host nodes, and avoids overloading the visual feedback. Usually you just want to see the updated area and not every little thing inside of it. This commit breaks tests. It's because it searches for host nodes that might have been unmounted by the hot update. I will fix this in follow-up commits.
This ensures that when we edit <div> to null, we highlight parent div instead.
This tracks unmounted fibers during a hot reload pass, and temporarily remembers their parent chain. Even though they get disconnected, this lets us find their closest mounted fibers and search for the visual feedback host node from there.
|
Did you consider using an effect tag? Then you can mutate during the actual commit phase, and you don't need an extra traversal. |
|
This approach is flawed because the visual feedback should happen before the reload is finished. Then it makes sense. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
This PR adds a return value to
scheduleHotUpdate:This is an array of host nodes which can be used by the hot reloading runtime to show visual feedback related to the edited UI. Here's a gif to give you an idea of a possible way to use this. Note how the updated components are briefly highlighted in the browser right after edit:
(Don't pay too much attention to the concrete visual feedback here; this is something we'll play more with. If this contextual feedback ends up being too much, we can always remove it and use a simple indicator, but I want to experiment more with it.)
The implementation goes as follows:
ParentandChildwere edited in the same file, we'll only mark theParent. This is because in the visual indication we'll likely want to coalesce children with parents anyway.Alternatives
No Component-Specific Visual Feedback
Maybe this is overkill and we just want an indicator in the screen corner or something. Then we can delete this code. I'm not sure and I wanted to play more with this.
Move It Into Consuming Runtime
We could just return
affectedFibersand call it a day. Then the consuming code could find the host nodes by copy pasting a similar implementation. That's actually how I started this. However, it's annoying to keep in sync with work tags. And the implementation is a bit tricky (e.g. tracking deletions). So if this is the workflow we want, we'd probably need to centralize it. Why not put it with the rest of the logic we're already testing then? So I put it here.