@samitouri / QOS-React-2 / commits / af3c04b43b

Warn better for key issues

We weren't warning for key issues at the top level (renderComponent) level at all. This makes us do that, even if it's not quite perfect. closes #1351

Paul O'Shannessy committed Apr 7, 2014 at 18:11 UTC af3c04b43ba1c6c9d72f53618a5d34601e653848
1 file changed +79 -63
src/core/ReactDescriptor.js
+79 -63
@@ -29,12 +29,25 @@ var warning = require('warning');
29 * object keys are not valid. This allows us to keep track of children between
30 * updates.
31 */
32 -var ownerHasExplicitKeyWarning = {};
33 -var ownerHasPropertyWarning = {};
32 +var ownerHasKeyUseWarning = {
33 + 'react_key_warning': {},
34 + 'react_numeric_key_warning': {}
35 +};
36 var ownerHasMonitoredObjectMap = {};
37
38 var NUMERIC_PROPERTY_REGEX = /^\d+$/;
39
40 +/**
41 + * Gets the current owner's displayName for use in warnings.
42 + *
43 + * @internal
44 + * @return {?string} Display name or undefined
45 + */
46 +function getCurrentOwnerDisplayName() {
47 + var current = ReactCurrentOwner.current;
48 + return current && current.constructor.displayName || undefined;
49 +}
50 +
51 /**
52 * Warn if the component doesn't have an explicit key assigned to it.
53 * This component is in an array. The array could grow and shrink or be
@@ -43,80 +56,86 @@ var NUMERIC_PROPERTY_REGEX = /^\d+$/;
56 *
57 * @internal
58 * @param {ReactComponent} component Component that requires a key.
59 + * @param {*} parentType component's parent's type.
60 */
47 -function validateExplicitKey(component) {
61 +function validateExplicitKey(component, parentType) {
62 if (component._store.validated || component.props.key != null) {
63 return;
64 }
65 component._store.validated = true;
66
53 - // We can't provide friendly warnings for top level components.
54 - if (!ReactCurrentOwner.current) {
67 + warnAndMonitorForKeyUse(
68 + 'react_key_warning',
69 + 'Each child in an array should have a unique "key" prop.',
70 + component,
71 + parentType
72 + );
73 +}
74 +
75 +/**
76 + * Warn if the key is being defined as an object property but has an incorrect
77 + * value.
78 + *
79 + * @internal
80 + * @param {string} name Property name of the key.
81 + * @param {ReactComponent} component Component that requires a key.
82 + * @param {*} parentType component's parent's type.
83 + */
84 +function validatePropertyKey(name, component, parentType) {
85 + if (!NUMERIC_PROPERTY_REGEX.test(name)) {
86 return;
87 }
88 + warnAndMonitorForKeyUse(
89 + 'react_numeric_key_warning',
90 + 'Child objects should have non-numeric keys so ordering is preserved.',
91 + component,
92 + parentType
93 + );
94 +}
95 +
96 +/**
97 + * Shared warning and monitoring code for the key warnings.
98 + *
99 + * @internal
100 + * @param {string} warningID The id used when logging.
101 + * @param {string} message The base warning that gets output.
102 + * @param {ReactComponent} component Component that requires a key.
103 + * @param {*} parentType component's parent's type.
104 + */
105 +function warnAndMonitorForKeyUse(warningID, message, component, parentType) {
106 + var ownerName = getCurrentOwnerDisplayName();
107 + var parentName = parentType.displayName;
108
58 - // Name of the component whose render method tried to pass children.
59 - var currentName = ReactCurrentOwner.current.constructor.displayName;
60 - if (ownerHasExplicitKeyWarning.hasOwnProperty(currentName)) {
109 + var useName = ownerName || parentName;
110 + var memoizer = ownerHasKeyUseWarning[warningID];
111 + if (memoizer.hasOwnProperty(useName)) {
112 return;
113 }
63 - ownerHasExplicitKeyWarning[currentName] = true;
114 + memoizer[useName] = true;
115
65 - var message = 'Each child in an array should have a unique "key" prop. ' +
66 - 'Check the render method of ' + currentName + '.';
116 + message += ownerName ?
117 + ` Check the render method of ${ownerName}.` :
118 + ` Check the renderComponent call using <${parentName}>.`;
119
120 + // Usually the current owner is the offender, but if it accepts children as a
121 + // property, it may be the creator of the child that's responsible for
122 + // assigning it a key.
123 var childOwnerName = null;
69 - if (component._owner !== ReactCurrentOwner.current) {
124 + if (component._owner && component._owner !== ReactCurrentOwner.current) {
125 // Name of the component that originally created this child.
71 - childOwnerName =
72 - component._owner &&
73 - component._owner.constructor.displayName;
74 -
75 - // Usually the current owner is the offender, but if it accepts
76 - // children as a property, it may be the creator of the child that's
77 - // responsible for assigning it a key.
78 - message += ' It was passed a child from ' + childOwnerName + '.';
126 + childOwnerName = component._owner.constructor.displayName;
127 +
128 + message += ` It was passed a child from ${childOwnerName}.`;
129 }
130
131 message += ' See http://fb.me/react-warning-keys for more information.';
82 - monitorCodeUse('react_key_warning', {
83 - component: currentName,
132 + monitorCodeUse(warningID, {
133 + component: useName,
134 componentOwner: childOwnerName
135 });
136 console.warn(message);
137 }
138
89 -/**
90 - * Warn if the key is being defined as an object property but has an incorrect
91 - * value.
92 - *
93 - * @internal
94 - * @param {string} name Property name of the key.
95 - * @param {ReactComponent} component Component that requires a key.
96 - */
97 -function validatePropertyKey(name) {
98 - // We can't provide friendly warnings for top level components.
99 - if (!ReactCurrentOwner.current) {
100 - return;
101 - }
102 -
103 - if (NUMERIC_PROPERTY_REGEX.test(name)) {
104 - // Name of the component whose render method tried to pass children.
105 - var currentName = ReactCurrentOwner.current.constructor.displayName;
106 - if (ownerHasPropertyWarning.hasOwnProperty(currentName)) {
107 - return;
108 - }
109 - ownerHasPropertyWarning[currentName] = true;
110 -
111 - monitorCodeUse('react_numeric_key_warning');
112 - console.warn(
113 - 'Child objects should have non-numeric keys so ordering is preserved. ' +
114 - 'Check the render method of ' + currentName + '. ' +
115 - 'See http://fb.me/react-warning-keys for more information.'
116 - );
117 - }
118 -}
119 -
139 /**
140 * Log that we're using an object map. We're considering deprecating this
141 * feature and replace it with proper Map and ImmutableMap data structures.
@@ -124,11 +143,7 @@ function validatePropertyKey(name) {
143 * @internal
144 */
145 function monitorUseOfObjectMap() {
127 - // Name of the component whose render method tried to pass children.
128 - // We only use this to avoid spewing the logs. We lose additional
129 - // owner stacks but hopefully one level is enough to trace the source.
130 - var currentName = (ReactCurrentOwner.current &&
131 - ReactCurrentOwner.current.constructor.displayName) || '';
146 + var currentName = getCurrentOwnerDisplayName() || '';
147 if (ownerHasMonitoredObjectMap.hasOwnProperty(currentName)) {
148 return;
149 }
@@ -143,14 +158,15 @@ function monitorUseOfObjectMap() {
158 *
159 * @internal
160 * @param {*} component Statically passed child of any type.
161 + * @param {*} parentType component's parent's type.
162 * @return {boolean}
163 */
148 -function validateChildKeys(component) {
164 +function validateChildKeys(component, parentType) {
165 if (Array.isArray(component)) {
166 for (var i = 0; i < component.length; i++) {
167 var child = component[i];
168 if (ReactDescriptor.isValidDescriptor(child)) {
153 - validateExplicitKey(child);
169 + validateExplicitKey(child, parentType);
170 }
171 }
172 } else if (ReactDescriptor.isValidDescriptor(component)) {
@@ -159,7 +175,7 @@ function validateChildKeys(component) {
175 } else if (component && typeof component === 'object') {
176 monitorUseOfObjectMap();
177 for (var name in component) {
162 - validatePropertyKey(name, component);
178 + validatePropertyKey(name, component[name], parentType);
179 }
180 }
181 }
@@ -247,14 +263,14 @@ ReactDescriptor.createFactory = function(type) {
263 var childrenLength = arguments.length - 1;
264 if (childrenLength === 1) {
265 if (__DEV__) {
250 - validateChildKeys(children);
266 + validateChildKeys(children, type);
267 }
268 props.children = children;
269 } else if (childrenLength > 1) {
270 var childArray = Array(childrenLength);
271 for (var i = 0; i < childrenLength; i++) {
272 if (__DEV__) {
257 - validateChildKeys(arguments[i + 1]);
273 + validateChildKeys(arguments[i + 1], type);
274 }
275 childArray[i] = arguments[i + 1];
276 }