-
-
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
[Fix] jsx-no-leaked-render
: invalid fixes in coerce mode
#3511
[Fix] jsx-no-leaked-render
: invalid fixes in coerce mode
#3511
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3511 +/- ##
=======================================
Coverage 97.57% 97.58%
=======================================
Files 130 130
Lines 9213 9221 +8
Branches 3342 3348 +6
=======================================
+ Hits 8990 8998 +8
Misses 223 223
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
0f4f518
to
2f682b3
Compare
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.
Thanks!
2f682b3
to
0dac159
Compare
@akulsr0 seems like tests are failing |
@ljharb I am not sure why tests are failing here, I have tried with different node versions in local, its passing all test cases. |
The failures are in eslint < 5. |
I’m not sure why the output is different in eslint 3 and 4, but we may have to make the test output condition on eslint version. |
Do we have any example case where output is based on eslint version? |
@akulsr0 mostly examples of test cases that are entirely skipped, but eslint-plugin-react/tests/lib/rules/sort-prop-types.js Lines 505 to 540 in 12fe944
|
438472f
to
f350303
Compare
I tweaked your changes so the test cases still run in eslint 3, and it exposed a bug, which i then fixed. This should hopefully be good to go once tests pass. |
Fixes #3431