Switch the Context to use the Parent tree instead of the Owner tree
sebmarkbage opened this issue · 36 comments
I'm pretty convinced at this point that Contexts are more useful in the Parent tree than the Owner tree. It also opens up new use cases for parent->child communication that wasn't possible before.
I'd be a big +1 for this. This would really make it much easier to design complex components in react.
@sebmarkbage I'm not entirely sure exactly what problem contexts are meant to solve. But in the hypothetical context of reinvented styles (without selectors) I can imagine a benefit in being able to propagate hoveringOverTheBigButton
(active/etc) to all descendants... and same for implicitly passing props from far up the hierarchy to far down (as is often requested). This should obviously be in the Parent tree then.
It seems that using a GUID style approach would be preferable for avoiding any and all conflicts, components would instead opt-in. Something like:
var Child = React.createClass({
contextSources: {
ancestorIsHovered: Parent...hovered
},
render: function() {
return <div>{this.context.ancestorIsHovered ? 'Ancestor hovered' : 'Ancestor not hovered'}</div>;
}
});
var Parent = React.createClass({
childContextTypes: {
hovered: React.PropTypes.boolean
},
getChildContext: function() {
return {hovered: true};
},
render: function() {
return <Child />;
}
});
But again, I'm not exactly sure what contexts are meant to solve so I could be way way of the mark here.
If someone who is more familiar with the React internals would care to put together a short summary of what needs to be done here, I'd be glad to give it a shot and make a PR. react-router depends heavily on context
, and it's starting to break for some people's use cases.
Things I don't fully understand:
- What exactly is the difference between the parent and the owner trees? Is one DOM-based and the other a React thing?
- How might this change affect users currently using
context
?
I'd love to know more about when/if this might happen and what it will take to get there. I share @1st1's belief that this will make component/library composition better. In my particular case, I'm working on a library that wraps ReactRouter, and I'd like to be able to pass data down from my library, through the routes, and into the app's components. I can't presently do this without either a) changes by ReactRouter to support this directly, or b) the ability to pass a context into an app's already-created routes
.
@sebmarkbage I assume we'll want to support reparenting someday… is there anything we need to keep in mind while doing this? Guessing not but wanted to check.
Things I learned yesterday while talking to @sebmarkbage about this:
- The parent tree is made up of parent components, not necessarily DOM elements. The owner tree is made up of components that were on the bottom of the render stack at the time a given component was rendered.
- Changing
context
to use the parent tree gets us one step closer to being able to remove_owner
entirely, which is a good thing. - The vast majority of people currently using
context
should be unaffected by this change since the owner tree is by definition a subset of the parent tree.
cc @JSFB
+1 for this. So happy this is finally being worked on!
We use context indirectly through react-router, and use it directly for analytics (e.g. in which of several "areas" of the screen / on which screen was "Sign Up" button pressed).
Another use case is when we want some pages to be "themed" with a specific background color and we'd like all controls, no matter how deep in hierarchy, be able to use this color (can't do this in CSS because we want to calculate whether text needs to be dark or light based on that, etc).
I'd like to use contexts for flux stores. Almost all current flux implementations use singleton stores which are really nice to use in the browser, but are pain to use in the server when doing isomorphic apps because multiple requests can be processed concurrently which does not fit well with singletons.
This can be fixed by creating instances of the stores, but then we have to manually pass the stores as props which can be cumbersome and you'll end up with components that depend on the store props which don't actually use the store themselves but only pass them forward others. Which is ugly imo.
I think with context and instance based flux stores we would get the best of both worlds.
There's been some talk about deprecating contexts in other issues. What's really going to happen to them?
Contexts are still an experimental feature, and while they're probably not going to be deprecated, they are not yet documented and may be subject to change without notice. For this reason, you may want to avoid using them in production applications until the API solidifies. That said, a flux store is a perfect example of a good use case for contexts.
On the server-side, I see no reason that everyone can't share a single flux store. Each request to the store would also need the userid for whom you're fetching the information, and apps running client-side would get a 403 error when trying to access data for another user. Your flux store may act as a cache, or may query the database directly, but either way, by making store requests that include the user, you can now share a single store across the app again.
Current status for contexts is: optimistically hopeful, but not ready for prime time yet.
Using a build off trunk, I'm getting warnings about owner tree vs. parent tree. Parent tree has the keys I want, but I can't access it. Perhaps I'm just not understanding how context is supposed to work.
Is this use case supported? <form.RenderForm>
creates a form instance in its componentWillMount()
and passes it as a form
child context variable. Its render()
method uses React.Children.only()
to return its only child (a <GridForm>
in this case) The intent is that <Field>
can declare form
in its contextTypes
and access this.context form
to render the field named in its props.
However, console output in the live version says this.context form
is undefined.
I don't think parent-based context is in master yet, is it? My impression was that only warnings are already added, but implementation is not there yet.
D'oh, I took "initial implementation" at face value when scanning updates to this issue 😵
I'm not really sure how much is implemented.
Obviously parent-based context is calculated and passed (or warning wouldn't work), but since React usually keeps deprecated behavior for one more version, I reckon that owner-based context is what you're getting in this.context
.
From the looks of how warning is implemented, you might get parent-based context like this: this._currentElement._context
. I'd assume this is what will be in this.context
in vNext+1.
I'm pretty sure there is currently a bug in the warnings in trunk too. The risk of running on trunk. :) cc @zpao for a stable build.
@insin @gaearon Master currently only warns if the contexts differ, but uses the old notion of context. To change over to use parent contexts should be a two line change, but we wanted to maintain the old behavior with warnings for a period of time until we're ready to pull the trigger on a switch.
Yes, it's entirely possible there is still a bug. Looking into the masking on _performContextUpdate now.
we wanted to maintain the old behavior with warnings for a period of time until we're ready to pull the trigger on a switch
And this is what I ❤️ about React.
Any updates on when this might be blessed to go into master? I have a bucketful of potential use cases, and I'd love to be able start playing around to see what I can do with it (with the understanding that it's still unstable).
Agreed, I have a use case for an animation library that for now has bugs because I'm having to roll a shoddy version of it myself. Would be nice just to know current thoughts on release schedule.
On the server-side, I see no reason that everyone can't share a single flux store.
@JSFB everything I've seen points to stores being fully synchronous, and that you actually do the asynchronous action in the action creator (which eventually triggers another action, which causes the store to update). Without all of the pieces instanced per request, it appears you'd have to
- have all stores act like they're serving every user at once,
- find a way to determine 'done' in this environment (i.e. when can I finish the request?),
- ensure stores aren't holding references to data after 'done', while ensuring they do hold it on the client,
- use an alternative mechanism to 'change' events (getting the data to the source of the request)
Could you clarify how you get around these? Are you thinking of simple service modules (export functions, communicate by callbacks/promises) rather than flux?
@brigand Yes, singletons are not without their drawbacks. Thus the motivation to pass flux stores as context variables => thus our desire to implement context variables.
Your interpretation is correct: If you use a singleton store server side, you will need to have stores act like they're serving every user at once. You may likely end up with a different server-side store implementation (conforming to the same interface) that is backed directly by the database (since the cost tradeoffs are entirely different on the server side, your security enforcement policy is inherently different, etc). If you are backed by the database, you are by definition the source of truth (can go straight to the database), and you therefore don't need to worry about tracking a 'done' state nor worry about 'change' events. You sacrifice a little isomorphism, to gain simplicity in your implementation (ie. you don't need to track done state because you aren't dealing with 'change' events server-side), which you may have wanted to do anyway for security/perf reasons.
It was not my intent to hijack this thread, merely to point out that there are alternatives for flux stores. You can use a singleton model as long as your store implementations conform to a multi-user interface instead of a single-user interface. That said, we are implementing context variables because we think they may be valuable in these situations. I'm not advocating one solution over the other, and both solutions have their tradeoffs.
@natew Even after we switch to parent based contexts, there are a number of design decisions surrounding contexts that we'll still need to figure out. It will likely take us some time before we feel comfortable publishing them as a supported feature.
Thank you for the clarification, that makes a lot of sense and I can see how it'd simplify things now.
@jwietelmann @natew If you're using react 0.13 beta 1, and building with Webpack (or similar), you can use react-parent-context-patch.js to play around with parent contexts for the moment. I assume it'll break pretty quickly though, so don't build anything which relies on it.
Make sure you require it before you require('react')
.
@jamesknelson thanks. I'll be hacking on that this week and let you how how it goes.
@jamesknelson Just added this in and I'm getting:
Uncaught TypeError: Can't add property _pendingStateQueue, object is not extensible
ContextPatch:58
Edit: This is actually after it's loaded, running an app:
ReactCompositeComponentMixin.mountComponent (ContextPatch.js:58)
ReactReconciler.mountComponent (ReactReconciler.js:38)
mountComponentIntoNode (ReactMount.js:232)
Mixin.perform (Transaction.js:130)
batchedMountComponentIntoNode (ReactMount.js:247)
Mixin.perform (Transaction.js:130)
ReactDefaultBatchingStrategy.batchedUpdates (ReactDefaultBat…Strategy.js:62)
batchedUpdates (ReactUpdates.js:95)
ReactMount._renderNewRootComponent (ReactMount.js:349)
ReactMount__renderNewRootComponent (ReactPerf.js:66)
ReactMount.render (ReactMount.js:392)
React_render (ReactPerf.js:66)
renderToDocument (render.js:25)
(anonymous function) (render.js:58)
React.createClass.statics.run.dispatchHandler (createRouter.js:397)
(anonymous function) (createRouter.js:372)
(anonymous function) (Transition.js:55)
(anonymous function) (Transition.js:55)
runTransitionToHooks (Transition.js:60)
assign.to (Transition.js:97)
(anonymous function) (createRouter.js:360)
runTransitionFromHooks (Transition.js:32)
assign.from (Transition.js:93)
React.createClass.statics.dispatch (createRouter.js:357)
React.createClass.statics.refresh (createRouter.js:430)
React.createClass.statics.run (createRouter.js:412)
runRouter (runRouter.js:45)
async (render.js:57)
run (index.js:11)
(anonymous function) (app.js:22)
(anonymous function) (main.js:551)
__webpack_require__ (main.js:509)
fn (main.js:77)
(anonymous function) (multi_main:3)
(anonymous function) (main.js:539)
__webpack_require__ (main.js:509)
(anonymous function) (main.js:532)
(anonymous function) (main.js:535)
this
at the time of the error is:
Using react-0.13-beta.1, requireing it before everything. Also this is using react-router.
@natew, I've been using my patch since I posted it, but don't believe I've gotten that issue. Are you using 0.13.0-beta.1?
Yea using beta-1, and loading before anything. It seems to load alright but error later. I didn't dig in too deep just to see if you had an ideas up front. It's not super high up my priority for now but I'll revisit again soon.
@jamesknelson is this enabled in RC2 or would it be worth updating and trying the patch there?
Is there any chance that we might get some documentation on how contexts work? I've seen a vague mention of this feature on the React blog. I've seen it in use in other projects such as React-router and perhaps also Reapp. There is code in those projects that throws warnings and needs migrating. I'd love to contribute but I feel unable to help make the situation better because I'm still in the dark about how contexts are supposed to work.
@zenlambda maybe this will help https://www.tildedave.com/2014/11/15/introduction-to-contexts-in-react-js.html
Just as a heads up to anyone using the contextPatch and running into type.toUpperCase() is undefined
errors, this just bug bit me and drove me mad for the last couple hours.
If your child Class is attempting to use an undefined component this happens. In my case I was trying to import "Textarea" rather than "TextArea".
I was certain the error was due to how I was mounting routes (in the parent class) and funny enough both children routes I tried had this same Textarea bug, so I spent forever changing the parents until I got this. 💀
Any chance this will land to 0.14?
@1st1 That's the plan.
Does React Native use parent context now as well? Seems like it still uses owner context.
React Native is still on React 0.13.2. We'll update after the 0.14 final release.
Cool, glad to know this is on the way! I've also used context to pass down stores, out of a desire to avoid singletons. I've used them in a few other cases as well.