Repository: yannickcr/eslint-plugin-react
Pull request #535 (2016-04-07 20:06:42)

Add a new rule to ensure position of first property in JSX
In our project we want to use the AirBnB React Style guidelines when it comes to React/JSX code style. One of the rules for JSX is to align components as such:

// bad
```js
<Foo superLongParam="bar"
     anotherSuperLongParam="baz" />
```

// good
```js
<Foo
  superLongParam="bar"
  anotherSuperLongParam="baz"
/>
```

However, I could not find a rule which enforces that the first property should be placed on a new line, in the case of a multiline JSX component.

As an experiment, I decided to try and implement this myself. The result is this PR.

I decided to make three options, although not sure if the other two are really relevant:
`always`: The first prop should always be on a new line, even when there is only 1 property.
`never`: The first prop should never be on a new line, so should always be on the same line as the opening component tag.
`multiline`: Follows AirBnB style. In the case of a multiline component the first prop should be on a new line, otherwise when all props are on a single line then this is allowed.

Please have a look at the code, tests, style, etc... If you have any comments or tips on how to improve this, please let me know. I'd love to see this make it to the project, as it already helped me find a few places which I missed out during my initial refactor to this AirBnB style.

jseminck: Seems like there is already an open PR: https://github.com/yannickcr/eslint-plugin-react/pull/427

And there are other issues asking for the same functionality: https://github.com/yannickcr/eslint-plugin-react/issues/410

