Update jsx-sort-props to allow sorting callbacks last

jsx-eslint · Oct 10, 2015 · 4e71dbf · 4e71dbf
1 parent a7d75cf
commit 4e71dbf
Show file tree

Hide file tree

Showing 3 changed files with 58 additions and 7 deletions.
diff --git a/docs/rules/jsx-sort-props.md b/docs/rules/jsx-sort-props.md
@@ -38,6 +38,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-props.js b/lib/rules/jsx-sort-props.js
@@ -8,27 +8,53 @@
 // 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) {
+      // Whether we have reached the callbacks section of the props. Only used
+      // when the `callbacksLast` setting is used.
+      var inCallbacks = false;
+
       node.attributes.reduce(function(memo, decl, idx, attrs) {
         if (decl.type === 'JSXSpreadAttribute') {
           return attrs[idx + 1];
         }
 
-        var lastPropName = memo.name.name;
-        var currenPropName = decl.name.name;
+        var previousPropName = memo.name.name;
+        var currentPropName = decl.name.name;
+        if (callbacksLast) {
+          var previousIsCallback = isCallbackPropName(previousPropName);
+          var currentIsCallback = isCallbackPropName(currentPropName);
+        }
 
         if (ignoreCase) {
-          lastPropName = lastPropName.toLowerCase();
-          currenPropName = currenPropName.toLowerCase();
+          previousPropName = previousPropName.toLowerCase();
+          currentPropName = currentPropName.toLowerCase();
         }
 
-        if (currenPropName < lastPropName) {
+        if (callbacksLast) {
+          if (!previousIsCallback && currentIsCallback) {
+            // Entering the callback prop section
+            inCallbacks = true;
+            return decl;
+          }
+          if (previousIsCallback && !currentIsCallback) {
+            // Encountered a non-callback prop after a callback prop
+            context.report(decl, 'Callbacks must be listed after all other props');
+            return memo;
+          }
+        }
+
+        if (currentPropName < previousPropName) {
           context.report(decl, 'Props should be sorted alphabetically');
           return memo;
         }
@@ -42,6 +68,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-props.js b/tests/lib/rules/jsx-sort-props.js
@@ -22,6 +22,13 @@ var expectedError = {
   message: 'Props should be sorted alphabetically',
   type: 'JSXAttribute'
 };
+var expectedCallbackError = {
+  message: 'Callbacks must be listed after all other props',
+  type: 'JSXAttribute'
+}
+var callbacksLastArgs = [{
+  callbacksLast: true
+}];
 var ignoreCaseArgs = [{
   ignoreCase: true
 }];
@@ -40,9 +47,12 @@ ruleTester.run('jsx-sort-props', rule, {
     {code: '<App {...this.props} a="c" b="b" c="a" />;', ecmaFeatures: features},
     {code: '<App c="a" {...this.props} a="c" b="b" />;', ecmaFeatures: features},
     {code: '<App A a />;', ecmaFeatures: features},
+    // Ignoring case
     {code: '<App a A />;', options: ignoreCaseArgs, ecmaFeatures: features},
     {code: '<App a B c />;', options: ignoreCaseArgs, ecmaFeatures: features},
-    {code: '<App A b C />;', options: ignoreCaseArgs, ecmaFeatures: features}
+    {code: '<App A b C />;', options: ignoreCaseArgs, ecmaFeatures: features},
+    // Sorting callbacks below all other props
+    {code: '<App a z onBar onFoo />;', options: callbacksLastArgs, ecmaFeatures: features}
   ],
   invalid: [
     {code: '<App b a />;', errors: [expectedError], ecmaFeatures: features},
@@ -53,6 +63,8 @@ ruleTester.run('jsx-sort-props', rule, {
     {code: '<App B A c />;', options: ignoreCaseArgs, errors: [expectedError], ecmaFeatures: features},
     {code: '<App c="a" a="c" b="b" />;', errors: 2, ecmaFeatures: features},
     {code: '<App {...this.props} c="a" a="c" b="b" />;', errors: 2, ecmaFeatures: features},
-    {code: '<App d="d" b="b" {...this.props} c="a" a="c" />;', errors: 2, ecmaFeatures: features}
+    {code: '<App d="d" b="b" {...this.props} c="a" a="c" />;', errors: 2, ecmaFeatures: features},
+    {code: '<App a z onFoo onBar />;', errors: [expectedError], options: callbacksLastArgs, ecmaFeatures: features},
+    {code: '<App a onBar onFoo z />;', errors: [expectedCallbackError], options: callbacksLastArgs, ecmaFeatures: features}
   ]
 });