diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index 50afd73d1f937..afa8a699b795b 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -7913,4 +7913,85 @@ describe('ReactDOMFizzServer', () => { ]); expect(postpones).toEqual([]); }); + + it('should NOT warn for using generator functions as components', async () => { + function* Foo() { + yield

Hello

; + yield

World

; + } + + await act(() => { + const {pipe} = renderToPipeableStream(); + pipe(writable); + }); + + expect(document.body.textContent).toBe('HelloWorld'); + }); + + it('should warn for using generators as children props', async () => { + function* getChildren() { + yield

Hello

; + yield

World

; + } + + function Foo() { + const children = getChildren(); + return
{children}
; + } + + await expect(async () => { + await act(() => { + const {pipe} = renderToPipeableStream(); + pipe(writable); + }); + }).toErrorDev( + '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.\n' + + ' in div (at **)\n' + + ' in Foo (at **)', + ); + + expect(document.body.textContent).toBe('HelloWorld'); + }); + + it('should warn for using other types of iterators as children', async () => { + function Foo() { + let i = 0; + const iterator = { + [Symbol.iterator]() { + return iterator; + }, + next() { + switch (i++) { + case 0: + return {done: false, value:

Hello

}; + case 1: + return {done: false, value:

World

}; + default: + return {done: true, value: undefined}; + } + }, + }; + return iterator; + } + + await expect(async () => { + await act(() => { + const {pipe} = renderToPipeableStream(); + pipe(writable); + }); + }).toErrorDev( + '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.\n' + + ' in Foo (at **)', + ); + + expect(document.body.textContent).toBe('HelloWorld'); + }); }); diff --git a/packages/react-dom/src/__tests__/ReactMultiChild-test.js b/packages/react-dom/src/__tests__/ReactMultiChild-test.js index 96413378f6aef..44d71fe7fc805 100644 --- a/packages/react-dom/src/__tests__/ReactMultiChild-test.js +++ b/packages/react-dom/src/__tests__/ReactMultiChild-test.js @@ -328,12 +328,77 @@ describe('ReactMultiChild', () => { ); }); - it('should warn for using generators as children', async () => { + it('should NOT warn for using generator functions as components', async () => { function* Foo() { yield

Hello

; yield

World

; } + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + await act(async () => { + root.render(); + }); + + expect(container.textContent).toBe('HelloWorld'); + }); + + it('should warn for using generators as children props', async () => { + function* getChildren() { + yield

Hello

; + yield

World

; + } + + function Foo() { + const children = getChildren(); + return
{children}
; + } + + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + await expect(async () => { + await act(async () => { + root.render(); + }); + }).toErrorDev( + '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.\n' + + ' in div (at **)\n' + + ' in Foo (at **)', + ); + + expect(container.textContent).toBe('HelloWorld'); + + // Test de-duplication + await act(async () => { + root.render(); + }); + }); + + it('should warn for using other types of iterators as children', async () => { + function Foo() { + let i = 0; + const iterator = { + [Symbol.iterator]() { + return iterator; + }, + next() { + switch (i++) { + case 0: + return {done: false, value:

Hello

}; + case 1: + return {done: false, value:

World

}; + default: + return {done: true, value: undefined}; + } + }, + }; + return iterator; + } + const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); await expect(async () => { @@ -341,13 +406,16 @@ describe('ReactMultiChild', () => { root.render(); }); }).toErrorDev( - '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.\n' + + '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.\n' + ' in Foo (at **)', ); + expect(container.textContent).toBe('HelloWorld'); + // Test de-duplication await act(async () => { root.render(); diff --git a/packages/react-reconciler/src/ReactChildFiber.js b/packages/react-reconciler/src/ReactChildFiber.js index 742206c47fbf5..ed6c328c1a6d0 100644 --- a/packages/react-reconciler/src/ReactChildFiber.js +++ b/packages/react-reconciler/src/ReactChildFiber.js @@ -33,7 +33,13 @@ import { REACT_LAZY_TYPE, REACT_CONTEXT_TYPE, } from 'shared/ReactSymbols'; -import {HostRoot, HostText, HostPortal, Fragment} from './ReactWorkTags'; +import { + HostRoot, + HostText, + HostPortal, + Fragment, + FunctionComponent, +} from './ReactWorkTags'; import isArray from 'shared/isArray'; import {enableRefAsProp} from 'shared/ReactFeatureFlags'; @@ -1114,52 +1120,46 @@ function createChildReconciler( ); } + const newChildren = iteratorFn.call(newChildrenIterable); + if (__DEV__) { - // We don't support rendering Generators because it's a mutation. - // See https://github.com/facebook/react/issues/12995 - if ( - typeof Symbol === 'function' && - // $FlowFixMe[prop-missing] Flow doesn't know about toStringTag - newChildrenIterable[Symbol.toStringTag] === 'Generator' - ) { - if (!didWarnAboutGenerators) { - console.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.', - ); + if (newChildren === newChildrenIterable) { + // We don't support rendering Generators as props because it's a mutation. + // See https://github.com/facebook/react/issues/12995 + // We do support generators if they were created by a GeneratorFunction component + // as its direct child since we can recreate those by rerendering the component + // as needed. + const isGeneratorComponent = + returnFiber.tag === FunctionComponent && + // $FlowFixMe[method-unbinding] + Object.prototype.toString.call(returnFiber.type) === + '[object GeneratorFunction]' && + // $FlowFixMe[method-unbinding] + Object.prototype.toString.call(newChildren) === '[object Generator]'; + if (!isGeneratorComponent) { + if (!didWarnAboutGenerators) { + console.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.', + ); + } + didWarnAboutGenerators = true; } - didWarnAboutGenerators = true; - } - - // Warn about using Maps as children - if ((newChildrenIterable: any).entries === iteratorFn) { + } else if ((newChildrenIterable: any).entries === iteratorFn) { + // Warn about using Maps as children if (!didWarnAboutMaps) { console.error( 'Using Maps as children is not supported. ' + 'Use an array of keyed ReactElements instead.', ); - } - didWarnAboutMaps = true; - } - - // First, validate keys. - // We'll get a different iterator later for the main pass. - const newChildren = iteratorFn.call(newChildrenIterable); - if (newChildren) { - let knownKeys: Set | null = null; - let step = newChildren.next(); - for (; !step.done; step = newChildren.next()) { - const child = step.value; - knownKeys = warnOnInvalidKey(child, knownKeys, returnFiber); + didWarnAboutMaps = true; } } } - const newChildren = iteratorFn.call(newChildrenIterable); - if (newChildren == null) { throw new Error('An iterable object provided no iterator.'); } @@ -1172,11 +1172,20 @@ function createChildReconciler( let newIdx = 0; let nextOldFiber = null; + let knownKeys: Set | null = null; + let step = newChildren.next(); + if (__DEV__) { + knownKeys = warnOnInvalidKey(step.value, knownKeys, returnFiber); + } for ( ; oldFiber !== null && !step.done; - newIdx++, step = newChildren.next() + newIdx++, + step = newChildren.next(), + knownKeys = __DEV__ + ? warnOnInvalidKey(step.value, knownKeys, returnFiber) + : null ) { if (oldFiber.index > newIdx) { nextOldFiber = oldFiber; @@ -1236,7 +1245,15 @@ function createChildReconciler( if (oldFiber === null) { // If we don't have any more existing children we can choose a fast path // since the rest will all be insertions. - for (; !step.done; newIdx++, step = newChildren.next()) { + for ( + ; + !step.done; + newIdx++, + step = newChildren.next(), + knownKeys = __DEV__ + ? warnOnInvalidKey(step.value, knownKeys, returnFiber) + : null + ) { const newFiber = createChild(returnFiber, step.value, lanes, debugInfo); if (newFiber === null) { continue; @@ -1261,7 +1278,15 @@ function createChildReconciler( const existingChildren = mapRemainingChildren(oldFiber); // Keep scanning and use the map to restore deleted items as moves. - for (; !step.done; newIdx++, step = newChildren.next()) { + for ( + ; + !step.done; + newIdx++, + step = newChildren.next(), + knownKeys = __DEV__ + ? warnOnInvalidKey(step.value, knownKeys, returnFiber) + : null + ) { const newFiber = updateFromMap( existingChildren, returnFiber, diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index 7ba6f334e759a..00ed4d8f06f6d 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -2150,36 +2150,49 @@ function replayElement( // rendered in the prelude and skip it. } -// $FlowFixMe[missing-local-annot] -function validateIterable(iterable, iteratorFn: Function): void { +function validateIterable( + task: Task, + iterable: Iterable, + childIndex: number, + iterator: Iterator, + iteratorFn: () => ?Iterator, +): void { if (__DEV__) { - // We don't support rendering Generators because it's a mutation. - // See https://github.com/facebook/react/issues/12995 - if ( - typeof Symbol === 'function' && - iterable[Symbol.toStringTag] === 'Generator' - ) { - if (!didWarnAboutGenerators) { - console.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.', - ); + if (iterator === iterable) { + // We don't support rendering Generators as props because it's a mutation. + // See https://github.com/facebook/react/issues/12995 + // We do support generators if they were created by a GeneratorFunction component + // as its direct child since we can recreate those by rerendering the component + // as needed. + const isGeneratorComponent = + task.componentStack !== null && + task.componentStack.tag === 1 && // FunctionComponent + // $FlowFixMe[method-unbinding] + Object.prototype.toString.call(task.componentStack.type) === + '[object GeneratorFunction]' && + // $FlowFixMe[method-unbinding] + Object.prototype.toString.call(iterator) === '[object Generator]'; + if (!isGeneratorComponent) { + if (!didWarnAboutGenerators) { + console.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.', + ); + } + didWarnAboutGenerators = true; } - didWarnAboutGenerators = true; - } - - // Warn about using Maps as children - if ((iterable: any).entries === iteratorFn) { + } else if ((iterable: any).entries === iteratorFn) { + // Warn about using Maps as children if (!didWarnAboutMaps) { console.error( 'Using Maps as children is not supported. ' + 'Use an array of keyed ReactElements instead.', ); + didWarnAboutMaps = true; } - didWarnAboutMaps = true; } } } @@ -2303,11 +2316,11 @@ function renderNodeDestructive( const iteratorFn = getIteratorFn(node); if (iteratorFn) { - if (__DEV__) { - validateIterable(node, iteratorFn); - } const iterator = iteratorFn.call(node); if (iterator) { + if (__DEV__) { + validateIterable(task, node, childIndex, iterator, iteratorFn); + } // We need to know how many total children are in this set, so that we // can allocate enough id slots to acommodate them. So we must exhaust // the iterator before we start recursively rendering the children. diff --git a/packages/react/src/jsx/ReactJSXElement.js b/packages/react/src/jsx/ReactJSXElement.js index 48feb83cdde35..36185639e67f0 100644 --- a/packages/react/src/jsx/ReactJSXElement.js +++ b/packages/react/src/jsx/ReactJSXElement.js @@ -1028,10 +1028,12 @@ function validateChildKeys(node, parentType) { // but now we print a separate warning for them later. if (iteratorFn !== node.entries) { const iterator = iteratorFn.call(node); - let step; - while (!(step = iterator.next()).done) { - if (isValidElement(step.value)) { - validateExplicitKey(step.value, parentType); + if (iterator !== node) { + let step; + while (!(step = iterator.next()).done) { + if (isValidElement(step.value)) { + validateExplicitKey(step.value, parentType); + } } } }