-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
feat: jsx-no-newline
: Enforce the removal of newlines around jsx tags
#2927
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like it'd be simpler as an option to https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/jsx-newline.md, rather than an entirely new rule that would conflict if both of the rules were enabled.
Yeah if that is the case some of the code could be reused. Let me see if I can give it a shot. |
lib/rules/jsx-newline.js
Outdated
return fixer.replaceText( | ||
firstAdjacentSibling, | ||
sourceCode.getText(firstAdjacentSibling) | ||
.replace(/\n/g, '') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ljharb This regex looks a bit sketchy. I guess it should just be like one \n
rather than many
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
line endings could also be \r\n
or whichever, i suppose
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah
firstAdjacentSibling | ||
if (configuration.removeNewLines) { | ||
parent.children.forEach((element) => { | ||
if (element.type === 'JSXText') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ljharb I need some help here. My logic for detecting new lines was like iterating through the parents and for each child I am checking if they kind of match a new line. My concern is that the new lines will get counted multiple times as the parser moves through the nodes.
For something like this
<div>
<button></button>
</div>
I tried printing like for each parent what the new lines was and I got something like this
Parent identified
Newline found
Parent identified
Parent identified
Parent identified
Newline found
I am confused like how is the newline found after the last div instead it should be found after the last button right ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In your example, there's 5 newlines, but 2 blank lines - i think what you want to find is a blank line, ie, \n\s*\n
, not a newline, which is \n
?
Enforces a rules that removes empty lines around jsx elements