diff --git a/eslint-rules/__tests__/warning-and-invariant-args-test.js b/eslint-rules/__tests__/warning-and-invariant-args-test.js index be460f6cfb..b6a25b0e11 100644 --- a/eslint-rules/__tests__/warning-and-invariant-args-test.js +++ b/eslint-rules/__tests__/warning-and-invariant-args-test.js @@ -51,17 +51,50 @@ eslintTester.addRuleTest('eslint-rules/warning-and-invariant-args', { code: "warning(true, 'expected %s, got %s');", errors: [ { - message: 'Expected 4 arguments in call to warning, but got 2 ' + - 'based on the number of "%s" substitutions', + message: + 'Expected 4 arguments in call to warning based on the number of ' + + '"%s" substitutions, but got 2', }, ], }, { - code: "warning(true, 'foobar', 'junk argument');", + code: "warning(true, 'foo is a bar under foobar', 'junk argument');", errors: [ { - message: 'Expected 2 arguments in call to warning, but got 3 ' + - 'based on the number of "%s" substitutions', + message: + 'Expected 2 arguments in call to warning based on the number of ' + + '"%s" substitutions, but got 3', + }, + ], + }, + { + code: "invariant(true, 'error!');", + errors: [ + { + message: + 'The invariant format should be able to uniquely identify this ' + + 'invariant. Please, use a more descriptive format than: error!', + }, + ], + }, + { + code: "warning(true, 'error!');", + errors: [ + { + message: + 'The warning format should be able to uniquely identify this ' + + 'warning. Please, use a more descriptive format than: error!', + }, + ], + }, + { + code: "warning(true, '%s %s, %s %s: %s (%s)', 1, 2, 3, 4, 5, 6);", + errors: [ + { + message: + 'The warning format should be able to uniquely identify this ' + + 'warning. Please, use a more descriptive format than: ' + + '%s %s, %s %s: %s (%s)', }, ], }, diff --git a/eslint-rules/warning-and-invariant-args.js b/eslint-rules/warning-and-invariant-args.js index 332262ed8b..3611935579 100644 --- a/eslint-rules/warning-and-invariant-args.js +++ b/eslint-rules/warning-and-invariant-args.js @@ -49,8 +49,8 @@ module.exports = function(context) { ); return; } - var str = getLiteralString(node.arguments[1]); - if (str === null) { + var format = getLiteralString(node.arguments[1]); + if (format === null) { context.report( node, 'The second argument to {{name}} must be a string literal', @@ -58,13 +58,22 @@ module.exports = function(context) { ); return; } - // count the number of string substitutions, plus the first two args - var expectedNArgs = (str.match(/%s/g) || []).length + 2; + if (format.length < 10 || /^[s\W]*$/.test(format)) { + context.report( + node, + 'The {{name}} format should be able to uniquely identify this ' + + '{{name}}. Please, use a more descriptive format than: {{format}}', + {name: node.callee.name, format: format} + ); + return; + } + // count the number of formating substitutions, plus the first two args + var expectedNArgs = (format.match(/%s/g) || []).length + 2; if (node.arguments.length !== expectedNArgs) { context.report( node, - 'Expected {{expectedNArgs}} arguments in call to {{name}}, but ' + - 'got {{length}} based on the number of "%s" substitutions', + 'Expected {{expectedNArgs}} arguments in call to {{name}} based on ' + + 'the number of "%s" substitutions, but got {{length}}', { expectedNArgs: expectedNArgs, name: node.callee.name, diff --git a/src/isomorphic/classic/element/ReactElementValidator.js b/src/isomorphic/classic/element/ReactElementValidator.js index 9bf72c6454..a7067ec045 100644 --- a/src/isomorphic/classic/element/ReactElementValidator.js +++ b/src/isomorphic/classic/element/ReactElementValidator.js @@ -97,10 +97,18 @@ function validateExplicitKey(element, parentType) { } element._store.validated = true; - warnAndMonitorForKeyUse( - 'Each child in an array or iterator should have a unique "key" prop.', - element, - parentType + var addenda = getAddendaForKeyUse('uniqueKey', element, parentType); + if (addenda === null) { + // we already showed the warning + return; + } + warning( + false, + 'Each child in an array or iterator should have a unique "key" prop.' + + '%s%s%s', + addenda.parentOrOwner || '', + addenda.childOwner || '', + addenda.url || '' ); } @@ -117,10 +125,18 @@ function validatePropertyKey(name, element, parentType) { if (!NUMERIC_PROPERTY_REGEX.test(name)) { return; } - warnAndMonitorForKeyUse( - 'Child objects should have non-numeric keys so ordering is preserved.', - element, - parentType + var addenda = getAddendaForKeyUse('numericKeys', element, parentType); + if (addenda === null) { + // we already showed the warning + return; + } + warning( + false, + 'Child objects should have non-numeric keys so ordering is preserved.' + + '%s%s%s', + addenda.parentOrOwner || '', + addenda.childOwner || '', + addenda.url || '' ); } @@ -128,49 +144,47 @@ function validatePropertyKey(name, element, parentType) { * Shared warning and monitoring code for the key warnings. * * @internal - * @param {string} message The base warning that gets output. + * @param {string} messageType A key used for de-duping warnings. * @param {ReactElement} element Component that requires a key. * @param {*} parentType element's parent's type. + * @returns {?object} A set of addenda to use in the warning message, or null + * if the warning has already been shown before (and shouldn't be shown again). */ -function warnAndMonitorForKeyUse(message, element, parentType) { +function getAddendaForKeyUse(messageType, element, parentType) { var ownerName = getCurrentOwnerDisplayName(); var parentName = typeof parentType === 'string' ? parentType : parentType.displayName || parentType.name; var useName = ownerName || parentName; - var memoizer = ownerHasKeyUseWarning[message] || ( - ownerHasKeyUseWarning[message] = {} + var memoizer = ownerHasKeyUseWarning[messageType] || ( + ownerHasKeyUseWarning[messageType] = {} ); - if (memoizer.hasOwnProperty(useName)) { - return; + if (memoizer[useName]) { + return null; } memoizer[useName] = true; - var parentOrOwnerAddendum = - ownerName ? ` Check the render method of ${ownerName}.` : - parentName ? ` Check the React.render call using <${parentName}>.` : - ''; + var addenda = { + parentOrOwner: + ownerName ? ` Check the render method of ${ownerName}.` : + parentName ? ` Check the React.render call using <${parentName}>.` : + null, + url: ' See https://fb.me/react-warning-keys for more information.', + childOwner: null, + }; // 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 childOwnerAddendum = ''; if (element && element._owner && element._owner !== ReactCurrentOwner.current) { - // Name of the component that originally created this child. - var childOwnerName = getName(element._owner); - - childOwnerAddendum = ` It was passed a child from ${childOwnerName}.`; + // Give the component that originally created this child. + addenda.childOwner = + ` It was passed a child from ${getName(element._owner)}.`; } - warning( - false, - '%s%s%s See https://fb.me/react-warning-keys for more information.', - message, - parentOrOwnerAddendum, - childOwnerAddendum - ); + return addenda; } /** diff --git a/src/shared/vendor/core/warning.js b/src/shared/vendor/core/warning.js index 90365feb7c..4d525db5ab 100644 --- a/src/shared/vendor/core/warning.js +++ b/src/shared/vendor/core/warning.js @@ -31,13 +31,6 @@ if (__DEV__) { ); } - if (format.length < 10 || /^[s\W]*$/.test(format)) { - throw new Error( - 'The warning format should be able to uniquely identify this ' + - 'warning. Please, use a more descriptive format than: ' + format - ); - } - if (format.indexOf('Failed Composite propType: ') === 0) { return; // Ignore CompositeComponent proptype check. }