Skip to content
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

Pass a context to rendered Component's child? #144

Closed
jakubzitny opened this issue Jan 28, 2016 · 9 comments
Closed

Pass a context to rendered Component's child? #144

jakubzitny opened this issue Jan 28, 2016 · 9 comments

Comments

@jakubzitny
Copy link
Contributor

I am using enzyme.mount to render a Component with some child Components and I need to pass a context to these child Components. I can pass a context with options.context to the Component but I would like to pass something else to Component's children.

The options object in mount would be good place to do this, however, it is currently not possible. I know I can create my own wrapper with childContextTypes, but for better readability and simplicity of tests, to do this directly with enzyme.mount would be amazing.

What do you think?

@blainekasten
Copy link
Contributor

I can pass a context with options.context to the Component but I would like to pass something else to Component's children.

Can you expand on "something else" ?

I'm curious to what you want to do that you can't currently do with the options.context param. And am sort of confused by your reference to creating a wrapper with childContextTypes as that is effectively what happens under the hood of that API.

@jakubzitny
Copy link
Contributor Author

Allright. Let's say I have a ComponentA that renders ComponentB inside and ComponentB has several ComponentC children. Now, I am unit testing ComponentB.

ComponentB has:

contextTypes: {
  storeAbc: ...
  somethingElse: ...
}

but ComponentC has more stuff in context:

contextTypes: {
  storeAbc: ...
  storeXyz: ...
  somethingElse: ...
  yourMum: ...
}

Now, when I want to mount ComponentB it should be able to render ComponentCs inside, but there is no option to pass the additional context to it via ComponentB. The extra stuff in ComponentC's context is not in ComponentB and if I passed it to ComponentB's context I'd get a violation or something. So now I need to create my own wrapper with it which I don't want. That's where enzyme is great.

Is it clear?
Am I missing something?

@blainekasten
Copy link
Contributor

That makes sense. So are you trying to test ComponentC through rendering ComponentB? Can you test that component on it's own? Or is it a matter of it just throwing errors when trying to test ComponentB?

If so, you could mock out ComponentC so it's still assertable that it is rendered as a child of ComponentB without throwing the errors from missing context. That might help in the interim.

I'm not sure if there is a bug here or not, @lelandrichardson will probably have a better concept of if something is missing here.

If I understand correctly, just to reiterate, this is effectively the problem you are having:

// given this tree structure

                      +--------------+
                      | ComponentA   | // this sets the context `foo` and `bar`
                      +--------------+
                             v
                      +--------------+
                      | ComponentB   | // subscribes to context `foo`
                      +--------------+
                             v
                      +--------------+
                      | ComponentC   | // subscribes to context `bar`
                      +--------------+

So when testing ComponentB who only cares about foo you can get the context to that, but it's child is not getting the context it needs as you aren't rendering ComponentA. Is that correct?

I guess, when I think about it, passing context to the mount/shallow API should be encompassing for all it's children.

mount(
  <ComponentA />,
  { options: { context: { foo: true, bar: false } }
);

And so you're saying this fails for you?

@lelandrichardson
Copy link
Collaborator

I may be misunderstanding how context works exactly, but it seems to me like the provided API should be all that is needed. My understanding was that context can be passed down to immediate children as well as deeper children (ie, grandchildren).

If this is needed we can add it, but I'm not sure that it is. Do you think you could work up a test that fails that you think should pass?

@jakubzitny
Copy link
Contributor Author

Sorry for late reply, so..

To answer @blainekasten's questions:

I am trying to unit test ComponentB.

ComponentC itself works great. Also shallow rendering ComponentB works allright. But when mounting ComponentB I get following:

Error: Invariant Violation: ReactCompositeComponent.getChildContext(): key “storeXyz" is not defined in childContextTypes.

Mocking out ComponentC is good idea, but with large project and a lot of cases where this happens it’s a hassle.

So in general you got the structure of the components, however the problem is not that ComponentAs don’t get the context (when mounting ComponentB). The problem is that ComponentB is getting all contexts (for ComponentB and ComponentCs) from the wrapper but has a problem with the stuff it doesn’t need/want.

I wrote a test case in my fork (@lelandrichardson) : jakubzitny@9094d7c

@jakubzitny
Copy link
Contributor Author

So I found where the problem is, I just don't know how to fix it. Yet.

In ReactWrapperComponent.jsx you specify context for children like this:

if (options.context && node.type.contextTypes) {
  // For full rendering, we are using this wrapper component to provide context if it is
  // specified in both the options AND the child component defines `contextTypes` statically.
  // In that case, we define both a `getChildContext()` function and a `childContextTypes` prop.
  objectAssign(spec, {
    childContextTypes: node.type.contextTypes,
    getChildContext() {
      return this.state.context;
    },
  });
} 

Both getChildContext and childContextTypes are needed and getChildContext "fills" the context properly with everything that's needed. However, childContextTypes is being filled with contextTypes of the "top" component only. So the contextTypes for all the children of the top component are missing.

@lelandrichardson any ideas how to get the context types of all children components without complex traversing or something like that? I was thinking maybe there could be new field options.contextTypes for mount, where user would be able to specify that.

@lelandrichardson
Copy link
Collaborator

@jakubzitny I see the problem now. Thanks for the explanation.

One thing we could do is add childContextTypes as an option to mount which would specify a merged childContextTypes declaration for the ReactWrapperComponent.

Do you think this could work? If you could write up a quick gist of a failing test case, I can make sure that it works for what you're trying to do...

@jakubzitny
Copy link
Contributor Author

The failing test case looks like this: https://gist.github.com/jakubzitny/7b9d526f77b2865219d3

And yeah I think adding it as an option is allright, it's probably the fastest way to fix this. There may be another possibility to "scan" all the children for their contextTypes and do it automatically. That would be a bit harder I think..

I can try to add it and submit a Pull request.

jakubzitny added a commit to jakubzitny/enzyme that referenced this issue Feb 7, 2016
jakubzitny added a commit to jakubzitny/enzyme that referenced this issue Feb 7, 2016
jakubzitny added a commit to jakubzitny/enzyme that referenced this issue Feb 7, 2016
@fmichea
Copy link

fmichea commented Jan 9, 2018

Not re-opening the ticket but just wanted to add for future readers that using context and childContextTypes replaces the getChildContext() method for the root component given to mount, which will be an issue if that root component also has its own context it propagates to its children.

To resolve this, you can simply wrap that root component in a <div></div> and have a helper function that gets the component (if you need it).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants