Update custom fork of indent rule to the new v5 rule #30
Conversation
| "prefer-template": "off", | ||
| "padding-line-between-statements": [ | ||
| "error", | ||
| { "blankLine": "any", "prev": [ "*" ], "next": "return" } |
There was a problem hiding this comment.
Is this turning the "blank line before return" rule on? I thought we just agreed to turn that off: silvermine/eslint-config-silvermine@7645529#diff-168726dbe96b3ce427e7fedce31bb0bcL170
There was a problem hiding this comment.
This is actually turning it off. I should have marked this "BlankLine": "any" means "leave a blank line or not".
For clarity, I added this and prefer-template because of the weird loop we get into with the plugin and config. We're not pushing the config version bump until this is there, but I need to update the config now to make linting on the new indent rule work.
There was a problem hiding this comment.
Ok, thanks. I misread it. I see it now
lib/rules/indent.js
Outdated
| minimum: 0, | ||
| }, | ||
| 'let': { | ||
| type: 'integer', | ||
| let: { |
There was a problem hiding this comment.
It's weird that our coding standards require quotes around const and var but not let. It seems like quotes around let should be required for the same reason as const and var.
There was a problem hiding this comment.
Yeah. I'm not sure why this is "fixed" this way by the ESLing rule. I can put the quotes back though.
There was a problem hiding this comment.
I looked into why. The list of keywords that the quote-props rule uses are reserved words in ES3: https://github.com/eslint/eslint/blob/master/lib/util/keywords.js and "let" is not among them because it wasn't a reserved word in ES3.
The rule only uses ES3 reserved words because only browsers that do not support ES5 (notably IE <= 8) throw an error if you try to use a reserved word as an unquoted prop name.
I'm not certain we even need this rule anymore, unless it's just a stylistic preference 🤷♂️
jimjenkins5
force-pushed
the
fix_indent
branch
from
bccf529
to
3231ed5
Compare
|
@yokuze back to you with the one change for quoting the let properties. |
jimjenkins5
force-pushed
the
fix_indent
branch
2 times, most recently
from
4fa1664
to
a67143e
Compare
tests/lib/rules/indent.test.js
Outdated
| @@ -34,11 +34,67 @@ validExample = formatCode( | |||
| ' var arr = [ 1, 2, 3 ],', | |||
| ' noInit, andAgain;', | |||
| '', | |||
| ' //This comment should be indented with the if statement', | |||
tests/lib/rules/indent.test.js
Outdated
| '', | ||
| 'class MyClass {', | ||
| ' /**', | ||
| ' * This should be indented with the function.', |
There was a problem hiding this comment.
This is supposed to be a valid example, but I think it's wrong ... isn't it supposed to be like this?
class MyClass {
/**
* This is how it should be indented
*/
constructor(num: number) {
// ...
}
}There was a problem hiding this comment.
Note that the non-first lines of the block comment are indented an extra space so that the asterisks align.
tests/lib/rules/indent.test.js
Outdated
| '', | ||
| 'const myClass = new MyClass();', | ||
| '', | ||
| 'for(let i = 1; i < 10; i++) {', |
|
@jimjenkins5 back to you |
jimjenkins5
force-pushed
the
fix_indent
branch
from
a67143e
to
d055f45
Compare
|
See all the stuff linting helps with. Unfortunately these are all linted by rules not being tested here... |
ESLint fixed some bugs in the indent rule. Since we have a custom fork to support 3 space indentation and lining up variable declarations, we needed to update the rule.
I "re-forked" the rule to update everything, then brought it up to our coding standards.