jsx-first-pro-new-line autofix implementation #2
Conversation
| @@ -120,6 +120,10 @@ ruleTester.run('jsx-first-prop-new-line', rule, { | |||
| invalid: [ | |||
| { | |||
| code: '<Foo prop="one" />', | |||
| output: [ | |||
| '<Foo', | |||
| ' prop="one" />' | |||
There was a problem hiding this comment.
The indentation looks off (cross ref line 101 and line 111).
There was a problem hiding this comment.
Hi I have been looking at this problem. To solve this, I would have to change the schema to intake the indent size. Would this be ok?
On a side note, jsx-indent-props solves this properly but would need a second run.
- Adjust old cases to use default 2 indents - Add new case to use optional rule for 4 indents
Utilised getNodeIndent helper from jsx-indent-props as base-code
|
Indentation has been fixed. A new test case has been added to showcase ability to specify indentation style. |
| @@ -120,6 +120,10 @@ ruleTester.run('jsx-first-prop-new-line', rule, { | |||
| invalid: [ | |||
| { | |||
| code: '<Foo prop="one" />', | |||
| output: [ | |||
There was a problem hiding this comment.
Pls add test cases for tabs too. Thanks.
| }, | ||
|
|
||
| create: function (context) { | ||
| var configuration = context.options[0]; | ||
| var extraColumnStart = 0; |
| byLastLine = byLastLine || false; | ||
| excludeCommas = excludeCommas || false; | ||
|
|
||
| var src = sourceCode.getText(node, node.loc.start.column + extraColumnStart); |
| message: 'Property should be placed on a new line' | ||
| message: 'Property should be placed on a new line', | ||
| fix: function(fixer) { | ||
| var nodeIndent = getNodeIndent(node, false, false); |
There was a problem hiding this comment.
getNodeIndent's 2nd and 3rd arguments are default to false, can remove here.
| } | ||
|
|
||
| function getNodeIndent(node, byLastLine, excludeCommas) { | ||
| byLastLine = byLastLine || false; |
There was a problem hiding this comment.
getNodeIndent is only called in one place, and called with byLastLine and excludeCommas set to false. Can just remove these 2 variables and short circuit their values into this function to make it simpler.
cut out unused vars based on feedback
|
function amended as requested. |
|
We will contact you via email for payment matters. Pls submit a PR upstream as well, for the benefit of everyone else. Thanks. :-) |
|
Forgot to mention that you should also update |
|
Are you guys still working on this? There are some teams across the globe that could really benefit from a --fix command for this eslint rule. Let me know if you need help. @snowypowers @shioju @yangshun @gyng @lawliet89 |
|
yes @revisionfour, but right now it can't handle the case where the first line has more than one prop. gonna fix that then push. thanks for asking. |
First submission of autofix