-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
add option for childContextTypes of ReactWrapper
#171
Conversation
// In that case, we define both a `getChildContext()` function and a `childContextTypes` prop. | ||
let childContextTypes = node.type.contextTypes; | ||
if (options.childContextTypes) { | ||
childContextTypes = options.childContextTypes; |
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.
should we merge node.type.contextTypes
and options.childContextTypes
?
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, it's probably better. I'll update this, however, I think there should be better automatic fix to this issue.
@jakubzitny I suppose you mean by traversing the tree and finding the required context types of the descendants? I'm really not sure if this is possible. At the time this code executes, the It's possible I'm not fully grokking the way context is working, but i'm unsure how react itself solves this if this is how context works. |
@lelandrichardson Yeah I meant something like that. I know it's not really possible without rendering. :( I was thinking about just looking at the contextTypes of all children somehow. Isn't it really possible to get the children of a component without rendering somehow? By looking at the The thing is that in React if you do this, you always specify both Btw I pushed a fixup for merging both childContextTypes. I will rebase it if you want to accept this PR. |
Going to go ahead and merge this in. I think this is the best way to do this at this point. Open to PRs for alternatives though. |
add option for childContextTypes of `ReactWrapper`
Is there any specific reason this wasn't built into the shallow wrapper API? |
@blainekasten there is no need for it in ShallowWrapper.. (the problem was with context of deeper child nodes, there are none in ShallowWrapper) |
Fixes #144.