Skip to content

Commit

Permalink
Merge pull request #1134 from MatthewHerbst/jsx-sort-props/new-rule-o…
Browse files Browse the repository at this point in the history
…ption--reservedFirst

[New] Add a `reservedFirst` option to the `jsx-sort-props` rule
  • Loading branch information
ljharb committed Apr 28, 2017
2 parents 14dbf99 + 6e18e40 commit dfe190b
Show file tree
Hide file tree
Showing 3 changed files with 242 additions and 5 deletions.
23 changes: 22 additions & 1 deletion docs/rules/jsx-sort-props.md
Expand Up @@ -28,7 +28,8 @@ The following patterns are considered okay and do not cause warnings:
"shorthandFirst": <boolean>,
"shorthandLast": <boolean>,
"ignoreCase": <boolean>,
"noSortAlphabetically": <boolean>
"noSortAlphabetically": <boolean>,
"reservedFirst": <boolean>|<array<string>>,
}]
...
```
Expand Down Expand Up @@ -75,6 +76,26 @@ When `true`, alphabetical order is not enforced:
<Hello tel={5555555} name="John" />
```

### `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
<Hello key={0} ref="John" name="John">
<div dangerouslySetInnerHTML={{__html: 'ESLint Plugin React!'}} ref="dangerDiv" />
</Hello>
```

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
<Hello key={'uuid'} name="John" ref="ref" />
```

## 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.
114 changes: 112 additions & 2 deletions lib/rules/jsx-sort-props.js
Expand Up @@ -4,6 +4,7 @@
*/
'use strict';

var elementType = require('jsx-ast-utils/elementType');
var propName = require('jsx-ast-utils/propName');

// ------------------------------------------------------------------------------
Expand All @@ -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<String>} 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: {
Expand Down Expand Up @@ -44,6 +111,9 @@ module.exports = {
// Whether alphabetical sorting should be enforced
noSortAlphabetically: {
type: 'boolean'
},
reservedFirst: {
type: ['array', 'boolean']
}
},
additionalProperties: false
Expand All @@ -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];
Expand All @@ -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;
Expand Down
110 changes: 108 additions & 2 deletions tests/lib/rules/jsx-sort-props.js
Expand Up @@ -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
}];
Expand All @@ -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: [
Expand Down Expand Up @@ -93,7 +119,38 @@ ruleTester.run('jsx-sort-props', rule, {
},
// noSortAlphabetically
{code: '<App a b />;', options: noSortAlphabeticallyArgs, parserOptions: parserOptions},
{code: '<App b a />;', options: noSortAlphabeticallyArgs, parserOptions: parserOptions}
{code: '<App b a />;', options: noSortAlphabeticallyArgs, parserOptions: parserOptions},
// reservedFirst
{
code: '<App children={<App />} key={0} ref="r" a b c />',
options: reservedFirstAsBooleanArgs,
parserOptions: parserOptions
},
{
code: '<App children={<App />} key={0} ref="r" a b c dangerouslySetInnerHTML={{__html: "EPR"}} />',
options: reservedFirstAsBooleanArgs,
parserOptions: parserOptions
},
{
code: '<App children={<App />} key={0} a ref="r" />',
options: reservedFirstAsArrayArgs,
parserOptions: parserOptions
},
{
code: '<App children={<App />} key={0} a dangerouslySetInnerHTML={{__html: "EPR"}} ref="r" />',
options: reservedFirstAsArrayArgs,
parserOptions: parserOptions
},
{
code: '<App ref="r" key={0} children={<App />} b a c />',
options: reservedFirstWithNoSortAlphabeticallyArgs,
parserOptions: parserOptions
},
{
code: '<div ref="r" dangerouslySetInnerHTML={{__html: "EPR"}} key={0} children={<App />} b a c />',
options: reservedFirstWithNoSortAlphabeticallyArgs,
parserOptions: parserOptions
}
],
invalid: [
{code: '<App b a />;', errors: [expectedError], parserOptions: parserOptions},
Expand Down Expand Up @@ -132,6 +189,55 @@ ruleTester.run('jsx-sort-props', rule, {
options: shorthandLastArgs,
parserOptions: parserOptions
},
{code: '<App b a />;', errors: [expectedError], options: sortAlphabeticallyArgs, parserOptions: parserOptions}
{code: '<App b a />;', errors: [expectedError], options: sortAlphabeticallyArgs, parserOptions: parserOptions},
// reservedFirst
{
code: '<App a key={1} />',
options: reservedFirstAsBooleanArgs,
errors: [expectedReservedFirstError],
parserOptions: parserOptions
},
{
code: '<div a dangerouslySetInnerHTML={{__html: "EPR"}} />',
options: reservedFirstAsBooleanArgs,
errors: [expectedReservedFirstError],
parserOptions: parserOptions
},
{
code: '<App ref="r" key={2} b />',
options: reservedFirstAsBooleanArgs,
errors: [expectedError],
parserOptions: parserOptions
},
{
code: '<App dangerouslySetInnerHTML={{__html: "EPR"}} key={2} b />',
options: reservedFirstAsBooleanArgs,
errors: [expectedReservedFirstError],
parserOptions: parserOptions
},
{
code: '<App key={3} children={<App />} />',
options: reservedFirstAsArrayArgs,
errors: [expectedError],
parserOptions: parserOptions
},
{
code: '<App z ref="r" />',
options: reservedFirstWithNoSortAlphabeticallyArgs,
errors: [expectedReservedFirstError],
parserOptions: parserOptions
},
{
code: '<App key={4} />',
options: reservedFirstAsEmptyArrayArgs,
errors: [expectedEmptyReservedFirstError],
parserOptions: parserOptions
},
{
code: '<App key={5} />',
options: reservedFirstAsInvalidArrayArgs,
errors: [expectedInvalidReservedFirstError],
parserOptions: parserOptions
}
]
});

0 comments on commit dfe190b

Please sign in to comment.