diff --git a/packages/react-dom/src/__tests__/ReactMultiChild-test.js b/packages/react-dom/src/__tests__/ReactMultiChild-test.js index 906934cfa66be..e1d4d87ea75ee 100644 --- a/packages/react-dom/src/__tests__/ReactMultiChild-test.js +++ b/packages/react-dom/src/__tests__/ReactMultiChild-test.js @@ -294,6 +294,67 @@ describe('ReactMultiChild', () => { ); }); + it('should warn for using generators as children', () => { + function* Foo() { + yield <h1 key="1">Hello</h1>; + yield <h1 key="2">World</h1>; + } + + const div = document.createElement('div'); + expect(() => { + ReactDOM.render(<Foo />, div); + }).toWarnDev( + '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' + + ' in Foo (at **)', + ); + + // Test de-duplication + ReactDOM.render(<Foo />, div); + }); + + it('should not warn for using generators in legacy iterables', () => { + const fooIterable = { + '@@iterator': function*() { + yield <h1 key="1">Hello</h1>; + yield <h1 key="2">World</h1>; + }, + }; + + function Foo() { + return fooIterable; + } + + const div = document.createElement('div'); + ReactDOM.render(<Foo />, div); + expect(div.textContent).toBe('HelloWorld'); + + ReactDOM.render(<Foo />, div); + expect(div.textContent).toBe('HelloWorld'); + }); + + it('should not warn for using generators in modern iterables', () => { + const fooIterable = { + [Symbol.iterator]: function*() { + yield <h1 key="1">Hello</h1>; + yield <h1 key="2">World</h1>; + }, + }; + + function Foo() { + return fooIterable; + } + + const div = document.createElement('div'); + ReactDOM.render(<Foo />, div); + expect(div.textContent).toBe('HelloWorld'); + + ReactDOM.render(<Foo />, div); + expect(div.textContent).toBe('HelloWorld'); + }); + it('should reorder bailed-out children', () => { class LetterInner extends React.Component { render() { diff --git a/packages/react-dom/src/__tests__/ReactMultiChildReconcile-test.js b/packages/react-dom/src/__tests__/ReactMultiChildReconcile-test.js index 090230be3e7a2..f04b7ece7aab2 100644 --- a/packages/react-dom/src/__tests__/ReactMultiChildReconcile-test.js +++ b/packages/react-dom/src/__tests__/ReactMultiChildReconcile-test.js @@ -248,7 +248,7 @@ function prepareChildrenArray(childrenArray) { return childrenArray; } -function prepareChildrenIterable(childrenArray) { +function prepareChildrenLegacyIterable(childrenArray) { return { '@@iterator': function*() { // eslint-disable-next-line no-for-of-loops/no-for-of-loops @@ -259,9 +259,27 @@ function prepareChildrenIterable(childrenArray) { }; } +function prepareChildrenModernIterable(childrenArray) { + return { + [Symbol.iterator]: function*() { + // eslint-disable-next-line no-for-of-loops/no-for-of-loops + for (const child of childrenArray) { + yield child; + } + }, + }; +} + function testPropsSequence(sequence) { testPropsSequenceWithPreparedChildren(sequence, prepareChildrenArray); - testPropsSequenceWithPreparedChildren(sequence, prepareChildrenIterable); + testPropsSequenceWithPreparedChildren( + sequence, + prepareChildrenLegacyIterable, + ); + testPropsSequenceWithPreparedChildren( + sequence, + prepareChildrenModernIterable, + ); } describe('ReactMultiChildReconcile', () => { @@ -311,7 +329,49 @@ describe('ReactMultiChildReconcile', () => { ); }); - it('should reset internal state if removed then readded in an iterable', () => { + it('should reset internal state if removed then readded in a legacy iterable', () => { + // Test basics. + const props = { + usernameToStatus: { + jcw: 'jcwStatus', + }, + }; + + const container = document.createElement('div'); + const parentInstance = ReactDOM.render( + <FriendsStatusDisplay + {...props} + prepareChildren={prepareChildrenLegacyIterable} + />, + container, + ); + let statusDisplays = parentInstance.getStatusDisplays(); + const startingInternalState = statusDisplays.jcw.getInternalState(); + + // Now remove the child. + ReactDOM.render( + <FriendsStatusDisplay prepareChildren={prepareChildrenLegacyIterable} />, + container, + ); + statusDisplays = parentInstance.getStatusDisplays(); + expect(statusDisplays.jcw).toBeFalsy(); + + // Now reset the props that cause there to be a child + ReactDOM.render( + <FriendsStatusDisplay + {...props} + prepareChildren={prepareChildrenLegacyIterable} + />, + container, + ); + statusDisplays = parentInstance.getStatusDisplays(); + expect(statusDisplays.jcw).toBeTruthy(); + expect(statusDisplays.jcw.getInternalState()).not.toBe( + startingInternalState, + ); + }); + + it('should reset internal state if removed then readded in a modern iterable', () => { // Test basics. const props = { usernameToStatus: { @@ -323,7 +383,7 @@ describe('ReactMultiChildReconcile', () => { const parentInstance = ReactDOM.render( <FriendsStatusDisplay {...props} - prepareChildren={prepareChildrenIterable} + prepareChildren={prepareChildrenModernIterable} />, container, ); @@ -332,7 +392,7 @@ describe('ReactMultiChildReconcile', () => { // Now remove the child. ReactDOM.render( - <FriendsStatusDisplay prepareChildren={prepareChildrenIterable} />, + <FriendsStatusDisplay prepareChildren={prepareChildrenModernIterable} />, container, ); statusDisplays = parentInstance.getStatusDisplays(); @@ -342,7 +402,7 @@ describe('ReactMultiChildReconcile', () => { ReactDOM.render( <FriendsStatusDisplay {...props} - prepareChildren={prepareChildrenIterable} + prepareChildren={prepareChildrenModernIterable} />, container, ); diff --git a/packages/react-reconciler/src/ReactChildFiber.js b/packages/react-reconciler/src/ReactChildFiber.js index 97aaac8c2c8b4..1ebe9863ebc56 100644 --- a/packages/react-reconciler/src/ReactChildFiber.js +++ b/packages/react-reconciler/src/ReactChildFiber.js @@ -46,6 +46,7 @@ import { import {StrictMode} from './ReactTypeOfMode'; let didWarnAboutMaps; +let didWarnAboutGenerators; let didWarnAboutStringRefInStrictMode; let ownerHasKeyUseWarning; let ownerHasFunctionTypeWarning; @@ -53,6 +54,7 @@ let warnForMissingKey = (child: mixed) => {}; if (__DEV__) { didWarnAboutMaps = false; + didWarnAboutGenerators = false; didWarnAboutStringRefInStrictMode = {}; /** @@ -903,6 +905,24 @@ function ChildReconciler(shouldTrackSideEffects) { ); 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 Flow doesn't know about toStringTag + newChildrenIterable[Symbol.toStringTag] === 'Generator' + ) { + warning( + didWarnAboutGenerators, + '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.', + ); + didWarnAboutGenerators = true; + } + // Warn about using Maps as children if ((newChildrenIterable: any).entries === iteratorFn) { warning(