Add eslint-plugin-regexp #142
Conversation
This change is created by: - the `npm i eslint-plugin-regexp` command - a few additions to the `.eslintrc.js` file. Close #141
.eslintrc.js
Outdated
| @@ -113,5 +114,8 @@ module.exports = { | |||
| "prefer-template": "error", | |||
| "sort-requires/sort-requires": "error", | |||
| strict: ["error", "global"], | |||
|
|
|||
| // Prefer code readability over a bit performance improvement. | |||
| "regexp/no-unused-capturing-group": "off", | |||
There was a problem hiding this comment.
[note] There is room for discussion about disabling regexp/no-unused-capturing-group. Please give me feedback if any.
See also #141 (comment)
There was a problem hiding this comment.
0-9 vs \d is not readable
There was a problem hiding this comment.
@alexander-akait Thanks for the feedback!
?:is readable...
Did you mean we should enable regexp/no-unused-capturing-group?
0-9vs\dis not readable
Did you mean we should disable prefer-d? This rule is for code style, so it seems acceptable to me that we can disable it.
There was a problem hiding this comment.
Did you mean we should enable regexp/no-unused-capturing-group?
Yes, I found perf is better here, ?: is not hard to read
Did you mean we should disable prefer-d? This rule is for code style, so it seems acceptable to me that we can disable it.
It is just my opinion, better to use explicit in some cases
/cc @hudochenkov @jeddy3
There was a problem hiding this comment.
About prefer-d, the following demo in stylelint/stylelint#5516 may be helpful.
Personally, I feel [0-9A-Za-z] is a bit more readable than [\dA-Za-z].
-const HEX = /^#[0-9A-Za-z]+/;
+const HEX = /^#[\dA-Za-z]+/;
There was a problem hiding this comment.
Let's turn off prefer-d for now. We can always revisit.
There was a problem hiding this comment.
Let's turn off
prefer-dfor now.
Done 7d67470.
What about regexp/no-unused-capturing-group?
In this case, ?: surely seems a bit hard to read, but performance is important too.
-const HEX = /^#([\da-f]{3,4}|[\da-f]{6}|[\da-f]{8})$/i;
+const HEX = /^#(?:[\da-f]{3,4}|[\da-f]{6}|[\da-f]{8})$/i;There was a problem hiding this comment.
Let's favour performance for now. We can always change back if it proves an issue.
|
@jeddy3 No, https://github.com/ota-meshi/eslint-plugin-regexp/blob/v1.1.0/lib/configs/recommended.ts#L2 Also, in the PR #133, we will remove the needless |
Whoops, I knew that... it's the same approach we're adovating for in |
Close #141
This change is created by:
npm i eslint-plugin-regexpcommand.eslintrc.jsfile.See also https://www.npmjs.com/package/eslint-plugin-regexp