@samitouri / QOS-React / commits / 14962f8b40

Continue warning if a Map is rendered as a child

The entries of a map are a two-element array of [key, value], which React will treat as fragments with children. This is unlikely to ever be the intended behavior, so we warn.

Andrew Clark committed Feb 6, 2017 at 17:07 UTC 14962f8b40373bc5029dda8fc9c940d90321e8fa
4 files changed +68
scripts/fiber/tests-passing.txt
+1
@@ -1512,6 +1512,7 @@ src/renderers/shared/shared/__tests__/ReactMultiChild-test.js
1512 * should replace children with different keys
1513 * should warn for duplicated array keys with component stack info
1514 * should warn for duplicated iterable keys with component stack info
1515 +* should warn for using maps as children with owner info
1516 * should reorder bailed-out children
1517 * prepares new children before unmounting old
1518
src/renderers/shared/fiber/ReactChildFiber.js
+24
@@ -42,6 +42,7 @@ if (__DEV__) {
42 var { getCurrentFiberStackAddendum } = require('ReactDebugCurrentFiber');
43 var { getComponentName } = require('ReactFiberTreeReflection');
44 var warning = require('warning');
45 + var didWarnAboutMaps = false;
46 }
47
48 const {
@@ -802,6 +803,29 @@ function ChildReconciler(shouldClone, shouldTrackSideEffects) {
803 }
804
805 if (__DEV__) {
806 + // Warn about using Maps as children
807 + if (typeof newChildrenIterable.entries === 'function') {
808 + const possibleMap = (newChildrenIterable : any);
809 + if (possibleMap.entries === iteratorFn) {
810 + let mapsAsChildrenAddendum = '';
811 + const owner = ReactCurrentOwner.owner || returnFiber._debugOwner;
812 + if (owner && typeof owner.tag === 'number') {
813 + const mapsAsChildrenOwnerName = getComponentName((owner : any));
814 + if (mapsAsChildrenOwnerName) {
815 + mapsAsChildrenAddendum = '\n\nCheck the render method of `' + mapsAsChildrenOwnerName + '`.';
816 + }
817 + }
818 + warning(
819 + didWarnAboutMaps,
820 + 'Using Maps as children is unsupported and will likely yield ' +
821 + 'unexpected results. Convert it to a sequence/iterable of keyed ' +
822 + 'ReactElements instead.%s',
823 + mapsAsChildrenAddendum
824 + );
825 + didWarnAboutMaps = true;
826 + }
827 + }
828 +
829 // First, validate keys.
830 // We'll get a different iterator later for the main pass.
831 const newChildren = iteratorFn.call(newChildrenIterable);
src/renderers/shared/shared/__tests__/ReactMultiChild-test.js
+19
@@ -263,6 +263,25 @@ describe('ReactMultiChild', () => {
263 });
264 });
265
266 + it('should warn for using maps as children with owner info', () => {
267 + spyOn(console, 'error');
268 + class Parent extends React.Component {
269 + render() {
270 + return (
271 + <div>{new Map([['foo', 0], ['bar', 1]])}</div>
272 + );
273 + }
274 + }
275 + var container = document.createElement('div');
276 + ReactDOM.render(<Parent />, container);
277 + expectDev(console.error.calls.count()).toBe(1);
278 + expectDev(console.error.calls.argsFor(0)[0]).toBe(
279 + 'Warning: Using Maps as children is unsupported and will likely yield ' +
280 + 'unexpected results. Convert it to a sequence/iterable of keyed ' +
281 + 'ReactElements instead.\n\nCheck the render method of `Parent`.'
282 + );
283 + });
284 +
285 it('should reorder bailed-out children', () => {
286 spyOn(console, 'error');
287
src/shared/utils/traverseAllChildren.js
+24
@@ -17,6 +17,7 @@ var REACT_ELEMENT_TYPE = require('ReactElementSymbol');
17 var getIteratorFn = require('getIteratorFn');
18 var invariant = require('invariant');
19 var KeyEscapeUtils = require('KeyEscapeUtils');
20 +var warning = require('warning');
21
22 var SEPARATOR = '.';
23 var SUBSEPARATOR = ':';
@@ -32,6 +33,8 @@ var SUBSEPARATOR = ':';
33 * pattern.
34 */
35
36 + var didWarnAboutMaps = false;
37 +
38 /**
39 * Generate a key string that identifies a component within a set.
40 *
@@ -106,6 +109,27 @@ function traverseAllChildrenImpl(
109 } else {
110 var iteratorFn = getIteratorFn(children);
111 if (iteratorFn) {
112 + if (__DEV__) {
113 + // Warn about using Maps as children
114 + if (iteratorFn === children.entries) {
115 + let mapsAsChildrenAddendum = '';
116 + if (ReactCurrentOwner.current) {
117 + var mapsAsChildrenOwnerName = ReactCurrentOwner.current.getName();
118 + if (mapsAsChildrenOwnerName) {
119 + mapsAsChildrenAddendum = '\n\nCheck the render method of `' + mapsAsChildrenOwnerName + '`.';
120 + }
121 + }
122 + warning(
123 + didWarnAboutMaps,
124 + 'Using Maps as children is unsupported and will likely yield ' +
125 + 'unexpected results. Convert it to a sequence/iterable of keyed ' +
126 + 'ReactElements instead.%s',
127 + mapsAsChildrenAddendum
128 + );
129 + didWarnAboutMaps = true;
130 + }
131 + }
132 +
133 var iterator = iteratorFn.call(children);
134 var step;
135 var ii = 0;