diff --git a/.eslintrc.js b/.eslintrc.js index fda855f401730..997e5bb46f5f9 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -55,6 +55,7 @@ module.exports = { // CUSTOM RULES // the second argument of warning/invariant should be a literal string 'react-internal/warning-and-invariant-args': ERROR, + 'react-internal/no-primitive-constructors': ERROR, }, globals: { diff --git a/eslint-rules/__tests__/no-primitive-constructors-test.js b/eslint-rules/__tests__/no-primitive-constructors-test.js new file mode 100644 index 0000000000000..8756117f8b924 --- /dev/null +++ b/eslint-rules/__tests__/no-primitive-constructors-test.js @@ -0,0 +1,50 @@ +/** + * Copyright 2015-present, Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + * + * @emails react-core + */ + +'use strict'; + +var rule = require('../no-primitive-constructors'); +var RuleTester = require('eslint').RuleTester; +var ruleTester = new RuleTester(); + +ruleTester.run('eslint-rules/no-primitive-constructors', rule, { + valid: [ + '!!obj', + "'' + obj", + '+string', + ], + invalid: [ + { + code: 'Boolean(obj)', + errors: [ + { + message: 'Do not use the Boolean constructor. To cast a value to a boolean, use double negation: !!value', + }, + ], + }, + { + code: 'String(obj)', + errors: [ + { + message: 'Do not use the String constructor. To cast a value to a string, concat it with the empty string (unless it\'s a symbol, which has different semantics): \'\' + value', + }, + ], + }, + { + code: 'Number(string)', + errors: [ + { + message: 'Do not use the Number constructor. To cast a value to a number, use the plus operator: +value', + }, + ], + }, + ], +}); diff --git a/eslint-rules/index.js b/eslint-rules/index.js index 5efee2d47caf2..d92df8309c1af 100644 --- a/eslint-rules/index.js +++ b/eslint-rules/index.js @@ -3,5 +3,6 @@ module.exports = { rules: { 'warning-and-invariant-args': require('./warning-and-invariant-args'), + 'no-primitive-constructors': require('./no-primitive-constructors'), }, }; diff --git a/eslint-rules/no-primitive-constructors.js b/eslint-rules/no-primitive-constructors.js new file mode 100644 index 0000000000000..0af2602d50352 --- /dev/null +++ b/eslint-rules/no-primitive-constructors.js @@ -0,0 +1,52 @@ +/** + * Copyright 2015-present, Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + * + * @emails react-core +*/ + +'use strict'; + +module.exports = function(context) { + function report(node, name, msg) { + context.report(node, `Do not use the ${name} constructor. ${msg}`); + } + + function check(node) { + const name = node.callee.name; + switch (name) { + case 'Boolean': + report( + node, + name, + 'To cast a value to a boolean, use double negation: !!value' + ); + break; + case 'String': + report( + node, + name, + 'To cast a value to a string, concat it with the empty string ' + + '(unless it\'s a symbol, which has different semantics): ' + + '\'\' + value' + ); + break; + case 'Number': + report( + node, + name, + 'To cast a value to a number, use the plus operator: +value' + ); + break; + } + } + + return { + CallExpression: check, + NewExpression: check, + }; +}; diff --git a/package.json b/package.json index b549bbb3617a6..32e05560eef11 100644 --- a/package.json +++ b/package.json @@ -46,7 +46,7 @@ "eslint-plugin-babel": "^3.3.0", "eslint-plugin-flowtype": "^2.25.0", "eslint-plugin-react": "^6.7.1", - "eslint-plugin-react-internal": "file:eslint-rules", + "eslint-plugin-react-internal": "file:./eslint-rules", "fbjs": "^0.8.9", "fbjs-scripts": "^0.6.0", "flow-bin": "^0.37.0", diff --git a/scripts/fiber/tests-passing.txt b/scripts/fiber/tests-passing.txt index c5ff7cee6acb5..2c6088f7591ea 100644 --- a/scripts/fiber/tests-passing.txt +++ b/scripts/fiber/tests-passing.txt @@ -1,3 +1,11 @@ +eslint-rules/__tests__/no-primitive-constructors-test.js +* !!obj +* '' + obj +* +string +* Boolean(obj) +* String(obj) +* Number(string) + eslint-rules/__tests__/warning-and-invariant-args-test.js * warning(true, 'hello, world'); * warning(true, 'expected %s, got %s', 42, 24); diff --git a/src/renderers/dom/fiber/ReactDOMFiber.js b/src/renderers/dom/fiber/ReactDOMFiber.js index cebe0414812b4..42899b7b2db11 100644 --- a/src/renderers/dom/fiber/ReactDOMFiber.js +++ b/src/renderers/dom/fiber/ReactDOMFiber.js @@ -196,8 +196,9 @@ var DOMRenderer = ReactFiberReconciler({ typeof props.children === 'string' || typeof props.children === 'number' ) { + const string = '' + props.children; const ownAncestorInfo = updatedAncestorInfo(hostContextDev.ancestorInfo, type, null); - validateDOMNesting(null, String(props.children), null, ownAncestorInfo); + validateDOMNesting(null, string, null, ownAncestorInfo); } parentNamespace = hostContextDev.namespace; } else { @@ -237,8 +238,9 @@ var DOMRenderer = ReactFiberReconciler({ typeof newProps.children === 'string' || typeof newProps.children === 'number' )) { + const string = '' + newProps.children; const ownAncestorInfo = updatedAncestorInfo(hostContextDev.ancestorInfo, type, null); - validateDOMNesting(null, String(newProps.children), null, ownAncestorInfo); + validateDOMNesting(null, string, null, ownAncestorInfo); } } return diffProperties(domElement, type, oldProps, newProps, rootContainerInstance); @@ -411,9 +413,9 @@ var ReactDOM = { } if (__DEV__) { - const isRootRenderedBySomeReact = Boolean(container._reactRootContainer); + const isRootRenderedBySomeReact = !!container._reactRootContainer; const rootEl = getReactRootElementInContainer(container); - const hasNonRootReactChild = Boolean(rootEl && ReactDOMComponentTree.getInstanceFromNode(rootEl)); + const hasNonRootReactChild = !!(rootEl && ReactDOMComponentTree.getInstanceFromNode(rootEl)); warning( !hasNonRootReactChild || diff --git a/src/renderers/dom/fiber/wrappers/ReactDOMFiberSelect.js b/src/renderers/dom/fiber/wrappers/ReactDOMFiberSelect.js index 88853b3014718..6e874d85f0254 100644 --- a/src/renderers/dom/fiber/wrappers/ReactDOMFiberSelect.js +++ b/src/renderers/dom/fiber/wrappers/ReactDOMFiberSelect.js @@ -134,7 +134,7 @@ var ReactDOMSelect = { var value = props.value; node._wrapperState = { initialValue: value != null ? value : props.defaultValue, - wasMultiple: Boolean(props.multiple), + wasMultiple: !!props.multiple, }; if ( @@ -153,11 +153,11 @@ var ReactDOMSelect = { didWarnValueDefaultValue = true; } - node.multiple = Boolean(props.multiple); + node.multiple = !!props.multiple; if (value != null) { - updateOptions(node, Boolean(props.multiple), value); + updateOptions(node, !!props.multiple, value); } else if (props.defaultValue != null) { - updateOptions(node, Boolean(props.multiple), props.defaultValue); + updateOptions(node, !!props.multiple, props.defaultValue); } }, @@ -168,18 +168,18 @@ var ReactDOMSelect = { node._wrapperState.initialValue = undefined; var wasMultiple = node._wrapperState.wasMultiple; - node._wrapperState.wasMultiple = Boolean(props.multiple); + node._wrapperState.wasMultiple = !!props.multiple; var value = props.value; if (value != null) { - updateOptions(node, Boolean(props.multiple), value); - } else if (wasMultiple !== Boolean(props.multiple)) { + updateOptions(node, !!props.multiple, value); + } else if (wasMultiple !== !!props.multiple) { // For simplicity, reapply `defaultValue` if `multiple` is toggled. if (props.defaultValue != null) { - updateOptions(node, Boolean(props.multiple), props.defaultValue); + updateOptions(node, !!props.multiple, props.defaultValue); } else { // Revert the select back to its default unselected state. - updateOptions(node, Boolean(props.multiple), props.multiple ? [] : ''); + updateOptions(node, !!props.multiple, props.multiple ? [] : ''); } } }, @@ -189,7 +189,7 @@ var ReactDOMSelect = { var value = props.value; if (value != null) { - updateOptions(node, Boolean(props.multiple), value); + updateOptions(node, !!props.multiple, value); } }, }; diff --git a/src/renderers/dom/shared/ReactBrowserEventEmitter.js b/src/renderers/dom/shared/ReactBrowserEventEmitter.js index a191a45b14919..4e994d9a030ed 100644 --- a/src/renderers/dom/shared/ReactBrowserEventEmitter.js +++ b/src/renderers/dom/shared/ReactBrowserEventEmitter.js @@ -154,7 +154,7 @@ var topEventMapping = { /** * To ensure no conflicts with other potential React instances on the page */ -var topListenersIDKey = '_reactListenersID' + String(Math.random()).slice(2); +var topListenersIDKey = '_reactListenersID' + ('' + Math.random()).slice(2); function getListeningForDocument(mountAt) { // In IE8, `mountAt` is a host object and doesn't have `hasOwnProperty` diff --git a/src/renderers/dom/shared/ReactDOMComponentTree.js b/src/renderers/dom/shared/ReactDOMComponentTree.js index 78c543bea24c3..b843b9407a004 100644 --- a/src/renderers/dom/shared/ReactDOMComponentTree.js +++ b/src/renderers/dom/shared/ReactDOMComponentTree.js @@ -31,7 +31,7 @@ var internalEventHandlersKey = '__reactEventHandlers$' + randomKey; */ function shouldPrecacheNode(node, nodeID) { return (node.nodeType === 1 && - node.getAttribute(ATTR_NAME) === String(nodeID)) || + node.getAttribute(ATTR_NAME) === ('' + nodeID)) || (node.nodeType === 8 && node.nodeValue === ' react-text: ' + nodeID + ' ') || (node.nodeType === 8 && diff --git a/src/renderers/dom/shared/validateDOMNesting.js b/src/renderers/dom/shared/validateDOMNesting.js index 8fa3216f70ce0..a0c2c7434966b 100644 --- a/src/renderers/dom/shared/validateDOMNesting.js +++ b/src/renderers/dom/shared/validateDOMNesting.js @@ -400,7 +400,7 @@ if (__DEV__) { childTag, ancestorInstance, ancestorTag, - Boolean(invalidParent) + !!invalidParent ) + '.'; } else { addendum = getCurrentFiberStackAddendum(); diff --git a/src/renderers/dom/stack/client/ReactDOMComponent.js b/src/renderers/dom/stack/client/ReactDOMComponent.js index 9fd3a45f921ef..035726db6e552 100644 --- a/src/renderers/dom/stack/client/ReactDOMComponent.js +++ b/src/renderers/dom/stack/client/ReactDOMComponent.js @@ -185,7 +185,7 @@ if (__DEV__) { return; } - validateDOMNesting(null, String(content), this, this._ancestorInfo); + validateDOMNesting(null, '' + content, this, this._ancestorInfo); this._contentDebugID = contentDebugID; if (hasExistingContent) { ReactInstrumentation.debugTool.onBeforeUpdateComponent(contentDebugID, content); diff --git a/src/renderers/dom/stack/client/ReactMount.js b/src/renderers/dom/stack/client/ReactMount.js index a1554d81bdcef..afd78bf317bce 100644 --- a/src/renderers/dom/stack/client/ReactMount.js +++ b/src/renderers/dom/stack/client/ReactMount.js @@ -440,7 +440,7 @@ var ReactMount = { callback === null || typeof callback === 'function', 'render(...): Expected the last optional `callback` argument to be a ' + 'function. Instead received: %s.', - String(callback) + '' + callback ); } if (!React.isValidElement(nextElement)) { diff --git a/src/renderers/dom/stack/client/wrappers/ReactDOMSelect.js b/src/renderers/dom/stack/client/wrappers/ReactDOMSelect.js index 6191b4999bd35..3bde1212c02d3 100644 --- a/src/renderers/dom/stack/client/wrappers/ReactDOMSelect.js +++ b/src/renderers/dom/stack/client/wrappers/ReactDOMSelect.js @@ -135,7 +135,7 @@ var ReactDOMSelect = { inst._wrapperState = { initialValue: value != null ? value : props.defaultValue, listeners: null, - wasMultiple: Boolean(props.multiple), + wasMultiple: !!props.multiple, }; if ( @@ -169,18 +169,18 @@ var ReactDOMSelect = { inst._wrapperState.initialValue = undefined; var wasMultiple = inst._wrapperState.wasMultiple; - inst._wrapperState.wasMultiple = Boolean(props.multiple); + inst._wrapperState.wasMultiple = !!props.multiple; var value = props.value; if (value != null) { - updateOptions(inst, Boolean(props.multiple), value); - } else if (wasMultiple !== Boolean(props.multiple)) { + updateOptions(inst, !!props.multiple, value); + } else if (wasMultiple !== !!props.multiple) { // For simplicity, reapply `defaultValue` if `multiple` is toggled. if (props.defaultValue != null) { - updateOptions(inst, Boolean(props.multiple), props.defaultValue); + updateOptions(inst, !!props.multiple, props.defaultValue); } else { // Revert the select back to its default unselected state. - updateOptions(inst, Boolean(props.multiple), props.multiple ? [] : ''); + updateOptions(inst, !!props.multiple, props.multiple ? [] : ''); } } }, @@ -191,7 +191,7 @@ var ReactDOMSelect = { var value = props.value; if (value != null) { - updateOptions(inst, Boolean(props.multiple), value); + updateOptions(inst, !!props.multiple, value); } } }, diff --git a/src/renderers/shared/fiber/ReactChildFiber.js b/src/renderers/shared/fiber/ReactChildFiber.js index 79f80c9e40887..57095c921ed45 100644 --- a/src/renderers/shared/fiber/ReactChildFiber.js +++ b/src/renderers/shared/fiber/ReactChildFiber.js @@ -95,7 +95,7 @@ function coerceRef(current: Fiber | null, element: ReactElement) { 'bug in React. Please file an issue.', mixedRef ); - const stringRef = String(mixedRef); + const stringRef = '' + mixedRef; // Check if previous string ref matches new string ref if (current !== null && current.ref !== null && current.ref._stringRef === stringRef) { return current.ref; @@ -117,7 +117,8 @@ function coerceRef(current: Fiber | null, element: ReactElement) { function throwOnInvalidObjectType(returnFiber : Fiber, newChild : Object) { if (returnFiber.type !== 'textarea') { - const childrenString = String(newChild); + // $FlowFixMe - Intentional cast to string + const childrenString = '' + newChild; let addendum = ''; if (__DEV__) { addendum = diff --git a/src/renderers/shared/fiber/ReactFiberClassComponent.js b/src/renderers/shared/fiber/ReactFiberClassComponent.js index 28562795370d8..15bd45bba42dd 100644 --- a/src/renderers/shared/fiber/ReactFiberClassComponent.js +++ b/src/renderers/shared/fiber/ReactFiberClassComponent.js @@ -47,7 +47,8 @@ if (__DEV__) { '%s(...): Expected the last optional `callback` argument to be a ' + 'function. Instead received: %s.', callerName, - String(callback) + // $FlowFixMe - Intentional cast to string + '' + callback ); }; } diff --git a/src/renderers/shared/fiber/ReactFiberReconciler.js b/src/renderers/shared/fiber/ReactFiberReconciler.js index 3f910aa1d9af2..2aa797ffb4ac7 100644 --- a/src/renderers/shared/fiber/ReactFiberReconciler.js +++ b/src/renderers/shared/fiber/ReactFiberReconciler.js @@ -167,7 +167,8 @@ module.exports = function( callback === null || typeof callback === 'function', 'render(...): Expected the last optional `callback` argument to be a ' + 'function. Instead received: %s.', - String(callback) + // $FlowFixMe - Intentional cast to string + '' + callback ); } addTopLevelUpdate(current, nextState, callback, priorityLevel); diff --git a/src/renderers/shared/fiber/ReactFiberScheduler.js b/src/renderers/shared/fiber/ReactFiberScheduler.js index 8ab6ee177f932..3f23452a0e4bd 100644 --- a/src/renderers/shared/fiber/ReactFiberScheduler.js +++ b/src/renderers/shared/fiber/ReactFiberScheduler.js @@ -776,7 +776,7 @@ module.exports = function(config : HostConfig(config : HostConfig(config : HostConfig