[Fix] jsx-newline: No newline between comments and jsx elements

jsx-eslint · Nov 17, 2022 · 12e9838 · 12e9838
1 parent 54c56a1
commit 12e9838
Show file tree

Hide file tree

Showing 3 changed files with 171 additions and 1 deletion.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -15,8 +15,10 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
 * configs: avoid legacy config system error ([#3461][] @ljharb)
 * [`sort-prop-types`]: restore autofixing ([#3452][] @ROSSROSALES)
 * [`no-unknown-property`]: do not check `fbs` elements ([#3494][] @brianogilvie)
+* [`jsx-newline`]: No newline between comments and jsx elements ([#3493][] @justmejulian)
 
 [#3494]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3494
+[#3493]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3493
 [#3461]: https://github.com/jsx-eslint/eslint-plugin-react/issues/3461
 [#3452]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3452
 [#3449]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3449

diff --git a/lib/rules/jsx-newline.js b/lib/rules/jsx-newline.js
@@ -72,6 +72,15 @@ module.exports = {
     const jsxElementParents = new Set();
     const sourceCode = context.getSourceCode();
 
+    function isBlockCommentInCurlyBraces(element) {
+      const elementRawValue = sourceCode.getText(element);
+      return /^\s*{\/\*/.test(elementRawValue);
+    }
+
+    function isNonBlockComment(element) {
+      return !isBlockCommentInCurlyBraces(element) && (element.type === 'JSXElement' || element.type === 'JSXExpressionContainer');
+    }
+
     return {
       'Program:exit'() {
         jsxElementParents.forEach((parent) => {
@@ -93,7 +102,15 @@ module.exports = {
               // Check adjacent sibling has the proper amount of newlines
               const isWithoutNewLine = !/\n\s*\n/.test(firstAdjacentSibling.value);
 
-              if (allowMultilines && (isMultilined(element) || isMultilined(secondAdjacentSibling))) {
+              if (isBlockCommentInCurlyBraces(element)) return;
+
+              if (
+                allowMultilines
+                && (
+                  isMultilined(element)
+                  || isMultilined(elements.slice(index + 2).find(isNonBlockComment))
+                )
+              ) {
                 if (!isWithoutNewLine) return;
 
                 const regex = /(\n)(?!.*\1)/g;
@@ -115,6 +132,7 @@ module.exports = {
               }
 
               if (isWithoutNewLine === prevent) return;
+
               const messageId = prevent
                 ? 'prevent'
                 : 'require';

diff --git a/tests/lib/rules/jsx-newline.js b/tests/lib/rules/jsx-newline.js
@@ -105,6 +105,44 @@ new RuleTester({ parserOptions }).run('jsx-newline', rule, {
         </Button>
       `,
     },
+    {
+      code: `
+        <Button popoverOpen='#settings-popover' style={{ width: 'fit-content' }}>
+          {/* fake-eslint-disable-next-line should also work inside a component */}
+          <Icon f7='gear' />
+        </Button>
+      `,
+    },
+    {
+      code: `
+        <Button popoverOpen='#settings-popover' style={{ width: 'fit-content' }}>
+          {/* should work inside a component */}
+          {/* and it should work when using multiple comments */}
+          <Icon f7='gear' />
+        </Button>
+      `,
+    },
+    {
+      code: `
+        <Button popoverOpen='#settings-popover' style={{ width: 'fit-content' }}>
+          {/* this is a multiline
+              block comment */}
+          <Icon f7='gear' />
+        </Button>
+      `,
+    },
+    {
+      code: `
+        <>
+          {/* does this */}
+          <Icon f7='gear' />
+
+          {/* also work with multiple components and inside a fragment? */}
+          <OneLineComponent />
+        </>
+      `,
+      features: ['fragment'],
+    },
     {
       code: `
         <>
@@ -122,6 +160,24 @@ new RuleTester({ parserOptions }).run('jsx-newline', rule, {
       features: ['fragment'],
       options: [{ prevent: true, allowMultilines: true }],
     },
+    {
+      code: `
+        <div>
+          {/* this does not have a newline */}
+          <Icon f7='gear' />
+          {/* neither does this */}
+          <OneLineComponent />
+
+          {/* but this one needs one */}
+          <Button>
+            <IconPreview />
+            Button 2
+            <span></span>
+          </Button>
+        </div>
+      `,
+      options: [{ prevent: true, allowMultilines: true }],
+    },
     {
       code: `
         <div>
@@ -223,6 +279,100 @@ new RuleTester({ parserOptions }).run('jsx-newline', rule, {
       `,
       errors: [{ messageId: 'require' }],
     },
+    {
+      code: `
+        <div>
+          {/* This should however still not work*/}
+          <Icon f7='gear' />
+
+          <OneLineComponent />
+          {/* Comments between components still need a newLine */}
+          <OneLineComponent />
+        </div>
+      `,
+      output: `
+        <div>
+          {/* This should however still not work*/}
+          <Icon f7='gear' />
+
+          <OneLineComponent />
+
+          {/* Comments between components still need a newLine */}
+          <OneLineComponent />
+        </div>
+      `,
+      errors: [{ messageId: 'require' }],
+    },
+    {
+      code: `
+        <div>
+          {/* this does not have a newline */}
+          <Icon f7='gear' />
+          {/* neither does this */}
+          <OneLineComponent />
+          {/* but this one needs one */}
+          <Button>
+            <IconPreview />
+            Button 2
+            <span></span>
+          </Button>
+        </div>
+      `,
+      output: `
+        <div>
+          {/* this does not have a newline */}
+          <Icon f7='gear' />
+          {/* neither does this */}
+          <OneLineComponent />
+
+          {/* but this one needs one */}
+          <Button>
+            <IconPreview />
+            Button 2
+            <span></span>
+          </Button>
+        </div>
+      `,
+      options: [{ prevent: true, allowMultilines: true }],
+      errors: [{ messageId: 'allowMultilines' }],
+    },
+    {
+      code: `
+        <div>
+          {/* this does not have a newline */}
+          <Icon f7='gear' />
+          {/* neither does this */}
+          <OneLineComponent />
+          {/* Multiline */}
+          {/* Block comments */}
+          {/* Stick to MultilineComponent */}
+          <Button>
+            <IconPreview />
+            Button 2
+            <span></span>
+          </Button>
+        </div>
+      `,
+      output: `
+        <div>
+          {/* this does not have a newline */}
+          <Icon f7='gear' />
+          {/* neither does this */}
+          <OneLineComponent />
+
+          {/* Multiline */}
+          {/* Block comments */}
+          {/* Stick to MultilineComponent */}
+          <Button>
+            <IconPreview />
+            Button 2
+            <span></span>
+          </Button>
+        </div>
+      `,
+      options: [{ prevent: true, allowMultilines: true }],
+      errors: [{ messageId: 'allowMultilines' }],
+    },
     {
       code: `
         <div>