setState calls within simulate events (async or not?)

[huan-ji]

Been researching several issues regarding this, no clear answer from what I can see.

So from what I've read, React setState can be async at times and is not reliably synchronous. I see that a callback to enzyme's setState has been added to address this. However, this does not address the cases where an event handler calls setState. If a wrapper simulates a click that also sets state, couldn't the event also be async if the setState is async?

The reason I bring this up is that all over the documentation examples the potentially async nature of setState is apparently ignored, all examples treat it as synchronous. For example see link below:
https://github.com/airbnb/enzyme/blob/master/docs/api/ShallowWrapper/simulate.md

Is it fine to treat simulated events as synchronous even though they contain setState calls which can be asynchronous? What's the official word on this?

[ljharb]

Events in the browser are sync.

[huan-ji]

Right but I'm talking about the state changes that happen due to the event from setState, couldn't they be async?

[ljharb]

Yes, sure, any setState call could be async (primarily in React 16+, I believe)

[huan-ji]

Right, so with direct setStates I can use a callback to address that, but setStates in event handlers I can't. However in the example I linked from the docs, it treats the event handler's setState as if it's synchronous.

I'm just wondering how the community deals with this, cause it should be a very common issue, tons of event handlers have setStates within them, all of which could be async. I've seen tons of examples both on and off of the docs and none of them seems to be addressing this, just wondering what I'm missing..

[ljharb]

The issue of sync over async is only an issue for testing, or for "doing things after the setState is done" - the latter is handled by a callback. The former is more complex and depends on your architecture.

[huan-ji]

Okay a more specific example, the example I posted from the documentation above:

expect(wrapper.find('.clicks-0').length).to.equal(1);
wrapper.find('a').simulate('click');
expect(wrapper.find('.clicks-1').length).to.equal(1);

In this case, the click changes the state and happens to be synchronous, so this test happens to work. But this seems to be by "chance" given that setState can be asynchronous. Would you say that the way that this was written in the simulate docs is not how it should be done given the potentially async nature of setState?

[ljharb]

Personally I think simulate should never be used - there's no common, reliable way for the event handler code to communicate to enzyme when its possibly async actions are completed. What I'd recommend is making your event handlers always return a promise; if you setState in them, resolve that promise in the setState callback; and then instead of ever using simulate, get the prop manually and invoke it directly in your tests.

[huan-ji]

Thank you, this helps a bunch.

[glambert]

@huan-ji could you post a snippet of the solution you went for? I'm looking at a way to test event methods with async setState in a React component like so:

handleFocus() {
  this.setState(() => ({ active: true }));
}

And my test:

describe('Events', () => {
  const component = shallow(
    <Component>
      Hello World
    </Component>
  );
  it('sets state to active onFocus', () => {
    component.simulate('focus');
    expect(component.state('active')).toBe(true);
  });
});

[ljharb]

There's no real way to do that, because React could update the state async.

You'd have to make handleFocus take a callback or return a promise, so that your test can know when it should be asserting (and not use simulate at all)

[ryang-bgl]

I got a lot of trouble doing testing with async stuff.

@ljharb
I like the behaviour way of testing using enzyme. So I always use simulate to simulate some user actions, and avoid using setState or change a props, or call a private-ish method of the component in the test. That way, the test could really test the behaviour of the Component, not the implementation details.

To me, the handler is an implementation detail.

But from your suggestion, we should really avoid using simulate at all? Due to the issue probably caused by the async in react?

[ljharb]

Yes. The problem is that "simulate" doesn't actually simulate anything - it just invokes a prop. So, it's better for your test to explicitly invoke a prop.

[craigkovatch]

I am very confused about this one, even after reading this thread. The official documentation for simulate suggests a scheme which is definitively unreliable:

class Foo extends React.Component {
  constructor(props) {
    super(props);
    this.state = { count: 0 };
  }

  render() {
    const { count } = this.state;
    return (
      <div>
        <div className={`clicks-${count}`}>
          {count} clicks
        </div>
        <a href="url" onClick={() => { this.setState({ count: count + 1 }); }}>
          Increment
        </a>
      </div>
    );
  }
}

const wrapper = mount(<Foo />);

