Skip to content

Commit faed6ae

Browse files
committed
Warn for Child Iterator of all types but allow Generator Components (#28853)
This doesn't change production behavior. We always render Iterables to our best effort in prod even if they're Iterators. But this does change the DEV warnings which indicates which are valid patterns to use. It's a footgun to use an Iterator as a prop when you pass between components because if an intermediate component rerenders without its parent, React won't be able to iterate it again to reconcile and any mappers won't be able to re-apply. This is actually typically not a problem when passed only to React host components but as a pattern it's a problem for composability. We used to warn only for Generators - i.e. Iterators returned from Generator functions. This adds a warning for Iterators created by other means too (e.g. Flight or the native Iterator utils). The heuristic is to check whether the Iterator is the same as the Iterable because that means it's not possible to get new iterators out of it. This case used to just yield non-sense like empty sets in DEV but not in prod. However, a new realization is that when the Component itself is a Generator Function, it's not actually a problem. That's because the React Element itself works as an Iterable since we can ask for new generators by calling the function again. So this adds a special case to allow the Generator returned from a Generator Function's direct child. The principle is “don’t pass iterators around” but in this case there is no iterator floating around because it’s between React and the JS VM. Also see #28849 for context on AsyncIterables. Related to this, but Hooks should ideally be banned in these for the same reason they're banned in Async Functions. DiffTrain build for commit 3682021.
1 parent cc4c9de commit faed6ae

File tree

13 files changed

+155
-158
lines changed

13 files changed

+155
-158
lines changed

compiled-rn/facebook-fbsource/xplat/js/RKJSModules/vendor/react-test-renderer/cjs/ReactTestRenderer-dev.js

Lines changed: 31 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* @noflow
88
* @nolint
99
* @preventMunge
10-
* @generated SignedSource<<8514bdf3885188125eab2e51d9ecb536>>
10+
* @generated SignedSource<<fbc5f0d556063e18e8065897a4edae21>>
1111
*/
1212

1313
'use strict';
@@ -6141,45 +6141,36 @@ function createChildReconciler(shouldTrackSideEffects) {
61416141
throw new Error('An object is not an iterable. This error is likely caused by a bug in ' + 'React. Please file an issue.');
61426142
}
61436143

6144-
{
6145-
// We don't support rendering Generators because it's a mutation.
6146-
// See https://github.com/facebook/react/issues/12995
6147-
if (typeof Symbol === 'function' && // $FlowFixMe[prop-missing] Flow doesn't know about toStringTag
6148-
newChildrenIterable[Symbol.toStringTag] === 'Generator') {
6149-
if (!didWarnAboutGenerators) {
6150-
error('Using Generators as children is unsupported and will likely yield ' + 'unexpected results because enumerating a generator mutates it. ' + 'You may convert it to an array with `Array.from()` or the ' + '`[...spread]` operator before rendering. Keep in mind ' + 'you might need to polyfill these features for older browsers.');
6151-
}
6152-
6153-
didWarnAboutGenerators = true;
6154-
} // Warn about using Maps as children
6144+
var newChildren = iteratorFn.call(newChildrenIterable);
61556145

6146+
{
6147+
if (newChildren === newChildrenIterable) {
6148+
// We don't support rendering Generators as props because it's a mutation.
6149+
// See https://github.com/facebook/react/issues/12995
6150+
// We do support generators if they were created by a GeneratorFunction component
6151+
// as its direct child since we can recreate those by rerendering the component
6152+
// as needed.
6153+
var isGeneratorComponent = returnFiber.tag === FunctionComponent && // $FlowFixMe[method-unbinding]
6154+
Object.prototype.toString.call(returnFiber.type) === '[object GeneratorFunction]' && // $FlowFixMe[method-unbinding]
6155+
Object.prototype.toString.call(newChildren) === '[object Generator]';
6156+
6157+
if (!isGeneratorComponent) {
6158+
if (!didWarnAboutGenerators) {
6159+
error('Using Iterators as children is unsupported and will likely yield ' + 'unexpected results because enumerating a generator mutates it. ' + 'You may convert it to an array with `Array.from()` or the ' + '`[...spread]` operator before rendering. You can also use an ' + 'Iterable that can iterate multiple times over the same items.');
6160+
}
61566161

6157-
if (newChildrenIterable.entries === iteratorFn) {
6162+
didWarnAboutGenerators = true;
6163+
}
6164+
} else if (newChildrenIterable.entries === iteratorFn) {
6165+
// Warn about using Maps as children
61586166
if (!didWarnAboutMaps) {
61596167
error('Using Maps as children is not supported. ' + 'Use an array of keyed ReactElements instead.');
6160-
}
6161-
6162-
didWarnAboutMaps = true;
6163-
} // First, validate keys.
6164-
// We'll get a different iterator later for the main pass.
61656168

6166-
6167-
var _newChildren = iteratorFn.call(newChildrenIterable);
6168-
6169-
if (_newChildren) {
6170-
var knownKeys = null;
6171-
6172-
var _step = _newChildren.next();
6173-
6174-
for (; !_step.done; _step = _newChildren.next()) {
6175-
var child = _step.value;
6176-
knownKeys = warnOnInvalidKey(child, knownKeys, returnFiber);
6169+
didWarnAboutMaps = true;
61776170
}
61786171
}
61796172
}
61806173

6181-
var newChildren = iteratorFn.call(newChildrenIterable);
6182-
61836174
if (newChildren == null) {
61846175
throw new Error('An iterable object provided no iterator.');
61856176
}
@@ -6190,9 +6181,14 @@ function createChildReconciler(shouldTrackSideEffects) {
61906181
var lastPlacedIndex = 0;
61916182
var newIdx = 0;
61926183
var nextOldFiber = null;
6184+
var knownKeys = null;
61936185
var step = newChildren.next();
61946186

6195-
for (; oldFiber !== null && !step.done; newIdx++, step = newChildren.next()) {
6187+
{
6188+
knownKeys = warnOnInvalidKey(step.value, knownKeys, returnFiber);
6189+
}
6190+
6191+
for (; oldFiber !== null && !step.done; newIdx++, step = newChildren.next(), knownKeys = warnOnInvalidKey(step.value, knownKeys, returnFiber) ) {
61966192
if (oldFiber.index > newIdx) {
61976193
nextOldFiber = oldFiber;
61986194
oldFiber = null;
@@ -6249,7 +6245,7 @@ function createChildReconciler(shouldTrackSideEffects) {
62496245
if (oldFiber === null) {
62506246
// If we don't have any more existing children we can choose a fast path
62516247
// since the rest will all be insertions.
6252-
for (; !step.done; newIdx++, step = newChildren.next()) {
6248+
for (; !step.done; newIdx++, step = newChildren.next(), knownKeys = warnOnInvalidKey(step.value, knownKeys, returnFiber) ) {
62536249
var _newFiber3 = createChild(returnFiber, step.value, lanes, debugInfo);
62546250

62556251
if (_newFiber3 === null) {
@@ -6274,7 +6270,7 @@ function createChildReconciler(shouldTrackSideEffects) {
62746270

62756271
var existingChildren = mapRemainingChildren(oldFiber); // Keep scanning and use the map to restore deleted items as moves.
62766272

6277-
for (; !step.done; newIdx++, step = newChildren.next()) {
6273+
for (; !step.done; newIdx++, step = newChildren.next(), knownKeys = warnOnInvalidKey(step.value, knownKeys, returnFiber) ) {
62786274
var _newFiber4 = updateFromMap(existingChildren, returnFiber, newIdx, step.value, lanes, debugInfo);
62796275

62806276
if (_newFiber4 !== null) {
@@ -22990,7 +22986,7 @@ identifierPrefix, onUncaughtError, onCaughtError, onRecoverableError, transition
2299022986
return root;
2299122987
}
2299222988

22993-
var ReactVersion = '19.0.0-canary-66405eaa';
22989+
var ReactVersion = '19.0.0-canary-c4e3bea9';
2299422990

2299522991
/*
2299622992
* The `'' + value` pattern (used in perf-sensitive code) throws for Symbol

compiled-rn/facebook-fbsource/xplat/js/RKJSModules/vendor/react-test-renderer/cjs/ReactTestRenderer-prod.js

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* @noflow
88
* @nolint
99
* @preventMunge
10-
* @generated SignedSource<<f3eb1768c908f25292e8f079ec672f53>>
10+
* @generated SignedSource<<8230647a80c4b86b8cf8384768883557>>
1111
*/
1212

1313
"use strict";
@@ -1825,7 +1825,7 @@ function createChildReconciler(shouldTrackSideEffects) {
18251825
nextOldFiber = null,
18261826
step = newChildrenIterable.next();
18271827
null !== oldFiber && !step.done;
1828-
newIdx++, step = newChildrenIterable.next()
1828+
newIdx++, step = newChildrenIterable.next(), null
18291829
) {
18301830
oldFiber.index > newIdx
18311831
? ((nextOldFiber = oldFiber), (oldFiber = null))
@@ -1849,7 +1849,7 @@ function createChildReconciler(shouldTrackSideEffects) {
18491849
if (step.done)
18501850
return deleteRemainingChildren(returnFiber, oldFiber), iteratorFn;
18511851
if (null === oldFiber) {
1852-
for (; !step.done; newIdx++, step = newChildrenIterable.next())
1852+
for (; !step.done; newIdx++, step = newChildrenIterable.next(), null)
18531853
(step = createChild(returnFiber, step.value, lanes)),
18541854
null !== step &&
18551855
((currentFirstChild = placeChild(step, currentFirstChild, newIdx)),
@@ -1862,7 +1862,7 @@ function createChildReconciler(shouldTrackSideEffects) {
18621862
for (
18631863
oldFiber = mapRemainingChildren(oldFiber);
18641864
!step.done;
1865-
newIdx++, step = newChildrenIterable.next()
1865+
newIdx++, step = newChildrenIterable.next(), null
18661866
)
18671867
(step = updateFromMap(oldFiber, returnFiber, newIdx, step.value, lanes)),
18681868
null !== step &&
@@ -9144,7 +9144,7 @@ var devToolsConfig$jscomp$inline_1019 = {
91449144
throw Error("TestRenderer does not support findFiberByHostInstance()");
91459145
},
91469146
bundleType: 0,
9147-
version: "19.0.0-canary-7a48fce4",
9147+
version: "19.0.0-canary-a8200c0a",
91489148
rendererPackageName: "react-test-renderer"
91499149
};
91509150
var internals$jscomp$inline_1238 = {
@@ -9175,7 +9175,7 @@ var internals$jscomp$inline_1238 = {
91759175
scheduleRoot: null,
91769176
setRefreshHandler: null,
91779177
getCurrentFiber: null,
9178-
reconcilerVersion: "19.0.0-canary-7a48fce4"
9178+
reconcilerVersion: "19.0.0-canary-a8200c0a"
91799179
};
91809180
if ("undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__) {
91819181
var hook$jscomp$inline_1239 = __REACT_DEVTOOLS_GLOBAL_HOOK__;

compiled-rn/facebook-fbsource/xplat/js/RKJSModules/vendor/react-test-renderer/cjs/ReactTestRenderer-profiling.js

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* @noflow
88
* @nolint
99
* @preventMunge
10-
* @generated SignedSource<<9206ca0f6781a4c840d6d393f26b3429>>
10+
* @generated SignedSource<<9f8b5b89043962a79b6b2123a526bf46>>
1111
*/
1212

1313
"use strict";
@@ -1913,7 +1913,7 @@ function createChildReconciler(shouldTrackSideEffects) {
19131913
nextOldFiber = null,
19141914
step = newChildrenIterable.next();
19151915
null !== oldFiber && !step.done;
1916-
newIdx++, step = newChildrenIterable.next()
1916+
newIdx++, step = newChildrenIterable.next(), null
19171917
) {
19181918
oldFiber.index > newIdx
19191919
? ((nextOldFiber = oldFiber), (oldFiber = null))
@@ -1937,7 +1937,7 @@ function createChildReconciler(shouldTrackSideEffects) {
19371937
if (step.done)
19381938
return deleteRemainingChildren(returnFiber, oldFiber), iteratorFn;
19391939
if (null === oldFiber) {
1940-
for (; !step.done; newIdx++, step = newChildrenIterable.next())
1940+
for (; !step.done; newIdx++, step = newChildrenIterable.next(), null)
19411941
(step = createChild(returnFiber, step.value, lanes)),
19421942
null !== step &&
19431943
((currentFirstChild = placeChild(step, currentFirstChild, newIdx)),
@@ -1950,7 +1950,7 @@ function createChildReconciler(shouldTrackSideEffects) {
19501950
for (
19511951
oldFiber = mapRemainingChildren(oldFiber);
19521952
!step.done;
1953-
newIdx++, step = newChildrenIterable.next()
1953+
newIdx++, step = newChildrenIterable.next(), null
19541954
)
19551955
(step = updateFromMap(oldFiber, returnFiber, newIdx, step.value, lanes)),
19561956
null !== step &&
@@ -9760,7 +9760,7 @@ var devToolsConfig$jscomp$inline_1101 = {
97609760
throw Error("TestRenderer does not support findFiberByHostInstance()");
97619761
},
97629762
bundleType: 0,
9763-
version: "19.0.0-canary-39a6938a",
9763+
version: "19.0.0-canary-359342b1",
97649764
rendererPackageName: "react-test-renderer"
97659765
};
97669766
(function (internals) {
@@ -9804,7 +9804,7 @@ var devToolsConfig$jscomp$inline_1101 = {
98049804
scheduleRoot: null,
98059805
setRefreshHandler: null,
98069806
getCurrentFiber: null,
9807-
reconcilerVersion: "19.0.0-canary-39a6938a"
9807+
reconcilerVersion: "19.0.0-canary-359342b1"
98089808
});
98099809
exports._Scheduler = Scheduler;
98109810
exports.act = act;

compiled-rn/facebook-fbsource/xplat/js/RKJSModules/vendor/react/cjs/JSXDEVRuntime-dev.js

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* @noflow
88
* @nolint
99
* @preventMunge
10-
* @generated SignedSource<<8f878889d9995d2ae240b302c52058bd>>
10+
* @generated SignedSource<<b7764b0ac63cb81257aa2e03fa2024ca>>
1111
*/
1212

1313
'use strict';
@@ -1304,11 +1304,14 @@ function validateChildKeys(node, parentType) {
13041304
// but now we print a separate warning for them later.
13051305
if (iteratorFn !== node.entries) {
13061306
var iterator = iteratorFn.call(node);
1307-
var step;
13081307

1309-
while (!(step = iterator.next()).done) {
1310-
if (isValidElement(step.value)) {
1311-
validateExplicitKey(step.value, parentType);
1308+
if (iterator !== node) {
1309+
var step;
1310+
1311+
while (!(step = iterator.next()).done) {
1312+
if (isValidElement(step.value)) {
1313+
validateExplicitKey(step.value, parentType);
1314+
}
13121315
}
13131316
}
13141317
}

compiled-rn/facebook-fbsource/xplat/js/RKJSModules/vendor/react/cjs/JSXRuntime-dev.js

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* @noflow
88
* @nolint
99
* @preventMunge
10-
* @generated SignedSource<<63334719117bc7e81815150b27588000>>
10+
* @generated SignedSource<<6786f79c50baa1d713a06b4ed12afb60>>
1111
*/
1212

1313
'use strict';
@@ -1328,11 +1328,14 @@ function validateChildKeys(node, parentType) {
13281328
// but now we print a separate warning for them later.
13291329
if (iteratorFn !== node.entries) {
13301330
var iterator = iteratorFn.call(node);
1331-
var step;
13321331

1333-
while (!(step = iterator.next()).done) {
1334-
if (isValidElement(step.value)) {
1335-
validateExplicitKey(step.value, parentType);
1332+
if (iterator !== node) {
1333+
var step;
1334+
1335+
while (!(step = iterator.next()).done) {
1336+
if (isValidElement(step.value)) {
1337+
validateExplicitKey(step.value, parentType);
1338+
}
13361339
}
13371340
}
13381341
}

compiled-rn/facebook-fbsource/xplat/js/RKJSModules/vendor/react/cjs/React-dev.js

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* @noflow
88
* @nolint
99
* @preventMunge
10-
* @generated SignedSource<<1ccd58cd787da0b40cbf5543503953db>>
10+
* @generated SignedSource<<745c28cba28ff3a4a3a391cfdfdd1c1b>>
1111
*/
1212

1313
'use strict';
@@ -27,7 +27,7 @@ if (
2727
}
2828
var dynamicFlagsUntyped = require('ReactNativeInternalFeatureFlags');
2929

30-
var ReactVersion = '19.0.0-canary-c627bf44';
30+
var ReactVersion = '19.0.0-canary-e8808e1d';
3131

3232
// ATTENTION
3333
// When adding new symbols to this file,
@@ -1889,11 +1889,14 @@ function validateChildKeys(node, parentType) {
18891889
// but now we print a separate warning for them later.
18901890
if (iteratorFn !== node.entries) {
18911891
var iterator = iteratorFn.call(node);
1892-
var step;
18931892

1894-
while (!(step = iterator.next()).done) {
1895-
if (isValidElement(step.value)) {
1896-
validateExplicitKey(step.value, parentType);
1893+
if (iterator !== node) {
1894+
var step;
1895+
1896+
while (!(step = iterator.next()).done) {
1897+
if (isValidElement(step.value)) {
1898+
validateExplicitKey(step.value, parentType);
1899+
}
18971900
}
18981901
}
18991902
}
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
ea26e38e33bffeba1ecc42688d7e8cd7e0da1c02
1+
368202181e772d411b2445930aea1edd9428b09b

0 commit comments

Comments
 (0)