Add support for Fibers in ReactDOMComponentTree and ReactTreeTraversal #8232
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
sophiebits
approved these changes
Nov 16, 2016
| } else if (typeof newProps.dangerouslySetInnerHTML === 'object' && | ||
| newProps.dangerouslySetInnerHTML !== null && | ||
| typeof newProps.dangerouslySetInnerHTML.__html === 'string') { | ||
| domElement.innerHTML = newProps.dangerouslySetInnerHTML.__html; |
There was a problem hiding this comment.
Does switching away from dangerouslySetInnerHTML work?
There was a problem hiding this comment.
No this is broken in more than one way. We have an issue on it in the Umbrella.
| * should control radio buttons | ||
| * should control radio buttons if the tree updates during render | ||
| * should warn with value and no onChange handler and readOnly specified | ||
| * should have a this value of undefined if bind is not used |
There was a problem hiding this comment.
why does this fail? sounds like something that should pass
There was a problem hiding this comment.
I think the error happens now because the event fires, so the expect does, but not yet sure why it isn't dispatched correctly.
This adds precaching to ReactDOMFiber. I.e. adding a handle from the DOM node to the internal Fiber. This means that we need to expose an internal handle to the reconciler. We use duck typing to figure out if it is a Fiber or Stack instance. The new failing tests are failing because this is now able to actually fire events onto Fibers and then the result of those events are incorrect where as they were ignored before.
This traverses parent based on the type of internal instance it is passed. If it is a Fiber it may have to traverse multiple steps until it finds a HostComponent. This will allow us to use the event system with Fiber.
sebmarkbage
force-pushed
the
treetraversal
branch
from
942591f to
b3af02a
Compare
This makes some basic events work with Fiber. This is however not a complete solution since we may be reading the wrong Fiber.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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 adds precaching to ReactDOMFiber. I.e. adding a handle from the DOM node to the internal Fiber. This means that we need to expose an internal handle to the reconciler.
We use duck typing inside ReactDOMComponentTree and ReactTreeTraversal to figure out if it is a Fiber or Stack instance.
The new failing tests are failing because this is now able to actually fire events onto Fibers and then the result of those events are incorrect where as they were ignored before.
In ReactTreeTraversal we traverse parent based on the type of internal instance it is passed. We could probably optimize this by doing the detection outside each loop and duplicate the loops.
If the parent is a Fiber it may have to traverse multiple steps until it finds a HostComponent since we don't have pointers directly to the next host container.
This will allow us to use the event system with Fiber.