Adds React.createContext API support #1548
Conversation
trueadm
changed the title
trueadm
changed the title
|
We need to handle the cases of sibling consumers without a new provider: <Provider value="a">
<Provider value="b">
<Consumer />
</Provider>
<Consumer />
</Provider><div>
<Provider value="b">
<Consumer />
</Provider>
<Consumer />
</div> |
| // the JSX transform converts to React, so we need to add it back in | ||
| this['React'] = React; | ||
| var { Provider, Consumer } = React.createContext(null); | ||
| // this is done otherwise the test fails |
There was a problem hiding this comment.
This is not a very helpful comment. :) Why does it fail if you don't?
There was a problem hiding this comment.
If we don't the test that runs eval on the getTrials function doesn't correctly pick up the closed over value. This has been a common pain with how we run tests this way – nothing else we can do for now.
| @@ -163,7 +163,8 @@ export type ReactEvaluatedNode = { | |||
| | "UNKNOWN_TYPE" | |||
| | "RENDER_PROPS" | |||
| | "UNSUPPORTED_COMPLETION" | |||
| | "ABRUPT_COMPLETION", | |||
| | "ABRUPT_COMPLETION" | |||
| | "NORMAL", | |||
There was a problem hiding this comment.
Normal is just a default case where neither folding or inlining happens – typically for things like React.Fragment or Context.Provider and Context.Consumer.
| // get the "render" prop child off the instance | ||
| let renderProp = Get(this.realm, propsValue, "children"); | ||
| if (renderProp instanceof ECMAScriptSourceFunctionValue && renderProp.$Call) { | ||
| if (this.componentTreeConfig.firstRenderOnly) { |
There was a problem hiding this comment.
Why is this for first render only? Can't we always fold context if we know its provider?
There was a problem hiding this comment.
No, because on update render, we need keep the children closure otherwise updates to it won't work.
src/react/reconcilation.js
Outdated
| } | ||
|
|
||
| newBranchState.addContext(contextConsumer); | ||
| if (this.componentTreeConfig.firstRenderOnly) { |
There was a problem hiding this comment.
Why is this first render only? If we know the value (which might be abstract), it should be fine for updates too. Since it's abstract it'll be able to update.
There was a problem hiding this comment.
I wanted to be on the safe side right now for updates, as we don't know if a parent in an unknown parent tree exists.
There was a problem hiding this comment.
Seems ok, but I don't understand why this is first render only instead of for updates too. Should be ok to do for updates too.
At some point we should start implementing more of this reconciliation logic in JS evaluated inside of Prepack instead. With some helpers exposed to it.
That way we can run tests against it in the React repo and run tests against it there. This is reaching into some implementation details that are quite likely to change.
|
I think this doesn't preserved the "changed bits" feature. Which is ok for now. |
|
@sebmarkbage How does changed bits work? I don't mind adding it in a follow up PR. Thanks for the review. I plan on following up on the extra stuff on the plane tomorrow. |
There was a problem hiding this comment.
@trueadm is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
There was a problem hiding this comment.
@trueadm is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
There was a problem hiding this comment.
@trueadm is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
There was a problem hiding this comment.
@trueadm is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
Release notes: none
Adds React.createContext support to the React compiler. Updates Yarn dependencies to use React 16.3 pre-release version so tests can validate new API works with the compiler.