From 4ab1b3e92391a8aa608eba065c2798559f5a924e Mon Sep 17 00:00:00 2001 From: Chris Laskey Date: Tue, 12 Jan 2016 15:04:21 -0500 Subject: [PATCH 1/2] Add `requiredFirst` option to jsx-sort-prop-types --- docs/rules/jsx-sort-prop-types.md | 19 ++++++- lib/rules/jsx-sort-prop-types.js | 26 +++++++++ tests/lib/rules/jsx-sort-prop-types.js | 74 ++++++++++++++++++++++++++ 3 files changed, 118 insertions(+), 1 deletion(-) diff --git a/docs/rules/jsx-sort-prop-types.md b/docs/rules/jsx-sort-prop-types.md index d7539df497..bf419001e5 100644 --- a/docs/rules/jsx-sort-prop-types.md +++ b/docs/rules/jsx-sort-prop-types.md @@ -80,7 +80,8 @@ class Component extends React.Component { ... "jsx-sort-prop-types": [, { "callbacksLast": , - "ignoreCase": + "ignoreCase": , + "requiredFirst": , }] ... ``` @@ -105,6 +106,22 @@ var Component = React.createClass({ }); ``` +### `requiredFirst` + +When `true`, prop types for required props must be listed before all other props: + +```js +var Component = React.createClass({ + propTypes: { + barRequired: React.PropTypes.any.isRequired, + fooRequired: React.PropTypes.any.isRequired, + a: React.PropTypes.number, + z: React.PropTypes.string, + }, +... +}); +``` + ## 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 declarations isn't a part of your coding standards, then you can leave this rule off. diff --git a/lib/rules/jsx-sort-prop-types.js b/lib/rules/jsx-sort-prop-types.js index 8149847afb..2022b93795 100644 --- a/lib/rules/jsx-sort-prop-types.js +++ b/lib/rules/jsx-sort-prop-types.js @@ -10,6 +10,7 @@ module.exports = function(context) { var configuration = context.options[0] || {}; + var requiredFirst = configuration.requiredFirst || false; var callbacksLast = configuration.callbacksLast || false; var ignoreCase = configuration.ignoreCase || false; @@ -38,10 +39,18 @@ module.exports = function(context) { return node.key.type === 'Identifier' ? node.key.name : node.key.value; } + function getValueName(node) { + return node.value.property.name; + } + function isCallbackPropName(propName) { return /^on[A-Z]/.test(propName); } + function isRequiredProp(node) { + return getValueName(node) === 'isRequired'; + } + /** * Checks if propTypes declarations are sorted * @param {Array} declarations The array of AST nodes being checked. @@ -51,6 +60,8 @@ module.exports = function(context) { declarations.reduce(function(prev, curr) { var prevPropName = getKey(prev); var currentPropName = getKey(curr); + var previousIsRequired = isRequiredProp(prev); + var currentIsRequired = isRequiredProp(curr); var previousIsCallback = isCallbackPropName(prevPropName); var currentIsCallback = isCallbackPropName(currentPropName); @@ -59,6 +70,18 @@ module.exports = function(context) { currentPropName = currentPropName.toLowerCase(); } + if (requiredFirst) { + if (previousIsRequired && !currentIsRequired) { + // Transition between required and non-required. Don't compare for alphabetical. + return curr; + } + if (!previousIsRequired && currentIsRequired) { + // Encountered a non-required prop after a required prop + context.report(prev, 'Required prop types must be listed before all other prop types'); + return prev; + } + } + if (callbacksLast) { if (!previousIsCallback && currentIsCallback) { // Entering the callback prop section @@ -117,6 +140,9 @@ module.exports = function(context) { module.exports.schema = [{ type: 'object', properties: { + requiredFirst: { + type: 'boolean' + }, callbacksLast: { type: 'boolean' }, diff --git a/tests/lib/rules/jsx-sort-prop-types.js b/tests/lib/rules/jsx-sort-prop-types.js index 560b267776..998d6583fa 100644 --- a/tests/lib/rules/jsx-sort-prop-types.js +++ b/tests/lib/rules/jsx-sort-prop-types.js @@ -191,6 +191,20 @@ ruleTester.run('jsx-sort-prop-types', rule, { '});' ].join('\n'), parserOptions: parserOptions + }, { + code: [ + 'var First = React.createClass({', + ' propTypes: {', + ' barRequired: React.PropTypes.func.isRequired,', + ' onBar: React.PropTypes.func,', + ' z: React.PropTypes.any', + ' },', + ' render: function() {', + ' return
;', + ' }', + '});' + ].join('\n'), + parserOptions: parserOptions }, { code: [ 'var First = React.createClass({', @@ -246,6 +260,43 @@ ruleTester.run('jsx-sort-prop-types', rule, { callbacksLast: true }], parserOptions: parserOptions + }, { + code: [ + 'class First extends React.Component {', + ' render() {', + ' return
;', + ' }', + '}', + 'First.propTypes = {', + ' barRequired: React.PropTypes.string.isRequired,', + ' a: React.PropTypes.any', + '};' + ].join('\n'), + options: [{ + requiredFirst: true + }], + parserOptions: parserOptions + }, { + code: [ + 'class First extends React.Component {', + ' render() {', + ' return
;', + ' }', + '}', + 'First.propTypes = {', + ' barRequired: React.PropTypes.string.isRequired,', + ' fooRequired: React.PropTypes.any.isRequired,', + ' a: React.PropTypes.any,', + ' z: React.PropTypes.string,', + ' onBar: React.PropTypes.func,', + ' onFoo: React.PropTypes.func', + '};' + ].join('\n'), + options: [{ + requiredFirst: true, + callbacksLast: true + }], + parserOptions: parserOptions }], invalid: [{ @@ -483,5 +534,28 @@ ruleTester.run('jsx-sort-prop-types', rule, { column: 5, type: 'Property' }] + }, { + code: [ + 'var First = React.createClass({', + ' propTypes: {', + ' a: React.PropTypes.any,', + ' barRequired: React.PropTypes.string.isRequired,', + ' onFoo: React.PropTypes.func', + ' },', + ' render: function() {', + ' return
;', + ' }', + '});' + ].join('\n'), + options: [{ + requiredFirst: true + }], + parserOptions: parserOptions, + errors: [{ + message: 'Required prop types must be listed before all other prop types', + line: 3, + column: 5, + type: 'Property' + }] }] }); From ce330d81ad0a4b25f0e80fa1032551985d64fb11 Mon Sep 17 00:00:00 2001 From: Chris Laskey Date: Tue, 12 Jan 2016 15:19:57 -0500 Subject: [PATCH 2/2] Add additional failure test --- lib/rules/jsx-sort-prop-types.js | 4 ++-- tests/lib/rules/jsx-sort-prop-types.js | 25 ++++++++++++++++++++++++- 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/lib/rules/jsx-sort-prop-types.js b/lib/rules/jsx-sort-prop-types.js index 2022b93795..a9e06fabd7 100644 --- a/lib/rules/jsx-sort-prop-types.js +++ b/lib/rules/jsx-sort-prop-types.js @@ -77,8 +77,8 @@ module.exports = function(context) { } if (!previousIsRequired && currentIsRequired) { // Encountered a non-required prop after a required prop - context.report(prev, 'Required prop types must be listed before all other prop types'); - return prev; + context.report(curr, 'Required prop types must be listed before all other prop types'); + return curr; } } diff --git a/tests/lib/rules/jsx-sort-prop-types.js b/tests/lib/rules/jsx-sort-prop-types.js index 998d6583fa..40978fec35 100644 --- a/tests/lib/rules/jsx-sort-prop-types.js +++ b/tests/lib/rules/jsx-sort-prop-types.js @@ -534,6 +534,29 @@ ruleTester.run('jsx-sort-prop-types', rule, { column: 5, type: 'Property' }] + }, { + code: [ + 'var First = React.createClass({', + ' propTypes: {', + ' fooRequired: React.PropTypes.string.isRequired,', + ' barRequired: React.PropTypes.string.isRequired,', + ' a: React.PropTypes.any', + ' },', + ' render: function() {', + ' return
;', + ' }', + '});' + ].join('\n'), + options: [{ + requiredFirst: true + }], + parserOptions: parserOptions, + errors: [{ + message: ERROR_MESSAGE, + line: 4, + column: 5, + type: 'Property' + }] }, { code: [ 'var First = React.createClass({', @@ -553,7 +576,7 @@ ruleTester.run('jsx-sort-prop-types', rule, { parserOptions: parserOptions, errors: [{ message: 'Required prop types must be listed before all other prop types', - line: 3, + line: 4, column: 5, type: 'Property' }]