-
Notifications
You must be signed in to change notification settings - Fork 49.8k
Fix obscure error message when passing an invalid style value for SSR #11173
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
Conversation
|
Can you please declare it in the top scope so we don’t need to worry about allocating? |
|
You can just pass |
|
And yes, we should Flow-ify that file. But that’s a separate problem. Wanna do it in another PR? |
|
Prettier is failing. |
1 similar comment
|
Prettier is failing. |
gaearon
left a comment
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.
Use emptyFunction.thatReturnsNull
|
Filed #11175 for Flow. |
|
👌 thanks, Dan! |
|
React |
|
Would it be possible for React to throw for invalid styles when inside React.StrictMode? |
This may be a regression to add to #11065.
Adding a reduced test case for this issue by opening a PR. When trying to server render an
iframewith an invalidstyletag the following error is thrown:I would expect this to throw the invalid props-style warning of:
This can be traced back to this line here where we are missing the third argument to
assertValidProps:react/src/renderers/shared/server/ReactPartialRenderer.js
Line 783 in a276ffc
This file is not checked by flow which is possibly how the missed argument slipped through, though naively ading the
@flowpragma to that file has 39 errors.What should the fix be? (I included a commit to implement the only solution I’ve considered thus far)
noopthird argument tocheckValidProps? (since the only otherownerref in that file is explicitly set tonullat https://github.com/facebook/react/blob/master/src/renderers/shared/server/ReactPartialRenderer.js#L57)