From 32fc7c9e0d385a5477e4823a24e38ab02df88637 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20O=E2=80=99Shannessy?= Date: Thu, 3 Apr 2014 16:42:43 -0700 Subject: [PATCH] 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. --- src/core/ReactDescriptor.js | 144 ++++++++++++++++++++---------------- 1 file changed, 80 insertions(+), 64 deletions(-) diff --git a/src/core/ReactDescriptor.js b/src/core/ReactDescriptor.js index be5f8ddd76ce..15603df07190 100644 --- a/src/core/ReactDescriptor.js +++ b/src/core/ReactDescriptor.js @@ -29,12 +29,23 @@ var warning = require('warning'); * object keys are not valid. This allows us to keep track of children between * updates. */ -var ownerHasExplicitKeyWarning = {}; -var ownerHasPropertyWarning = {}; +var ownerHasKeyUseWarning = { + 'react_key_warning': {}, + 'react_numeric_key_warning': {} +}; var ownerHasMonitoredObjectMap = {}; var NUMERIC_PROPERTY_REGEX = /^\d+$/; +/** + * Gets the current owner's displayName for use in warnings. + * @return {?string} Display name or undefined + */ +function getCurrentOwnerDisplayName() { + var current = ReactCurrentOwner.current; + return current && current.constructor.displayName || undefined; +} + /** * Warn if the component doesn't have an explicit key assigned to it. * This component is in an array. The array could grow and shrink or be @@ -43,92 +54,96 @@ var NUMERIC_PROPERTY_REGEX = /^\d+$/; * * @internal * @param {ReactComponent} component Component that requires a key. + * @param {*} parentType component's parent's type. */ -function validateExplicitKey(component) { +function validateExplicitKey(component, parentType) { if (component._store.validated || component.props.key != null) { return; } component._store.validated = true; - // We can't provide friendly warnings for top level components. - if (!ReactCurrentOwner.current) { + warnAndMonitorForKeyUse( + 'react_key_warning', + 'Each child in an array should have a unique "key" prop.', + component, + parentType + ); +} + +/** + * Warn if the key is being defined as an object property but has an incorrect + * value. + * + * @internal + * @param {string} name Property name of the key. + * @param {ReactComponent} component Component that requires a key. + * @param {*} parentType component's parent's type. + */ +function validatePropertyKey(name, component, parentType) { + if (!NUMERIC_PROPERTY_REGEX.test(name)) { return; } + warnAndMonitorForKeyUse( + 'react_numeric_key_warning', + 'Child objects should have non-numeric keys so ordering is preserved.', + component, + parentType + ); +} + +/** + * Shared warning and monitoring code for the key warnings. + * + * @internal + * @param {string} warningID The id used when logging. + * @param {string} message The base warning that gets output. + * @param {ReactComponent} component Component that requires a key. + * @param {*} parentType component's parent's type. + */ +function warnAndMonitorForKeyUse(warningID, message, component, parentType) { + var ownerName = getCurrentOwnerDisplayName(); + var parentName = parentType.displayName; - // Name of the component whose render method tried to pass children. - var currentName = ReactCurrentOwner.current.constructor.displayName; - if (ownerHasExplicitKeyWarning.hasOwnProperty(currentName)) { + var useName = ownerName || parentName; + var memoizer = ownerHasKeyUseWarning[warningID]; + if (memoizer.hasOwnProperty(useName)) { return; } - ownerHasExplicitKeyWarning[currentName] = true; + memoizer[useName] = true; - var message = 'Each child in an array should have a unique "key" prop. ' + - 'Check the render method of ' + currentName + '.'; + message += ownerName ? + ` Check the render method of ${ownerName}.` : + ` Check the renderComponent call using <${parentName}>.`; + // Usually the current owner is the offender, but if it accepts children as a + // property, it may be the creator of the child that's responsible for + // assigning it a key. var childOwnerName = null; - if (component._owner !== ReactCurrentOwner.current) { + if (component._owner && component._owner !== ReactCurrentOwner.current) { // Name of the component that originally created this child. - childOwnerName = - component._owner && - component._owner.constructor.displayName; - - // Usually the current owner is the offender, but if it accepts - // children as a property, it may be the creator of the child that's - // responsible for assigning it a key. - message += ' It was passed a child from ' + childOwnerName + '.'; + childOwnerName = component._owner.constructor.displayName; + + message += ` It was passed a child from ${childOwnerName}.`; } message += ' See http://fb.me/react-warning-keys for more information.'; - monitorCodeUse('react_key_warning', { - component: currentName, + monitorCodeUse(warningID, { + component: useName, componentOwner: childOwnerName }); console.warn(message); } -/** - * Warn if the key is being defined as an object property but has an incorrect - * value. - * - * @internal - * @param {string} name Property name of the key. - * @param {ReactComponent} component Component that requires a key. - */ -function validatePropertyKey(name) { - // We can't provide friendly warnings for top level components. - if (!ReactCurrentOwner.current) { - return; - } - - if (NUMERIC_PROPERTY_REGEX.test(name)) { - // Name of the component whose render method tried to pass children. - var currentName = ReactCurrentOwner.current.constructor.displayName; - if (ownerHasPropertyWarning.hasOwnProperty(currentName)) { - return; - } - ownerHasPropertyWarning[currentName] = true; - - monitorCodeUse('react_numeric_key_warning'); - console.warn( - 'Child objects should have non-numeric keys so ordering is preserved. ' + - 'Check the render method of ' + currentName + '. ' + - 'See http://fb.me/react-warning-keys for more information.' - ); - } -} - /** * Log that we're using an object map. We're considering deprecating this * feature and replace it with proper Map and ImmutableMap data structures. * * @internal + * @param {object} displayNames Contains strings with displayNames for possible + * owner components. */ -function monitorUseOfObjectMap() { - // Name of the component whose render method tried to pass children. - // We only use this to avoid spewing the logs. We lose additional - // owner stacks but hopefully one level is enough to trace the source. - var currentName = (ReactCurrentOwner.current && - ReactCurrentOwner.current.constructor.displayName) || ''; +function monitorUseOfObjectMap(displayNames) { + var currentName = getCurrentOwnerDisplayName() || ''; if (ownerHasMonitoredObjectMap.hasOwnProperty(currentName)) { return; } @@ -143,14 +158,15 @@ function monitorUseOfObjectMap() { * * @internal * @param {*} component Statically passed child of any type. + * @param {*} parentType component's parent's type. * @return {boolean} */ -function validateChildKeys(component) { +function validateChildKeys(component, parentType) { if (Array.isArray(component)) { for (var i = 0; i < component.length; i++) { var child = component[i]; if (ReactDescriptor.isValidDescriptor(child)) { - validateExplicitKey(child); + validateExplicitKey(child, parentType); } } } else if (ReactDescriptor.isValidDescriptor(component)) { @@ -159,7 +175,7 @@ function validateChildKeys(component) { } else if (component && typeof component === 'object') { monitorUseOfObjectMap(); for (var name in component) { - validatePropertyKey(name, component); + validatePropertyKey(name, component[name], parentType); } } } @@ -247,14 +263,14 @@ ReactDescriptor.createFactory = function(type) { var childrenLength = arguments.length - 1; if (childrenLength === 1) { if (__DEV__) { - validateChildKeys(children); + validateChildKeys(children, type); } props.children = children; } else if (childrenLength > 1) { var childArray = Array(childrenLength); for (var i = 0; i < childrenLength; i++) { if (__DEV__) { - validateChildKeys(arguments[i + 1]); + validateChildKeys(arguments[i + 1], type); } childArray[i] = arguments[i + 1]; }