Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from Nov 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -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
Expand Down
20 changes: 19 additions & 1 deletion lib/rules/jsx-newline.js
Expand Up @@ -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) => {
Expand All @@ -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;
Expand All @@ -115,6 +132,7 @@ module.exports = {
}

if (isWithoutNewLine === prevent) return;

const messageId = prevent
? 'prevent'
: 'require';
Expand Down
150 changes: 150 additions & 0 deletions tests/lib/rules/jsx-newline.js
Expand Up @@ -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: `
<>
Expand All @@ -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>
Expand Down Expand Up @@ -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>
Expand Down