Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a reservedFirst option to the jsx-sort-props rule #1134

Merged
merged 1 commit into from Apr 28, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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: {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we ensure that additionalProperties are disallowed on the object form? (not sure if line 73 covers this or not)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that it (line 73) does do that. http://eslint.org/docs/developer-guide/working-with-rules

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
}
]
});