[Fiber] adds childContextTypes warnings for functional components
#9043
Conversation
gaearon
changed the title
childContextTypes warnings for functional componentschildContextTypes warnings for functional components
trueadm
force-pushed
the
fix-fiber-functional-componment-childcontext
branch
from
5713667 to
9f75f35
Compare
ReactStatelessComponent-test.js fails due to warnings not showing up when functional components mount or update. This PR ports the existing warnings from stack and re-applies them to fiber functional components and attempts to re-use some logic in ReactFiberContext that currently exists for showing warnings when dealing with class components.
trueadm
force-pushed
the
fix-fiber-functional-componment-childcontext
branch
from
9f75f35 to
a9325e2
Compare
| Component.displayName || Component.name || 'Component' | ||
| ); | ||
| if (Component.childContextTypes) { | ||
| warnAboutMissingGetChildContext(workInProgress); |
There was a problem hiding this comment.
I think the test is wrong. It doesn't make sense to both warn for childContextTypes being defined and also warn for that you should provide a getChildContext which you cannot do. It's not actionable.
IMO we should either fix this in Stack and remove it or gate the test on ReactDOMFeatureFlags.useFiber and change the assertion for the Fiber case.
There was a problem hiding this comment.
Agreed, completely. I was unsure if we wanted 1:1 parity with Stack in regards to these strange warning messages. I don't know why anyone would ever need the second warning message when using functional components, given they can't ever use getChildContext on the components. Was there intentions at some point to allow functional components to use getChildContext?
|
@sebmarkbage I've updated the PR to reflect those changes. I opted to instead add a flag in the test for Fiber rather than changing the warning messages on Stack. |
| 'childContextTypes from it.' | ||
| ); | ||
| // Stack and Fiber differ in terms of they show warnings | ||
| if (!ReactDOMFeatureFlags.useFiber) { |
There was a problem hiding this comment.
Let's flip this to be a positive condition, it's easy to misread.
ReactStatelessComponent-test.jscurrently fails behind Fiber flags due to warnings not showing up when functional components mount or update whenchildContextTypesare defined on the functional component.This PR ports the existing warning messages from stack and re-applies them to Fiber's functional components. Existing login within
ReactFiberContext.jswas also re-used on functional components (previously it was only used for class components) to fully full in line with the expected behaviour for the test assertions.