expect(wrapper.find('.clicks-0').length).to.equal(1);
wrapper.find('a').simulate('click');
expect(wrapper.find('.clicks-1').length).to.equal(1);

Setting state in an event handler has got to be one of the most (if not the most) common use case for an event handler. Changing all event handlers to return a promise in order to fit Enzyme seems like a huge violation of SOC -- why should my product code be written around Enzyme in this way?

IMO setState on wrapper should always be synchronous. If not -- shouldn't I be able to call wrapper.update() to force sync? (Doesn't seem to be working in my currently-failing tests after upgrading to React 16.8.6/Enzyme 3.9)

@ljharb any advice here would be super helpful. Not using simulate/directly invoking the prop doesn't solve the issue of not being able to write tests around handlers if I can't somehow "force sync".

[ljharb]

The answer is “because React and enzyme don’t and can’t expose the internal JS code your functions have done, unless it exposes a way to do so”.

In other words, you’re not writing around enzyme - you’re writing around the javascript language itself, which is literally your only option to test that code.

[neaumusic]

instead of testing

React --> onClick --> setState

just test

onClick --> setState

for example

myComponent.instance().onChange({ target: { checked: true } });

and then utilize setState's callback:

myComponent.setState({}, () => {
    expect(myComponent.state().checked).toBe(true);
});

[neaumusic]

@ljharb I believe enzyme should flush the entire 'click' event queue -- that element probably has a queue of onClick handlers (maybe just a root handler) or an iterator that will specify it's done when the entire stack is exhausted. enzyme should wrap that event queue as part of simulate('click') before returning to the rest of the thread's execution

[ljharb]

@asyncb that's impossible, because there's no such concept in javascript, and the browser doesn't expose that either.

[neaumusic]

@ljharb I'm 99% sure the event queue is deterministic and ordered. the same as setState({}, cb) guarantees that setState flushed, onClick(cb) could mean all click handlers flushed

[ljharb]

@asyncb yes but in your enzyme tests, or in enzyme itself, one has no way of knowing if the component code sets a timeout that in turn sets another timeout. There is no way in javascript to know that "everything pending" has flushed - jest provides such a mechanism, but that's because it hijacks all of setTimeout and friends before your user code runs to do it.

[alexanderkjeldaas]

@ljharb This issue is not about other timeouts etc, it's about not knowing whether setState in a handler has executed. This is deterministic, and hooking into setState({}, cb) will solve it.

[ljharb]

@alexanderkjeldaas that would only be the case synchronously - again, it is impossible to know when setState calls in an unknown future tick have executed, since the test will have already concluded by then. enzyme already hooks into setState in the way you describe, but it can't violate the way the language works.

[neaumusic]

this just sounds incredibly stupid to me, and I don't want to be disrespectful, but "you don't understand how the language works" and "you have to account for set timeout if you're testing a react event handler" is not helping

similar to cypress, assertions could be automatically retried until a passing state occurs

[ljharb]

@asyncb i'm not trying to be patronizing, but i can assure you, with a significant amount of authority, that this is indeed how the language works. It is utterly impossible for enzyme to handle this without mutating the global environment.

Cypress' approach makes sense for integration tests, but enzyme is a unit test framework - retries are not appropriate.

[neaumusic]

the react update cycle is deterministic, whether or not you think it's utterly impossible to test subsequent states, and enzyme should either care about testing async code or it should not

1. you don't care about async code because it's the programmers responsibility to compensate
2. you care about async code, and should provide hooks for flushing the DOM event queue, waiting for directly synchronous setStates, and potentially gatekeeping by watching for a prop/state combination before making an assertion

unsubscribing from thread

[craigkovatch]

(Modulo the rage-quit) @ljharb I think ^ this might present a reasonable compromise. Maybe enzyme can't guarantee anything, but why not do the "expected" thing for the vast-majority case?

[ljharb]

Because, as I’ve stated, the language simply doesn’t allow the expected thing to be possible for the majority use case.

Literally anything we did here would be rife with assumptions, absolutely would break nondeterministically for nearly everybody. It’s simply not worth it - there’s no getting around that tests can’t magically know what your code is doing.