-
Notifications
You must be signed in to change notification settings - Fork 47.5k
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 production crash #11286
Fix production crash #11286
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.
Oh, whoops 😮
@@ -211,6 +211,28 @@ describe('ReactDOMProduction', () => { | |||
); | |||
}); | |||
|
|||
// Regression test verifying that trying to access (missing) component stack doesn't crash. | |||
it('should throw on children for void elements', () => { | |||
const errorCode = 137; |
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.
You could put the entire error string into this var since it's duplicated below?
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.
Yeah, I guess.. Just copy paste from above tests. I don't really care. I'll have to get back to this and find another way to test it when I enable production testing of bundles.
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.
Sure, no big deal. Just seemed pretty arbitrary the way it was currently setup.
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.
I agree. I just don't want to spend time here because I'll delete those tests anyway.
Fixes an embarrassing production crash I introduced in #11284. I misunderstood what this
fbjs
function does.Verified this fixes it by trying the rebuilt bundle on FB.com in production mode.
Also by newly added regression test (which fails on master).