-
-
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
ReactWrapper::setProps() can only be called on the root #1925
Comments
Conceptually, the enzyme wrapper is rendered, from the root, as a pure function of props and state. It makes sense to be able to set state on any component, to be able to simulate certain inputs. However, any props that you want to pass to your This is a bit of a special case, where you're the one determining the props passed to So, you can |
Thanks for the quick reply! We'll give this workaround a try. In the meantime, should this behavior eventually be changed? I've run into a similar use case before where I had a component that renders a table cell, and I had to create the rest of the table context.
|
I'd expect the same thing - It's worth thinking about if this is a common enough use case to warrant special API support for it. |
In that case above, I think this would be a little more convenient for me:
|
@LandonSchropp the challenge is that the only things you should be able to set props on is things in the children you passed in - not things that, say, AwesomeTableCell renders. |
@ljharb Maybe I'm missing something here, but isn't it impossible to test Even those the child elements require a certain context, my tests are still only concerned with |
@LandonSchropp sorry if i wasn't clear. Here's what I'm suggesting from your OP: const wrapper = mount(
<Provider store={ store }>
<Awesome name="Landon" />
</Provider>
);
expect(store.awesomePerson).toBe('Landon');
wrapper.setProps({ children: <Awesome name="Bob" /> });
wrapper.update(); // this may or may not be needed, you'll have to try it and see
expect(store.awesomePerson).toBe('Bob'); (Note that you're mutating a prop, which is a very very very bad and un-Reacty thing to do) With your other snippet: const wrapper = mount(
<table>
<tbody>
<tr>
<AwesomeTableCell name="Landon" />
</tr>
</tbody>
</table>
);
// some assertion about Landon
wrapper.setProps({
children: (
<tbody>
<tr>
<AwesomeTableCell name="Bob" />
</tr>
</tbody>
),
});
wrapper.update(); // maybe
// some assertion about Bob |
I have a similar problem. I would like to test my component which is dependant on the provider. it('Renders valid no data text', () => {
let wrapper = mount(
<ThemeProvider>
<ResponsiveTable
noDataText="no data"
loadingError={false}
isLoading={false} data={[]}/>
</ThemeProvider>
)
expect(wrapper.contains(<div>no data</div>)).toBeTruthy()
wrapper.find('ResponsiveTable').instance().setProps({isLoading:'true'}) // this doesnt work due to lack of setProps method
expect(wrapper.contains(<div>no data</div>)).toBeFalsy()
}) If the |
wrapper.setProps({
children: (
<ResponsiveTable
noDataText="no data"
loadingError={false}
isLoading={true}
data={[]}
/>
)
}); |
@ljharb Thanks again for the detailed replies. I'm still getting used to testing React with Enzyme, and it seems to me I still have some learning to do to properly build components that correctly fit into the correct model. Based on your response, I think I have enough information to work around my current issue. I appreciate your help in getting there. 😀 If it's okay with you, I would like to leave this issue open as a feature request. I do think there are a few use cases where it would be more convenient to be able to call |
I think leaving this open is a great idea; so far all the use cases have been about setting props on one of the elements initially passed in to the wrapper - not about setting props on a rendered child component - and there might be something to explore there. |
Thanks for the response.But the idea behind updating the props in the child component is that i would like to test componentDidUpdate(prevProps) {
const { showNotification, hideNotification } = this.props.notificationBlanket
if (this.props.isLoading && !prevProps.isLoading) {
showNotification({
type: 'pending',
name: 'loadingIndicator'
})
}
if (!this.props.isLoading && prevProps.isLoading) {
hideNotification()
}
} Maybe this code is not perfect but i have build Ommiting the use case, how to test componentDidUpdate in that scenario?when the I know that you can pass the context directly to the mounted component.Is it the only solution for this? |
Generally i shallow-render the component, and setProps directly on it. Instead of wrapping it in the Provider, I’d manually provide the context option to shallow. |
@ljharb how do you handle, for instance, when you need to force focus for accessibility purposes so you use a ref... It doesn't look as though refs are compatible with |
@mmoss since string refs are deprecated, you can use a ref callback, explicitly invoke it, and trust that it gets a DOM node. You can also use |
@ljharb I'm not sure I'm following how that would let me test this... The issue I've got is that I'm using export class MyButton extends React.Component {
buttonRef = React.createRef();
componentDidUpdate(prevProps, prevState) {
if (prevProps.someProp !== this.props.someProp) {
this.buttonRef.current.focus();
}
}
render() {
return (
<React.Fragment>
<MyContext.Consumer />
<button
ref={this.buttonRef}
// ... Update: I worked around this by just stubbing the ref and using shallow rendering. I needed to assert that the focus method was invoked anyhoo. |
(enzyme doesn't yet support Context; not sure if that's relevant to your example or not) I think stubbing the ref is probably the simplest approach, since |
@ljharb I am using react-router's Router and Link components. I want to write a test suite which tests the following: For test a to pass, I can not use shallow, since the Link component is abstracted by my own component so that react-router imports do not pollute the code base. Unfortunately, once mounted, the react-router Link component throws an error unless a Router component is present somewhere above it in the element tree. Due to this, the mount root becomes the Router and not the Link component. |
@EyalPerry it can still be done by re-setting the “children” prop on the root, albeit not very ergonomically. This is definitely a use case i think that can be well addressed without needing to setProps on a child; it’s being worked on. |
This comment has been minimized.
This comment has been minimized.
@ljharb it's being worked on === you are working on setProps for non root wrappers? |
@ljharb The solution you propose does indeed work, but it is not as I would like. I wrap enzyme in a thin layer of sugar and use that layer. resetting children whenever I want to set props sounds like it could break tests, my spider senses are tingling. |
@EyalPerry no, i don't think it ever makes sense to set props anywhere but the top of the tree, just like in react. However, I think the use case of "i need to test X, but in order to do that, I have to wrap it in 1 or more "provider"-like components, and i'll need to set props on X" can be addressed with a separate API when creating the initial wrapper. This includes your Router use case. iow, instead of: const wrapper = mount(<Router><Link /></Router>); you'd do something similar to: const wrapper = mount(<Link />, { wrappedWith(root) { return <Router>{root}</Router>; } }); |
@ljharb awesome. Thank you for the update! |
We had a similar situation for a component that does require a TranslationContext to be tested correctly (so, root is translationcontext and the tested component is inside). What we did is to wrap the whole thing into another ad-hoc component and pass-through the properties like such: mount(
React.createElement(
props => (
<I18nextProvider i18n={i18n}>
<ComponentUnderTest {...props} />
</I18nextProvider>
),
properties)
); That way, you can set props on the root and they trickle down to the component under test |
But what if I export component wrapped in HOC? e.g in component I have |
@adarrra i'm not sure why you'd need to. In your case, the HOC-wrapped component is in charge of what props Navigation receives, so if you set props and/or state on the wrapped component, it will affect what Navigation gets. |
Similar to this, I solved this by making a proxy component:
|
If the goal is to provide the component with a particular context there is an alternative approach. Mock the context and pass it into the second argument of
This example is adapted/stolen from this solution on how to to do enzyme testing with react router. |
Using the @flq solution, you can build a reusable test function: For example, wrapping a component with a
import it into your
Use it whenever you need to wrap something in a MemoryRouter:
|
Given the rise of Context as an architectural model, the idea that props will be on the root node simply isn't true anymore. All of these dozens-of-lines-long work arounds don't address the basic issue, which is that enzyme was designed for a different React architecture than the one we are now working with. |
@bethcodes that idea hasn't ever been entirely true, because of components that provide old-style context. The basic issue is that React has introduced many new features without sufficient regard for how people will test them in a more granular way than "ReactDOM.render everything". |
The workaround proposed by @jaydlawrence was very helpful and I was able to make use of it successfully, thanks! |
Yes that is the way to go for the moment 👍 |
I'm trying this setProps form mapatataprop but it doesn't update it and the props still remains same as coming from store |
Current behavior
My team is trying to test a component that consumes a MobX wrapper like this:
In this simplified example,
Awesome
's implementation would look something like this:We're trying to test the
componentDidUpdate
method. Because we need to use aProvider
, we can't callsetProps
on the root element. Instead, we tried to do this:Unfortunately, this produces this error in Enzyme:
It looks like this issue has been opened before with
setState
, but doesn't seem to be working forsetProps
.Expected behavior
We should be able to set the props of a child component.
Your environment
API
Version
Adapter
The text was updated successfully, but these errors were encountered: