Update to newer eslint #3643
Merged
Update to newer eslint #3643
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This might help if we wanna keep scope fresh w/ latest eslint. Otherwise we could disable those rules.. https://github.com/thibaudcolas/react-destructuring-assignment-codemod/blob/master/README.md |
|
@fbarl what do you think? |
|
Some icky cases for us:
handleMouseEnter(ev) {
- this.props.enterEdge(decodeIdAttribute(ev.currentTarget.id));
+ const { enterEdge } = this.props;
+ enterEdge(decodeIdAttribute(ev.currentTarget.id));
}I think disabling is okay really. |
dholbach
pushed a commit
that referenced
this pull request
Jul 1, 2019
as per discussion in #3643 (comment)
|
The new list of lint problems is: |
fbarl
reviewed
Jul 3, 2019
| </i> | ||
| ) |
There was a problem hiding this comment.
A lot of this autofixed HTML content has very weird indentation now - I wonder if there's a nicer way to represent it that is still acceptable for the linter..
| return BLURRED_EDGES_LAYER; | ||
| } else if (edge.get('highlighted')) { | ||
| } if (edge.get('highlighted')) { |
There was a problem hiding this comment.
I personally find this rule rather confusing (if being the same as else if only because of the return statements feels like an extra cognitive load), but I think I'd be able to live with it :)
There was a problem hiding this comment.
I'd put the if on the next line.
There was a problem hiding this comment.
This was done by eslint using the --fix option.
There was a problem hiding this comment.
This was basically ef44eb3 - let me know if you'd prefer me to revert this and we either
- figure out if we want to fix our code to comply with new
eslintrules - or ignore some of those new rules
| @@ -31,6 +31,7 @@ | |||
| "no-param-reassign": 0, | |||
| "no-restricted-properties": 0, | |||
| "object-curly-spacing": 0, | |||
| "react/destructuring-assignment": 0, | |||
|
@fbarl and @foot Thanks for the reviews so far. I set up this Draft PR, so we can collectively fix this. I want to get the security issues resolved, but I won't have time to dive deep enough into this to actually have an opinion on JavaScript indentation. This is what I could fix easily. The rest will have to be done by somebody else If fear. |
dholbach
force-pushed
the
security-updates
branch
from
feaca43
to
39c5535
Compare
dholbach
pushed a commit
that referenced
this pull request
Jul 18, 2019
as per discussion in #3643 (comment)
|
Almost there: |
…by using unreleased eslint-loader (waiting for release)
as per discussion in #3643 (comment)
dholbach
force-pushed
the
security-updates
branch
from
618cc73
to
2ab7ada
Compare
|
|
I just addressed all the remaining I'd suggest to test only the behavior when reviewing this PR as the code change mostly consists of functions that moved around. |
|
Ready for review now. |
dholbach
force-pushed
the
security-updates
branch
from
478e325
to
a700530
Compare
dholbach
pushed a commit
that referenced
this pull request
Aug 9, 2019
as per discussion in #3643 (comment)
|
Since @fbarl and I worked on this, we both tested it and the warnings in the browser console look OK, we'll go ahead and merge it. |
fbarl
approved these changes
Aug 12, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I started work on updating
eslintfor a security update. Some issues I was able to resolve by runningeslint --fix, these still need work and I'd need help: