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

ReactWrapper.update() is not forcing a re-render #2042

Open
2 of 4 tasks
nicholasrice opened this issue Mar 9, 2019 · 32 comments
Open
2 of 4 tasks

ReactWrapper.update() is not forcing a re-render #2042

nicholasrice opened this issue Mar 9, 2019 · 32 comments

Comments

@nicholasrice
Copy link

nicholasrice commented Mar 9, 2019

Current behavior

min-repro here: https://github.com/nicholasrice/enzyme-react-wrapper-update-repro

Using mount to mount a component, calling the update method of the returned ReactWrapper instance does not seem to be forcing a re-render. With slight changes, I implemented the example from https://airbnb.io/enzyme/docs/api/ReactWrapper/update.html and am expierencing a test failure.

On a slight aside, I think the assertions being made in the above documentation should be 1 for the first call and 2 for the second call, instead of 0 for the first call and 1 for the second.

const React = require('react');
const Enzyme = require('enzyme');
const Adapter = require('enzyme-adapter-react-16');

Enzyme.configure({ adapter: new Adapter() });

test("ReactWrapper.update should re-render a react component", () => {
    const spy = jest.fn();

    class ImpureRender extends React.Component {
        constructor(props) {
            super(props);

            this.count = 0;
        }

        render() {
            this.count += 1;
            spy(this.count);

            return React.createElement("div", {}, this.count);

        }

    }

    const wrapper = Enzyme.mount(React.createElement(ImpureRender));

    expect(spy).toHaveBeenCalledTimes(1)

    // Update the component - should force re-render per https://github.com/airbnb/enzyme/blob/master/docs/api/ReactWrapper/update.md
    wrapper.update();

    expect(spy).toHaveBeenCalledTimes(2)
});

Expected behavior

I would expect that calling the update method of a ReactWrapper would call the render method of the mounted component.

Your environment

API

  • shallow
  • mount
  • render

Version

library version
enzyme 3.9.0
react 16.8.4
react-dom 16.8.4
react-test-renderer 16.8.4
adapter (below) 1.10.0

Adapter

  • enzyme-adapter-react-16
@ljharb
Copy link
Member

ljharb commented Mar 9, 2019

Since this relies on an impure render (which is obviously a very bad idea in React), can you elaborate on the actual use case where this is popping up? If props and state and context haven't changed, a rerender shouldn't need to call render, even via enzyme's update.

@nicholasrice
Copy link
Author

Yea the min-repro isn't a real use-case (but it is pulled from Enzyme documentation). I'm trying to write a test to validate that a context provider is providing the same context object between render cycles, and only providing a new object when a certain prop or parent context is changed. This involves checking object references after multiple renders of a component.

The above component could easily be changed to have a "pure" render by removing this.count and the issue would still manifest. The documentation says: "Forces a re-render." Apologies if I'm not understanding something here, but by that claim I would expect the render method of a React.Component that does not implement shouldComponentUpdate to get executed.

@ljharb
Copy link
Member

ljharb commented Mar 10, 2019

Hmm, that does make sense. It does stand to reason that a call to update forces a rerender.

@msakrejda
Copy link

Hi, I've also just run into the same issue. I definitely understand that impure renders are a bad idea, but in my case I'm testing something that involves mocking out a render prop component that relies on external state. The real version of the component updates its own state and re-renders, but in the mock it's easier to just craft calls to the children passed in. But that means only changing the mock's implementation of render, which means I need update to force a re-render.

Since you tagged as help wanted, is this a good place to start looking?

@ljharb
Copy link
Member

ljharb commented Mar 15, 2019

@uhoh-itsmaciek yes, but be aware many other methods call into this.update(), and we may not want those to force a rerender.

@msakrejda
Copy link

Interesting. Why would they be calling it? The internal documentation is the same: the method forces a re-render.

@ljharb
Copy link
Member

ljharb commented Mar 15, 2019

They call it when state and/or props change, usually.

I'm just saying we'll have to be careful about it, and that it might be better to refactor so that the current behavior is preserved for all internal update attempts.

@ghbakhtiari
Copy link

Any updates on this?

@ljharb
Copy link
Member

ljharb commented Jul 30, 2019

Nope, it's still open, has the "help wanted" label, and there's no associated PR.

@hannaholl
Copy link

hannaholl commented Aug 7, 2019

I'm having the same issue.

Doing wrapper.instance().forceUpdate() works for me but looking at the documentation I would expect wrapper.update() to do the same. I don't know how "safe" using forceUpdate is but for now it's fixed the problem for me.

@ducin
Copy link

ducin commented Sep 5, 2019

Having the same issue.

Having gone through this thread, the easiest solution is to update the docs, stating that it updates only if there were some changes to props/state. If update is an internal tool used by enzyme, there's no need to adapt it to invalid docs. Adapt docs to what update actually is.

@Hiroki111
Copy link

Event wrapped.instance().forceUpdate() doesn't re-render :(

let wrapped;
beforeEach(() => {
    wrapped = mount(
      <Provider store={store}>
        <Test />
      </Provider>
   );
});

it("disables submit button if it's already uploading a file", () => {
    const uploadButton = wrapped.find("#uploadButton");
    expect(uploadButton.prop("disabled")).toEqual(false);

    wrapped.instance().setState({ uploading: true }, () => {
        wrapped.instance().forceUpdate();
        
        expect(uploadButton.prop("disabled")).toEqual(false);
        // This is supposed to fail, but it passes
    });
});

@ljharb
Copy link
Member

ljharb commented Sep 7, 2019

What does wrapped.debug() look like, before the setState, and after the forceUpdate?

@ghost
Copy link

ghost commented Sep 24, 2019

I'm having the same issue.

Doing wrapper.instance().forceUpdate() works for me but looking at the documentation I would expect wrapper.update() to do the same. I don't know how "safe" using forceUpdate is but for now it's fixed the problem for me.

TypeError: wrapper.instance(...).forceUpdate is not a function ... I got this error

@sameekshakumari
Copy link

Event wrapped.instance().forceUpdate() doesn't re-render :(

let wrapped;
beforeEach(() => {
    wrapped = mount(
      <Provider store={store}>
        <Test />
      </Provider>
   );
});

it("disables submit button if it's already uploading a file", () => {
    const uploadButton = wrapped.find("#uploadButton");
    expect(uploadButton.prop("disabled")).toEqual(false);

    wrapped.instance().setState({ uploading: true }, () => {
        wrapped.instance().forceUpdate();
        
        expect(uploadButton.prop("disabled")).toEqual(false);
        // This is supposed to fail, but it passes
    });
});
```This works

@SwinX
Copy link

SwinX commented Nov 11, 2019

Event wrapped.instance().forceUpdate() doesn't re-render :(

let wrapped;
beforeEach(() => {
    wrapped = mount(
      <Provider store={store}>
        <Test />
      </Provider>
   );
});

it("disables submit button if it's already uploading a file", () => {
    const uploadButton = wrapped.find("#uploadButton");
    expect(uploadButton.prop("disabled")).toEqual(false);

    wrapped.instance().setState({ uploading: true }, () => {
        wrapped.instance().forceUpdate();
        
        expect(uploadButton.prop("disabled")).toEqual(false);
        // This is supposed to fail, but it passes
    });
});
```This works

In your test assertion is never triggered because test runner never has a chance to wait until setState callback call: neither promise was returned from test not test finish callback was used.

@MrMuzyk
Copy link

MrMuzyk commented Nov 21, 2019

I'm having the same issue.

Doing wrapper.instance().forceUpdate() works for me but looking at the documentation I would expect wrapper.update() to do the same. I don't know how "safe" using forceUpdate is but for now it's fixed the problem for me.

Im not 100% sure but i think that:

Enzyme's update() method checks if props/state changed and based on that decides to update component or not.

forceUpdate() in this case is actually React's method. Notice that you don't call it at wrapper but at wrapper.instance(). It causes a re-render no matter what https://reactjs.org/docs/react-component.html#forceupdate.

I'd say that this is acceptable solution.

@shaun-weddell
Copy link

forceUpdate won't work in a functional component though, as there is no instance

@nathan5x
Copy link

@shaun-weddell - You're absolutely right. For React FC - the instance() method will return null. So, obviously we can't call forceUpdate()

@PFight
Copy link

PFight commented Jun 25, 2020

This code works for functional component:

    let component = mount(<test.component />);
    component.setProps({ foo: 42 }); // re-render

Actually, my component even don't need to have a prop foo.

@lquixada
Copy link

lquixada commented Jul 30, 2020

Following @PFight idea, this generic approach worked for me:

const component = mount(<YourComponent {...anyProps} />);
component.setProps(); // Forces react component tree to re-render.
component.update(); // Syncs the enzyme component tree snapshot with the react component tree.

@mmassaki
Copy link

mmassaki commented Jul 31, 2020

Actually, since setProps expect next props to merge, component.setProps() should be enough to trigger a re-render

@lquixada
Copy link

@mmassaki you're right! just fixed the suggestion! thanks!

@travisdahl
Copy link

setProps fixed the problem for me but doesnt seem like a real solution

@ljharb
Copy link
Member

ljharb commented Aug 29, 2020

.update isn't supposed to rerender. It's supposed to update the enzyme tree based on the latest state of the react tree.

In other words, if you haven't changed any props or state, there's nothing to rerender.

@travisdahl
Copy link

in my particular case, its not the "rendering" i care about but its the only way I can get the new mock value to be recognized...

Note: in a beforeEach I am doing the shallow mounting and mocking my module for general use.

in this test I want to mock a different return value. if i dont force the rerender with wrapper.setProps() it never uses this new mock value, but instead the one from the before each.

...

  let wrapper;
  let props;

  beforeEach(() => {
    myMockModule = jest.fn().mockReturnValue({
      fizz: 'buzz',
    });

    wrapper = shallow(<App  />);
  });

 ...


    test('my test', () => {
      myMockModule.mockReturnValueOnce({
        foo: 'bar',
      });
      wrapper.setProps(); // without this it doesnt work
      expect(wrapper).toMatchSnapshot();
    });

@ljharb
Copy link
Member

ljharb commented Aug 29, 2020

What do you mean the new mock value? All mocks should be done before creating the wrapper.

It makes no sense whatsoever to call shallow in a beforeEach. Repetition in tests is good.

@travisdahl
Copy link

travisdahl commented Aug 29, 2020

hmmm interesting.... Ive written a few thousand tests set up like that. 🤣 my bad.

i usually do a shallow in the before each and then set props and state in each test and assert im getting the desired results. gets rid of a lot of boiler plate and ensures I always get a fresh render. why does it "makes no sense whatsoever"? It seems like it is repetition, just automatic? i.e. what is the problem in doing it in a beforeEach if you are gonna do it at the beginning of every test anyway? I'd love to hear more about it, but dont want to hijack this thread either.

@ljharb
Copy link
Member

ljharb commented Aug 29, 2020

The difference is that each test that has a tiny modification can change it without shenanigans like "oops, let me update the wrapper and rerender it", and that failure messages are clearer.

@travisdahl
Copy link

travisdahl commented Aug 29, 2020

yes, i guess my code snippet above proves your point. If i was mocking before i created the wrapper in each test I wouldnt have the problem I do. Thanks!

The crazy part is the setProps actually does fix it though and let me re-mock the value. Maybe just a happy side-effect

@igortas
Copy link

igortas commented Dec 25, 2020

How to force to re-render the component, when I'm updating redux state with store.dispatch(action)?
With wrapper.setState(), I'm updating the wrapper prop(s) with the setted state, but that prop is not propagate down below the tree.
I've tried with wrapper.update(), but does not help.

@xveganxxxedgex
Copy link

xveganxxxedgex commented Jul 28, 2022

I know this is an old issue, but in case anyone ever comes across this issue while trying to test redux useSelector state changes like the comment above mentions, I ended up mocking the selector itself to get the component to have the correct selector state.

Assuming your component has something like this:

const someState = useSelector(getSomeState);

In your test file:

describe('checks some state', () => {
    let stateSpy;

    beforeAll(() => {
        nextActiveStageSpy = jest.spyOn(selectors, "getSomeState").mockImplementation(() => "new-state");
    });

    afterAll(() => {
        nextActiveStageSpy.mockRestore();
    });
    
    ...
}

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

No branches or pull requests