So would be nice to include this one way or another. I'm up for trying to improve this specific implementation or helping out with the other PR if needed.
ljharb: It would be super ideal if one of these approaches made it in ASAP. I'd love to include it in the next release of the eslint plugin, which will be a semver-major anyways. (@yannickcr)
coveralls: 
[![Coverage Status](https://coveralls.io/builds/5713791/badge)](https://coveralls.io/builds/5713791)

Coverage increased (+0.03%) to 96.973% when pulling **76f2096539c4e9f16a3fff3e237459db880b44cf on jseminck:master** into **0efb7ad6f1646ef4686d437538f0ba2ff3b217d8 on yannickcr:master**.


Patch:
diff --git a/README.md b/README.md
index b9e0e4ae..61479bc4 100644
--- a/README.md
+++ b/README.md
@@ -102,6 +102,7 @@ The plugin has a [recommended configuration](#user-content-recommended-configura
 * [jsx-closing-bracket-location](docs/rules/jsx-closing-bracket-location.md): Validate closing bracket location in JSX (fixable)
 * [jsx-curly-spacing](docs/rules/jsx-curly-spacing.md): Enforce or disallow spaces inside of curly braces in JSX attributes (fixable)
 * [jsx-equals-spacing](docs/rules/jsx-equals-spacing.md): Enforce or disallow spaces around equal signs in JSX attributes (fixable)
+* [jsx-first-prop-new-line](docs/rules/jsx-first-prop-new-line.md): Enforce position of the first prop in JSX
 * [jsx-handler-names](docs/rules/jsx-handler-names.md): Enforce event handler naming conventions in JSX
 * [jsx-indent-props](docs/rules/jsx-indent-props.md): Validate props indentation in JSX (fixable)
 * [jsx-indent](docs/rules/jsx-indent.md): Validate JSX indentation
diff --git a/docs/rules/jsx-first-prop-new-line.md b/docs/rules/jsx-first-prop-new-line.md
new file mode 100644
index 00000000..0a6a6213
--- /dev/null
+++ b/docs/rules/jsx-first-prop-new-line.md
@@ -0,0 +1,74 @@
+# Configure the position of the first property (jsx-first-prop-new-line)
+
+Ensure correct position of the first property.
+
+## Rule Details
+
+This rule checks whether the first property of all JSX elements is correctly placed. There are three possible configurations:
+* `always`: The first property should always be placed on a new line.
+* `never` : The first property should never be placed on a new line, e.g. should always be on the same line as the Component opening tag.
+* `multiline`: The first property should always be placed on a new line when the properties are spread over multiple lines.
+
+The following patterns are considered warnings when configured `"always"`:
+
+```js
+<Hello personal={true} />
+
+<Hello personal={true}
+    foo="bar"
+/>
+```
+
+The following patterns are not considered warnings when configured `"always"`:
+
+```js
+<Hello
+    personal />
+
+<Hello
+    personal
+/>
+```
+
+The following patterns are considered warnings when configured `"never"`:
+
+```js
+<Hello
+    personal />
+
+<Hello
+    personal
+/>
+```
+
+The following patterns are not considered warnings when configured `"never"`:
+
+```js
+<Hello personal={true} />
+
+<Hello personal={true}
+    foo="bar"
+/>
+```
+
+The following patterns are considered warnings when configured `"multiline"`:
+
+```js
+<Hello personal
+    prop />
+```
+
+The following patterns are not considered warnings when configured `"multiline"`:
+
+```js
+<Hello personal={true} />
+
+<Hello
+    personal={true}
+    foo="bar"
+/>
+```
+
+## When not to use
+
+If you are not using JSX then you can disable this rule.
diff --git a/index.js b/index.js
index effb5ae9..78eebaf9 100644
--- a/index.js
+++ b/index.js
@@ -42,7 +42,8 @@ module.exports = {
     'jsx-key': require('./lib/rules/jsx-key'),
     'no-string-refs': require('./lib/rules/no-string-refs'),
     'prefer-stateless-function': require('./lib/rules/prefer-stateless-function'),
-    'require-render-return': require('./lib/rules/require-render-return')
+    'require-render-return': require('./lib/rules/require-render-return'),
+    'jsx-first-prop-new-line': require('./lib/rules/jsx-first-prop-new-line')
   },
   configs: {
     recommended: {
diff --git a/lib/rules/jsx-first-prop-new-line.js b/lib/rules/jsx-first-prop-new-line.js
new file mode 100644
index 00000000..d9fb0502
--- /dev/null
+++ b/lib/rules/jsx-first-prop-new-line.js
@@ -0,0 +1,46 @@
+/**
+ * @fileoverview Ensure proper position of the first property in JSX
+ * @author Joachim Seminck
+ */
+'use strict';
+
+// ------------------------------------------------------------------------------
+// Rule Definition
+// ------------------------------------------------------------------------------
+
+module.exports = function (context) {
+  var configuration = context.options[0];
+
+  function isMultilineJSX(jsxNode) {
+    return jsxNode.loc.start.line < jsxNode.loc.end.line;
+  }
+
+  return {
+    JSXOpeningElement: function (node) {
+      if ((configuration === 'multiline' && isMultilineJSX(node)) || (configuration === 'always')) {
+        node.attributes.forEach(function(decl) {
+          if (decl.loc.start.line === node.loc.start.line) {
+            context.report({
+              node: decl,
+              message: 'Property should be placed on a new line'
+            });
+          }
+        });
+      } else if (configuration === 'never' && node.attributes.length > 0) {
+        var firstNode = node.attributes[0];
+        if (node.loc.start.line < firstNode.loc.start.line) {
+          context.report({
+            node: firstNode,
+            message: 'Property should be placed on the same line as the component declaration'
+          });
+          return;
+        }
+      }
+      return;
+    }
+  };
+};
+
+module.exports.schema = [{
+  enum: ['always', 'never', 'multiline']
+}];
diff --git a/lib/rules/no-deprecated.js b/lib/rules/no-deprecated.js
index 959858e9..1f4fb02b 100644
--- a/lib/rules/no-deprecated.js
+++ b/lib/rules/no-deprecated.js
@@ -48,6 +48,12 @@ module.exports = function(context) {
     deprecated.MemberExpression[pragma + '.findDOMNode'] = ['0.14.0', 'ReactDOM.findDOMNode'];
     deprecated.MemberExpression[pragma + '.renderToString'] = ['0.14.0', 'ReactDOMServer.renderToString'];
     deprecated.MemberExpression[pragma + '.renderToStaticMarkup'] = ['0.14.0', 'ReactDOMServer.renderToStaticMarkup'];
+    // 15.0.0
+    deprecated.MemberExpression[pragma + '.addons.LinkedStateMixin'] = ['15.0.0'];
+    deprecated.MemberExpression['ReactPerf.printDOM'] = ['15.0.0', 'ReactPerf.printOperations'];
+    deprecated.MemberExpression['Perf.printDOM'] = ['15.0.0', 'Perf.printOperations'];
+    deprecated.MemberExpression['ReactPerf.getMeasurementsSummaryMap'] = ['15.0.0', 'ReactPerf.getWasted'];
+    deprecated.MemberExpression['Perf.getMeasurementsSummaryMap'] = ['15.0.0', 'Perf.getWasted'];
 
     return deprecated;
   }
diff --git a/lib/rules/no-unknown-property.js b/lib/rules/no-unknown-property.js
index b54eee86..9a5f5636 100644
--- a/lib/rules/no-unknown-property.js
+++ b/lib/rules/no-unknown-property.js
@@ -4,6 +4,8 @@
  */
 'use strict';
 
+var versionUtil = require('../util/version');
+
 // ------------------------------------------------------------------------------
 // Constants
 // ------------------------------------------------------------------------------
@@ -11,12 +13,13 @@
 var UNKNOWN_MESSAGE = 'Unknown property \'{{name}}\' found, use \'{{standardName}}\' instead';
 
 var DOM_ATTRIBUTE_NAMES = {
-  // Standard
   'accept-charset': 'acceptCharset',
   class: 'className',
   for: 'htmlFor',
-  'http-equiv': 'httpEquiv',
-  // SVG
+  'http-equiv': 'httpEquiv'
+};
+
+var SVGDOM_ATTRIBUTE_NAMES = {
   'clip-path': 'clipPath',
   'fill-opacity': 'fillOpacity',
   'font-family': 'fontFamily',
@@ -87,13 +90,18 @@ function isTagName(node) {
 
 /**
  * Get the standard name of the attribute.
+ * @param {Object} context The current rule context.
  * @param {String} name - Name of the attribute.
  * @returns {String} The standard name of the attribute.
  */
-function getStandardName(name) {
+function getStandardName(context, name) {
   if (DOM_ATTRIBUTE_NAMES[name]) {
     return DOM_ATTRIBUTE_NAMES[name];
   }
+  // Also check for SVG attribute for React <15
+  if (!versionUtil.test(context, '15.0.0') && SVGDOM_ATTRIBUTE_NAMES[name]) {
+    return SVGDOM_ATTRIBUTE_NAMES[name];
+  }
   var i;
   var found = DOM_PROPERTY_NAMES.some(function(element, index) {
     i = index;
@@ -114,7 +122,7 @@ module.exports = function(context) {
 
     JSXAttribute: function(node) {
       var name = sourceCode.getText(node.name);
-      var standardName = getStandardName(name);
+      var standardName = getStandardName(context, name);
       if (!isTagName(node) || !standardName) {
         return;
       }
diff --git a/lib/rules/prefer-stateless-function.js b/lib/rules/prefer-stateless-function.js
index 296a1ab6..c0727df4 100644
--- a/lib/rules/prefer-stateless-function.js
+++ b/lib/rules/prefer-stateless-function.js
@@ -7,6 +7,7 @@
 'use strict';
 
 var Components = require('../util/Components');
+var versionUtil = require('../util/version');
 
 // ------------------------------------------------------------------------------
 // Rule Definition
@@ -209,7 +210,15 @@ module.exports = Components.detect(function(context, components, utils) {
         }
         scope = scope.upper;
       }
-      if (!blockNode || !blockNode.key || blockNode.key.name !== 'render' || utils.isReturningJSX(node, true)) {
+      var isRender = blockNode && blockNode.key && blockNode.key.name === 'render';
+      var allowNull = versionUtil.test(context, '15.0.0'); // Stateless components can return null since React 15
+      var isReturningJSX = utils.isReturningJSX(node, !allowNull);
+      var isReturningNull = node.argument && (node.argument.value === null || node.argument.value === false);
+      if (
+        !isRender ||
+        (allowNull && (isReturningJSX || isReturningNull)) ||
+        (!allowNull && isReturningJSX)
+      ) {
         return;
       }
       markReturnAsInvalid(node);


Test patch:
diff --git a/tests/lib/rules/jsx-first-prop-new-line.js b/tests/lib/rules/jsx-first-prop-new-line.js
new file mode 100644
index 00000000..2fd2dab2
--- /dev/null
+++ b/tests/lib/rules/jsx-first-prop-new-line.js
@@ -0,0 +1,149 @@
+/**
+ * @fileoverview Ensure proper position of the first property in JSX
+ * @author Joachim Seminck
+ */
+'use strict';
+
+// -----------------------------------------------------------------------------
+// Requirements
+// -----------------------------------------------------------------------------
+
+var rule = require('../../../lib/rules/jsx-first-prop-new-line');
+var RuleTester = require('eslint').RuleTester;
+
+var parserOptions = 'babel-eslint';
+
+// -----------------------------------------------------------------------------
+// Tests
+// -----------------------------------------------------------------------------
+
+var ruleTester = new RuleTester();
+ruleTester.run('jsx-first-prop-new-line', rule, {
+
+  valid: [
+    {
+      code: '<Foo />',
+      options: ['never'],
+      parser: parserOptions
+    },
+    {
+      code: '<Foo prop="bar" />',
+      options: ['never'],
+      parser: parserOptions
+    },
+    {
+      code: '<Foo {...this.props} />',
+      options: ['never'],
+      parser: parserOptions
+    },
+    {
+      code: '<Foo a a a />',
+      options: ['never'],
+      parser: parserOptions
+    },
+    {
+      code: [
+        '<Foo a',
+        '  b ',
+        '/>'
+      ].join('\n'),
+      options: ['never'],
+      parser: parserOptions
+    },
+    {
+      code: '<Foo />',
+      options: ['multiline'],
+      parser: parserOptions
+    },
+    {
+      code: '<Foo prop="one" />',
+      options: ['multiline'],
+      parser: parserOptions
+    },
+    {
+      code: '<Foo {...this.props} />',
+      options: ['multiline'],
+      parser: parserOptions
+    },
+    {
+      code: '<Foo a a a />',
+      options: ['multiline'],
+      parser: parserOptions
+    },
+    {
+      code: [
+        '<Foo',
+        '  propOne="one"',
+        '  propTwo="two"',
+        '/>'
+      ].join('\n'),
+      options: ['multiline'],
+      parser: parserOptions
+    },
+    {
+      code: [
+        '<Foo',
+        '  {...this.props}',
+        '  propTwo="two"',
+        '/>'
+      ].join('\n'),
+      options: ['multiline'],
+      parser: parserOptions
+    },
+    {
+      code: '<Foo />',
+      options: ['always'],
+      parser: parserOptions
+    },
+    {
+      code: [
+        '<Foo',
+        '  propOne="one"',
+        '  propTwo="two"',
+        '/>'
+      ].join('\n'),
+      options: ['always'],
+      parser: parserOptions
+    },
+    {
+      code: [
+        '<Foo',
+        '  {...this.props}',
+        '  propTwo="two"',
+        '/>'
+      ].join('\n'),
+      options: ['always'],
+      parser: parserOptions
+    }
+  ],
+
+  invalid: [
+    {
+      code: '<Foo prop="one" />',
+      options: ['always'],
+      errors: [{message: 'Property should be placed on a new line'}],
+      parser: parserOptions
+    },
+    {
+      code: [
+        '<Foo propOne="one"',
+        '  propTwo="two"',
+        '/>'
+      ].join('\n'),
+      options: ['always'],
+      errors: [{message: 'Property should be placed on a new line'}],
+      parser: parserOptions
+    },
+    {
+      code: [
+        '<Foo',
+        ' propOne="one"',
+        ' propTwo="two"',
+        '/>'
+      ].join('\n'),
+      options: ['never'],
+      errors: [{message: 'Property should be placed on the same line as the component declaration'}],
+      parser: parserOptions
+    }
+  ]
+});
diff --git a/tests/lib/rules/no-unknown-property.js b/tests/lib/rules/no-unknown-property.js
index c0fcef6b..920700f9 100644
--- a/tests/lib/rules/no-unknown-property.js
+++ b/tests/lib/rules/no-unknown-property.js
@@ -36,7 +36,9 @@ ruleTester.run('no-unknown-property', rule, {
     {code: '<div data-foo="bar"></div>;', parserOptions: parserOptions},
     {code: '<div class="foo" is="my-elem"></div>;', parserOptions: parserOptions},
     {code: '<div {...this.props} class="foo" is="my-elem"></div>;', parserOptions: parserOptions},
-    {code: '<atom-panel class="foo"></atom-panel>;', parserOptions: parserOptions}
+    {code: '<atom-panel class="foo"></atom-panel>;', parserOptions: parserOptions},
+    {code: '<use xlink:href="bar" />;', parserOptions: parserOptions},
+    {code: '<rect clip-path="bar" />;', parserOptions: parserOptions}
   ],
   invalid: [{
     code: '<div class="bar"></div>;',
@@ -77,11 +79,21 @@ ruleTester.run('no-unknown-property', rule, {
     code: '<use xlink:href="bar" />;',
     output: '<use xlinkHref="bar" />;',
     errors: [{message: 'Unknown property \'xlink:href\' found, use \'xlinkHref\' instead'}],
-    parserOptions: parserOptions
+    parserOptions: parserOptions,
+    settings: {
+      react: {
+        version: '0.14.0'
+      }
+    }
   }, {
     code: '<rect clip-path="bar" />;',
     output: '<rect clipPath="bar" />;',
     errors: [{message: 'Unknown property \'clip-path\' found, use \'clipPath\' instead'}],
-    parserOptions: parserOptions
+    parserOptions: parserOptions,
+    settings: {
+      react: {
+        version: '0.14.0'
+      }
+    }
   }]
 });
diff --git a/tests/lib/rules/prefer-stateless-function.js b/tests/lib/rules/prefer-stateless-function.js
index 515c755d..c37b47ea 100644
--- a/tests/lib/rules/prefer-stateless-function.js
+++ b/tests/lib/rules/prefer-stateless-function.js
@@ -154,7 +154,7 @@ ruleTester.run('prefer-stateless-function', rule, {
       ].join('\n'),
       parser: 'babel-eslint'
     }, {
-      // Can return null (ES6)
+      // Can return null (ES6, React 0.14.0)
       code: [
         'class Foo extends React.Component {',
         '  render() {',
@@ -165,9 +165,14 @@ ruleTester.run('prefer-stateless-function', rule, {
         '  }',
         '}'
       ].join('\n'),
-      parser: 'babel-eslint'
+      parser: 'babel-eslint',
+      settings: {
+        react: {
+          version: '0.14.0'
+        }
+      }
     }, {
-      // Can return null (ES5)
+      // Can return null (ES5, React 0.14.0)
       code: [
         'var Foo = React.createClass({',
         '  render: function() {',
@@ -178,9 +183,14 @@ ruleTester.run('prefer-stateless-function', rule, {
         '  }',
         '});'
       ].join('\n'),
-      parserOptions: parserOptions
+      parserOptions: parserOptions,
+      settings: {
+        react: {
+          version: '0.14.0'
+        }
+      }
     }, {
-      // Can return null (shorthand if in return)
+      // Can return null (shorthand if in return, React 0.14.0)
       code: [
         'class Foo extends React.Component {',
         '  render() {',
@@ -188,7 +198,12 @@ ruleTester.run('prefer-stateless-function', rule, {
         '  }',
         '}'
       ].join('\n'),
-      parser: 'babel-eslint'
+      parser: 'babel-eslint',
+      settings: {
+        react: {
+          version: '0.14.0'
+        }
+      }
     }, {
       code: [
         'export default (Component) => (',
@@ -341,6 +356,51 @@ ruleTester.run('prefer-stateless-function', rule, {
       errors: [{
         message: 'Component should be written as a pure function'
       }]
+    }, {
+      // Can return null (ES6)
+      code: [
+        'class Foo extends React.Component {',
+        '  render() {',
+        '    if (!this.props.foo) {',
+        '      return null;',
+        '    }',
+        '    return <div>{this.props.foo}</div>;',
+        '  }',
+        '}'
+      ].join('\n'),
+      parser: 'babel-eslint',
+      errors: [{
+        message: 'Component should be written as a pure function'
+      }]
+    }, {
+      // Can return null (ES5)
+      code: [
+        'var Foo = React.createClass({',
+        '  render: function() {',
+        '    if (!this.props.foo) {',
+        '      return null;',
+        '    }',
+        '    return <div>{this.props.foo}</div>;',
+        '  }',
+        '});'
+      ].join('\n'),
+      parserOptions: parserOptions,
+      errors: [{
+        message: 'Component should be written as a pure function'
+      }]
+    }, {
+      // Can return null (shorthand if in return)
+      code: [
+        'class Foo extends React.Component {',
+        '  render() {',
+        '    return true ? <div /> : null;',
+        '  }',
+        '}'
+      ].join('\n'),
+      parser: 'babel-eslint',
+      errors: [{
+        message: 'Component should be written as a pure function'
+      }]
     }
   ]
 });