Update jsx-sort-prop-types to allow sorting callbacks last

jsx-eslint · Oct 10, 2015 · 73d125f · 73d125f
1 parent 719764b
commit 73d125f
Show file tree

Hide file tree

Showing 5 changed files with 220 additions and 6 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>
+}]
 ...
 ```
 

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
@@ -42,7 +42,7 @@ module.exports = function(context) {
           }
           if (previousIsCallback && !currentIsCallback) {
             // Encountered a non-callback prop after a callback prop
-            context.report(decl, 'Callbacks must be listed after all other props');
+            context.report(memo, 'Callbacks must be listed after all other props');
             return memo;
           }
         }

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