Merge pull request #242 from Daniel15/jsx-callbacks-last

Update `jsx-sort-props` to allow sorting callbacks last
jsx-eslint · Oct 14, 2015 · 7bbbda7 · 7bbbda7
2 parents 677c70a + 73d125f
commit 7bbbda7
Show file tree

Hide file tree

Showing 6 changed files with 275 additions and 12 deletions.
diff --git a/docs/rules/jsx-sort-prop-types.md b/docs/rules/jsx-sort-prop-types.md
@@ -78,14 +78,33 @@ class Component extends React.Component {
 
 ```js
 ...
-"jsx-sort-prop-types": [<enabled>, { "ignoreCase": <boolean> }]
+"jsx-sort-prop-types": [<enabled>, {
+  "callbacksLast": <boolean>,
+  "ignoreCase": <boolean>
+}]
 ...
 ```
 
 ### `ignoreCase`
 
 When `true` the rule ignores the case-sensitivity of the declarations order.
 
+### `callbacksLast`
+
+When `true`, prop types for props beginning with "on" must be listed after all other props:
+
+```js
+var Component = React.createClass({
+  propTypes: {
+    a: React.PropTypes.number,
+    z: React.PropTypes.string,
+    onBar: React.PropTypes.func,
+    onFoo: React.PropTypes.func,
+  },
+...
+});
+```
+
 ## 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/docs/rules/jsx-sort-props.md b/docs/rules/jsx-sort-props.md
@@ -24,7 +24,10 @@ The following patterns are considered okay and do not cause warnings:
 
 ```js
 ...
-"jsx-sort-props": [<enabled>, { "ignoreCase": <boolean> }]
+"jsx-sort-props": [<enabled>, {
+  "callbacksLast": <boolean>,
+  "ignoreCase": <boolean>
+}]
 ...
 ```
 
@@ -38,6 +41,14 @@ The following patterns are considered okay and do not cause warnings:
 <Hello name="John" Number="2" />;
 ```
 
+### `callbacksLast`
+
+When `true`, callbacks must be listed after all other props:
+
+```js
+<Hello tel={5555555} onClick={this._handleClick} />
+```
+
 ## 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-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 callbacksLast = configuration.callbacksLast || false;
   var ignoreCase = configuration.ignoreCase || false;
 
   /**
@@ -39,6 +40,10 @@ module.exports = function(context) {
     return node.key.type === 'Identifier' ? node.key.name : node.key.value;
   }
 
+  function isCallbackPropName(propName) {
+    return /^on[A-Z]/.test(propName);
+  }
+
   /**
    * Checks if propTypes declarations are sorted
    * @param {Array} declarations The array of AST nodes being checked.
@@ -47,14 +52,28 @@ module.exports = function(context) {
   function checkSorted(declarations) {
     declarations.reduce(function(prev, curr) {
       var prevPropName = getKey(prev);
-      var currenPropName = getKey(curr);
+      var currentPropName = getKey(curr);
+      var previousIsCallback = isCallbackPropName(prevPropName);
+      var currentIsCallback = isCallbackPropName(currentPropName);
 
       if (ignoreCase) {
         prevPropName = prevPropName.toLowerCase();
-        currenPropName = currenPropName.toLowerCase();
+        currentPropName = currentPropName.toLowerCase();
+      }
+
+      if (callbacksLast) {
+        if (!previousIsCallback && currentIsCallback) {
+          // Entering the callback prop section
+          return curr;
+        }
+        if (previousIsCallback && !currentIsCallback) {
+          // Encountered a non-callback prop after a callback prop
+          context.report(prev, 'Callback prop types must be listed after all other prop types');
+          return prev;
+        }
       }
 
-      if (currenPropName < prevPropName) {
+      if (currentPropName < prevPropName) {
         context.report(curr, 'Prop types declarations should be sorted alphabetically');
         return prev;
       }
@@ -100,6 +119,9 @@ module.exports = function(context) {
 module.exports.schema = [{
   type: 'object',
   properties: {
+    callbacksLast: {
+      type: 'boolean'
+    },
     ignoreCase: {
       type: 'boolean'
     }

diff --git a/lib/rules/jsx-sort-props.js b/lib/rules/jsx-sort-props.js
@@ -8,10 +8,15 @@
 // Rule Definition
 // ------------------------------------------------------------------------------
 
+function isCallbackPropName(propName) {
+  return /^on[A-Z]/.test(propName);
+}
+
 module.exports = function(context) {
 
   var configuration = context.options[0] || {};
   var ignoreCase = configuration.ignoreCase || false;
+  var callbacksLast = configuration.callbacksLast || false;
 
   return {
     JSXOpeningElement: function(node) {
@@ -20,15 +25,29 @@ module.exports = function(context) {
           return attrs[idx + 1];
         }
 
-        var lastPropName = memo.name.name;
-        var currenPropName = decl.name.name;
+        var previousPropName = memo.name.name;
+        var currentPropName = decl.name.name;
+        var previousIsCallback = isCallbackPropName(previousPropName);
+        var currentIsCallback = isCallbackPropName(currentPropName);
 
         if (ignoreCase) {
-          lastPropName = lastPropName.toLowerCase();
-          currenPropName = currenPropName.toLowerCase();
+          previousPropName = previousPropName.toLowerCase();
+          currentPropName = currentPropName.toLowerCase();
+        }
+
+        if (callbacksLast) {
+          if (!previousIsCallback && currentIsCallback) {
+            // Entering the callback prop section
+            return decl;
+          }
+          if (previousIsCallback && !currentIsCallback) {
+            // Encountered a non-callback prop after a callback prop
+            context.report(memo, 'Callbacks must be listed after all other props');
+            return memo;
+          }
         }
 
-        if (currenPropName < lastPropName) {
+        if (currentPropName < previousPropName) {
           context.report(decl, 'Props should be sorted alphabetically');
           return memo;
         }
@@ -42,6 +61,11 @@ module.exports = function(context) {
 module.exports.schema = [{
   type: 'object',
   properties: {
+    // Whether callbacks (prefixed with "on") should be listed at the very end,
+    // after all other props.
+    callbacksLast: {
+      type: 'boolean'
+    },
     ignoreCase: {
       type: 'boolean'
     }

diff --git a/tests/lib/rules/jsx-sort-prop-types.js b/tests/lib/rules/jsx-sort-prop-types.js
@@ -208,6 +208,69 @@ ruleTester.run('jsx-sort-prop-types', rule, {
       experimentalObjectRestSpread: true,
       jsx: true
     }
+  }, {
+    code: [
+      'var First = React.createClass({',
+      '  propTypes: {',
+      '    a: React.PropTypes.any,',
+      '    z: React.PropTypes.string,',
+      '    onBar: React.PropTypes.func,',
+      '    onFoo: React.PropTypes.func',
+      '  },',
+      '  render: function() {',
+      '    return <div />;',
+      '  }',
+      '});'
+    ].join('\n'),
+    options: [{
+      callbacksLast: true
+    }],
+    ecmaFeatures: {
+      jsx: true
+    }
+  }, {
+    code: [
+      'class Component extends React.Component {',
+      '  static propTypes = {',
+      '    a: React.PropTypes.any,',
+      '    z: React.PropTypes.string,',
+      '    onBar: React.PropTypes.func,',
+      '    onFoo: React.PropTypes.func',
+      '  }',
+      '  render() {',
+      '    return <div />;',
+      '  }',
+      '}'
+    ].join('\n'),
+    options: [{
+      callbacksLast: true
+    }],
+    parser: 'babel-eslint',
+    ecmaFeatures: {
+      classes: true,
+      jsx: true
+    }
+  }, {
+    code: [
+      'class First extends React.Component {',
+      '  render() {',
+      '    return <div />;',
+      '  }',
+      '}',
+      'First.propTypes = {',
+      '    a: React.PropTypes.any,',
+      '    z: React.PropTypes.string,',
+      '    onBar: React.PropTypes.func,',
+      '    onFoo: React.PropTypes.func',
+      '};'
+    ].join('\n'),
+    options: [{
+      callbacksLast: true
+    }],
+    ecmaFeatures: {
+      classes: true,
+      jsx: true
+    }
   }],
 
   invalid: [{
@@ -364,5 +427,112 @@ ruleTester.run('jsx-sort-prop-types', rule, {
       jsx: true
     },
     errors: 2
+  }, {
+    code: [
+      'var First = React.createClass({',
+      '  propTypes: {',
+      '    a: React.PropTypes.any,',
+      '    z: React.PropTypes.string,',
+      '    onFoo: React.PropTypes.func,',
+      '    onBar: React.PropTypes.func',
+      '  },',
+      '  render: function() {',
+      '    return <div />;',
+      '  }',
+      '});'
+    ].join('\n'),
+    options: [{
+      callbacksLast: true
+    }],
+    ecmaFeatures: {
+      jsx: true
+    },
+    errors: [{
+      message: ERROR_MESSAGE,
+      line: 6,
+      column: 5,
+      type: 'Property'
+    }]
+  }, {
+    code: [
+      'class Component extends React.Component {',
+      '  static propTypes = {',
+      '    a: React.PropTypes.any,',
+      '    z: React.PropTypes.string,',
+      '    onFoo: React.PropTypes.func,',
+      '    onBar: React.PropTypes.func',
+      '  }',
+      '  render() {',
+      '    return <div />;',
+      '  }',
+      '}'
+    ].join('\n'),
+    options: [{
+      callbacksLast: true
+    }],
+    parser: 'babel-eslint',
+    ecmaFeatures: {
+      classes: true,
+      jsx: true
+    },
+    errors: [{
+      message: ERROR_MESSAGE,
+      line: 6,
+      column: 5,
+      type: 'Property'
+    }]
+  }, {
+    code: [
+      'class First extends React.Component {',
+      '  render() {',
+      '    return <div />;',
+      '  }',
+      '}',
+      'First.propTypes = {',
+      '    a: React.PropTypes.any,',
+      '    z: React.PropTypes.string,',
+      '    onFoo: React.PropTypes.func,',
+      '    onBar: React.PropTypes.func',
+      '};'
+    ].join('\n'),
+    options: [{
+      callbacksLast: true
+    }],
+    ecmaFeatures: {
+      classes: true,
+      jsx: true
+    },
+    errors: [{
+      message: ERROR_MESSAGE,
+      line: 10,
+      column: 5,
+      type: 'Property'
+    }]
+  }, {
+    code: [
+      'var First = React.createClass({',
+      '  propTypes: {',
+      '    a: React.PropTypes.any,',
+      '    onBar: React.PropTypes.func,',
+      '    onFoo: React.PropTypes.func,',
+      '    z: React.PropTypes.string',
+      '  },',
+      '  render: function() {',
+      '    return <div />;',
+      '  }',
+      '});'
+    ].join('\n'),
+    options: [{
+      callbacksLast: true
+    }],
+    ecmaFeatures: {
+      jsx: true
+    },
+    errors: [{
+      message: 'Callback prop types must be listed after all other prop types',
+      line: 5,
+      column: 5,
+      type: 'Property'
+    }]
   }]
 });