[Fiber] Support iterables #8446
Merged
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
Stack currently supports rendering iterables, but Fiber does not. Previously we didn't have any public API tests for iterables. We have tests for traverseAllChildren() which is shared between React.Children and Stack. However Fiber doesn't currently use it, and likely won't. So this commit is a first step towards actually testing iterable support via public API. The next step will be to port traverseAllChildren() tests to test React.Children API instead.
This uses the same exact algorithm as array reconciliation but uses iterator to step through. This gets reconcile tests to pass again but introduces a regression in ReactMultiChildText case which uses Maps as children. It passed before because Maps were ignored, but now it's failing because this actually runs the Map code path in Fiber. We can throw early in this case when we want to follow up on this.
gaearon
force-pushed
the
iterable-tests
branch
from
2b24fa9 to
71eefd0
Compare
This function was used in React.Children and Stack. The corresponding reconciliation functionality is being tested by ReactMultiChild tests. So we can move these tests to ReactChildren and test its public API.
sebmarkbage
reviewed
Nov 28, 2016
| @@ -919,10 +1050,14 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) { | |||
|
|
|||
| const iteratorFn = getIteratorFn(newChild); | |||
| if (iteratorFn) { | |||
| const iterator = iteratorFn.call(newChild); | |||
There was a problem hiding this comment.
Why do this here instead of inside reconcileChildrenIterator? Flow?
There was a problem hiding this comment.
We'd have to either get iterator function again inside of it, or pass it along. Both seemed a bit silly.
sebmarkbage
approved these changes
Nov 28, 2016
acusti
pushed a commit
to brandcast/react
that referenced
this pull request
Mar 15, 2017
* Add iterable cases to MultiChildReconcile test Stack currently supports rendering iterables, but Fiber does not. Previously we didn't have any public API tests for iterables. We have tests for traverseAllChildren() which is shared between React.Children and Stack. However Fiber doesn't currently use it, and likely won't. So this commit is a first step towards actually testing iterable support via public API. The next step will be to port traverseAllChildren() tests to test React.Children API instead. * Implement iterable reconciliation in Fiber This uses the same exact algorithm as array reconciliation but uses iterator to step through. This gets reconcile tests to pass again but introduces a regression in ReactMultiChildText case which uses Maps as children. It passed before because Maps were ignored, but now it's failing because this actually runs the Map code path in Fiber. We can throw early in this case when we want to follow up on this. * Rewrite traverseAllChildren() tests against React.Children API This function was used in React.Children and Stack. The corresponding reconciliation functionality is being tested by ReactMultiChild tests. So we can move these tests to ReactChildren and test its public API.
laurinenas
pushed a commit
to laurinenas/react
that referenced
this pull request
May 28, 2018
* Add iterable cases to MultiChildReconcile test Stack currently supports rendering iterables, but Fiber does not. Previously we didn't have any public API tests for iterables. We have tests for traverseAllChildren() which is shared between React.Children and Stack. However Fiber doesn't currently use it, and likely won't. So this commit is a first step towards actually testing iterable support via public API. The next step will be to port traverseAllChildren() tests to test React.Children API instead. * Implement iterable reconciliation in Fiber This uses the same exact algorithm as array reconciliation but uses iterator to step through. This gets reconcile tests to pass again but introduces a regression in ReactMultiChildText case which uses Maps as children. It passed before because Maps were ignored, but now it's failing because this actually runs the Map code path in Fiber. We can throw early in this case when we want to follow up on this. * Rewrite traverseAllChildren() tests against React.Children API This function was used in React.Children and Stack. The corresponding reconciliation functionality is being tested by ReactMultiChild tests. So we can move these tests to ReactChildren and test its public API.
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.
Add iterable cases to MultiChildReconcile test 0fa8116
Stack currently supports rendering iterables, but Fiber does not.
Previously we didn't have any public API tests for iterables. We have tests for traverseAllChildren() which is shared between React.Children and Stack. However Fiber doesn't currently use it, and likely won't. So this commit is a first step towards actually testing iterable support via public API. The next step will be to port traverseAllChildren() tests to test React.Children API instead.
Implement iterable reconciliation in Fiber 71eefd0
This uses the same exact algorithm as array reconciliation but uses iterator to step through.
This gets reconcile tests to pass again but introduces a regression in ReactMultiChildText case which uses Maps as children. It passed before because Maps were ignored, but now it's failing because this actually runs the Map code path in Fiber. We can throw early in this case when we want to follow up on this.
Rewrite traverseAllChildren() tests against React.Children API 2b7e0a0
This function was used in React.Children and Stack.
The corresponding reconciliation functionality is being tested by ReactMultiChild tests.
So we can move these tests to ReactChildren and test its public API.