-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Fix failures of jsx-max-depth on empty nodes #1720
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
base: master
Are you sure you want to change the base?
Conversation
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 for the fix!
It's pretty important to find the code this broke on, so that we can add a regression test.
@@ -90,7 +90,8 @@ module.exports = { | |||
|
|||
return isJSXElement(writeExpr) | |||
&& writeExpr | |||
|| writeExpr.type === 'Identifier' | |||
|| writeExpr |
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.
probably this whole chain should be rewritten for clarity; at the least, let's use parens so as to not mix && and || :-)
@j-martyn @ljharb Any ETA ? I see this crash too. FYI, it happens on files with jsx-control-statements. |
@ferhatelmas if you could provide a failing test case (on a branch in your fork), and link it here, i'll add it to the PR and we can get this in. |
cc @alexzherdev if you're interested in taking a look at this one, i'm happy to add any commits you like to this PR branch :-) (cc #1824) |
I'll see if I can reproduce this 👍 |
ping @alexzherdev, any progress? |
59af733
to
865ed16
Compare
069314a
to
181c68f
Compare
380e32c
to
51d342b
Compare
On some my code (didn't check what code exactly) I got this:
So, for now, I just have fixed it. No tests have been changed as I didn't investigate it.