From 6e18e40896ab3d22be5376675affad240ea4d4f9 Mon Sep 17 00:00:00 2001 From: Matthew Herbst Date: Thu, 30 Mar 2017 16:39:09 -0700 Subject: [PATCH] Add a `reservedFirst` option to the `jsx-sort-props` rule This adds a new option, `reservedFirst`, to the `jsx-sort-props` rule. This option can be enabled with a boolean, or an array configuration. Using the array configuration allows customizing the list of reserved props that are checked. --- docs/rules/jsx-sort-props.md | 23 +++++- lib/rules/jsx-sort-props.js | 114 +++++++++++++++++++++++++++++- tests/lib/rules/jsx-sort-props.js | 110 +++++++++++++++++++++++++++- 3 files changed, 242 insertions(+), 5 deletions(-) diff --git a/docs/rules/jsx-sort-props.md b/docs/rules/jsx-sort-props.md index 81d1a7bfdf..71b055b362 100644 --- a/docs/rules/jsx-sort-props.md +++ b/docs/rules/jsx-sort-props.md @@ -28,7 +28,8 @@ The following patterns are considered okay and do not cause warnings: "shorthandFirst": , "shorthandLast": , "ignoreCase": , - "noSortAlphabetically": + "noSortAlphabetically": , + "reservedFirst": |>, }] ... ``` @@ -75,6 +76,26 @@ When `true`, alphabetical order is not enforced: ``` +### `reservedFirst` + +This can be a boolean or an array option. + +When `reservedFirst` is defined, React reserved props (`children`, `dangerouslySetInnerHTML` - **only for DOM components**, `key`, and `ref`) must be listed before all other props, but still respecting the alphabetical order: + +```jsx + +
+ +``` + +If given as an array, the array's values will override the default list of reserved props. **Note**: the values in the array may only be a **subset** of React reserved props. + +With `reservedFirst: [2, ["key"]]`, the following will not warn: + +```jsx + +``` + ## When not to use This rule is a formatting preference and not following it won't negatively affect the quality of your code. If alphabetizing props isn't a part of your coding standards, then you can leave this rule off. diff --git a/lib/rules/jsx-sort-props.js b/lib/rules/jsx-sort-props.js index 335675bfe0..953352b6b4 100644 --- a/lib/rules/jsx-sort-props.js +++ b/lib/rules/jsx-sort-props.js @@ -4,6 +4,7 @@ */ 'use strict'; +var elementType = require('jsx-ast-utils/elementType'); var propName = require('jsx-ast-utils/propName'); // ------------------------------------------------------------------------------ @@ -14,6 +15,72 @@ function isCallbackPropName(name) { return /^on[A-Z]/.test(name); } +var COMPAT_TAG_REGEX = /^[a-z]|\-/; +function isDOMComponent(node) { + var name = elementType(node); + + // Get namespace if the type is JSXNamespacedName or JSXMemberExpression + if (name.indexOf(':') > -1) { + name = name.substring(0, name.indexOf(':')); + } else if (name.indexOf('.') > -1) { + name = name.substring(0, name.indexOf('.')); + } + + return COMPAT_TAG_REGEX.test(name); +} + +var RESERVED_PROPS_LIST = [ + 'children', + 'dangerouslySetInnerHTML', + 'key', + 'ref' +]; + +function isReservedPropName(name, list) { + return list.indexOf(name) >= 0; +} + +/** + * Checks if the `reservedFirst` option is valid + * @param {Object} context The context of the rule + * @param {Boolean|Array} reservedFirst The `reservedFirst` option + * @return {?Function} If an error is detected, a function to generate the error message, otherwise, `undefined` + */ +// eslint-disable-next-line consistent-return +function validateReservedFirstConfig(context, reservedFirst) { + if (reservedFirst) { + if (Array.isArray(reservedFirst)) { + // Only allow a subset of reserved words in customized lists + // eslint-disable-next-line consistent-return + var nonReservedWords = reservedFirst.filter(function(word) { + if (!isReservedPropName(word, RESERVED_PROPS_LIST)) { + return true; + } + }); + + if (reservedFirst.length === 0) { + return function(decl) { + context.report({ + node: decl, + message: 'A customized reserved first list must not be empty' + }); + }; + } else if (nonReservedWords.length > 0) { + return function(decl) { + context.report({ + node: decl, + message: 'A customized reserved first list must only contain a subset of React reserved props.' + + ' Remove: {{ nonReservedWords }}', + data: { + nonReservedWords: nonReservedWords.toString() + } + }); + }; + } + } + } +} + module.exports = { meta: { docs: { @@ -44,6 +111,9 @@ module.exports = { // Whether alphabetical sorting should be enforced noSortAlphabetically: { type: 'boolean' + }, + reservedFirst: { + type: ['array', 'boolean'] } }, additionalProperties: false @@ -58,9 +128,19 @@ module.exports = { var shorthandFirst = configuration.shorthandFirst || false; var shorthandLast = configuration.shorthandLast || false; var noSortAlphabetically = configuration.noSortAlphabetically || false; + var reservedFirst = configuration.reservedFirst || false; + var reservedFirstError = validateReservedFirstConfig(context, reservedFirst); + var reservedList = Array.isArray(reservedFirst) ? reservedFirst : RESERVED_PROPS_LIST; return { JSXOpeningElement: function(node) { + // `dangerouslySetInnerHTML` is only "reserved" on DOM components + if (reservedFirst && !isDOMComponent(node)) { + reservedList = reservedList.filter(function(prop) { + return prop !== 'dangerouslySetInnerHTML'; + }); + } + node.attributes.reduce(function(memo, decl, idx, attrs) { if (decl.type === 'JSXSpreadAttribute') { return attrs[idx + 1]; @@ -70,15 +150,45 @@ module.exports = { var currentPropName = propName(decl); var previousValue = memo.value; var currentValue = decl.value; - var previousIsCallback = isCallbackPropName(previousPropName); - var currentIsCallback = isCallbackPropName(currentPropName); if (ignoreCase) { previousPropName = previousPropName.toLowerCase(); currentPropName = currentPropName.toLowerCase(); } + if (reservedFirst) { + if (reservedFirstError) { + reservedFirstError(decl); + return memo; + } + + var previousIsReserved = isReservedPropName(previousPropName, reservedList); + var currentIsReserved = isReservedPropName(currentPropName, reservedList); + + if (previousIsReserved && currentIsReserved) { + if (!noSortAlphabetically && currentPropName < previousPropName) { + context.report({ + node: decl, + message: 'Props should be sorted alphabetically' + }); + return memo; + } + return decl; + } + if (!previousIsReserved && currentIsReserved) { + context.report({ + node: decl, + message: 'Reserved props must be listed before all other props' + }); + return memo; + } + return decl; + } + if (callbacksLast) { + var previousIsCallback = isCallbackPropName(previousPropName); + var currentIsCallback = isCallbackPropName(currentPropName); + if (!previousIsCallback && currentIsCallback) { // Entering the callback prop section return decl; diff --git a/tests/lib/rules/jsx-sort-props.js b/tests/lib/rules/jsx-sort-props.js index 1e3366419f..1664baae0d 100644 --- a/tests/lib/rules/jsx-sort-props.js +++ b/tests/lib/rules/jsx-sort-props.js @@ -41,6 +41,16 @@ var expectedShorthandLastError = { message: 'Shorthand props must be listed after all other props', type: 'JSXAttribute' }; +var expectedReservedFirstError = { + message: 'Reserved props must be listed before all other props', + type: 'JSXAttribute' +}; +var expectedEmptyReservedFirstError = { + message: 'A customized reserved first list must not be empty' +}; +var expectedInvalidReservedFirstError = { + message: 'A customized reserved first list must only contain a subset of React reserved props. Remove: notReserved' +}; var callbacksLastArgs = [{ callbacksLast: true }]; @@ -63,6 +73,22 @@ var noSortAlphabeticallyArgs = [{ var sortAlphabeticallyArgs = [{ noSortAlphabetically: false }]; +var reservedFirstAsBooleanArgs = [{ + reservedFirst: true +}]; +var reservedFirstAsArrayArgs = [{ + reservedFirst: ['children', 'dangerouslySetInnerHTML', 'key'] +}]; +var reservedFirstWithNoSortAlphabeticallyArgs = [{ + noSortAlphabetically: true, + reservedFirst: true +}]; +var reservedFirstAsEmptyArrayArgs = [{ + reservedFirst: [] +}]; +var reservedFirstAsInvalidArrayArgs = [{ + reservedFirst: ['notReserved'] +}]; ruleTester.run('jsx-sort-props', rule, { valid: [ @@ -93,7 +119,38 @@ ruleTester.run('jsx-sort-props', rule, { }, // noSortAlphabetically {code: ';', options: noSortAlphabeticallyArgs, parserOptions: parserOptions}, - {code: ';', options: noSortAlphabeticallyArgs, parserOptions: parserOptions} + {code: ';', options: noSortAlphabeticallyArgs, parserOptions: parserOptions}, + // reservedFirst + { + code: '} key={0} ref="r" a b c />', + options: reservedFirstAsBooleanArgs, + parserOptions: parserOptions + }, + { + code: '} key={0} ref="r" a b c dangerouslySetInnerHTML={{__html: "EPR"}} />', + options: reservedFirstAsBooleanArgs, + parserOptions: parserOptions + }, + { + code: '} key={0} a ref="r" />', + options: reservedFirstAsArrayArgs, + parserOptions: parserOptions + }, + { + code: '} key={0} a dangerouslySetInnerHTML={{__html: "EPR"}} ref="r" />', + options: reservedFirstAsArrayArgs, + parserOptions: parserOptions + }, + { + code: '} b a c />', + options: reservedFirstWithNoSortAlphabeticallyArgs, + parserOptions: parserOptions + }, + { + code: '
} b a c />', + options: reservedFirstWithNoSortAlphabeticallyArgs, + parserOptions: parserOptions + } ], invalid: [ {code: ';', errors: [expectedError], parserOptions: parserOptions}, @@ -132,6 +189,55 @@ ruleTester.run('jsx-sort-props', rule, { options: shorthandLastArgs, parserOptions: parserOptions }, - {code: ';', errors: [expectedError], options: sortAlphabeticallyArgs, parserOptions: parserOptions} + {code: ';', errors: [expectedError], options: sortAlphabeticallyArgs, parserOptions: parserOptions}, + // reservedFirst + { + code: '', + options: reservedFirstAsBooleanArgs, + errors: [expectedReservedFirstError], + parserOptions: parserOptions + }, + { + code: '
', + options: reservedFirstAsBooleanArgs, + errors: [expectedReservedFirstError], + parserOptions: parserOptions + }, + { + code: '', + options: reservedFirstAsBooleanArgs, + errors: [expectedError], + parserOptions: parserOptions + }, + { + code: '', + options: reservedFirstAsBooleanArgs, + errors: [expectedReservedFirstError], + parserOptions: parserOptions + }, + { + code: '} />', + options: reservedFirstAsArrayArgs, + errors: [expectedError], + parserOptions: parserOptions + }, + { + code: '', + options: reservedFirstWithNoSortAlphabeticallyArgs, + errors: [expectedReservedFirstError], + parserOptions: parserOptions + }, + { + code: '', + options: reservedFirstAsEmptyArrayArgs, + errors: [expectedEmptyReservedFirstError], + parserOptions: parserOptions + }, + { + code: '', + options: reservedFirstAsInvalidArrayArgs, + errors: [expectedInvalidReservedFirstError], + parserOptions: parserOptions + } ] });