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

wrapper.simulate() should be wrapped in ReactTestUtils.act() #2084

Closed
mikesteele opened this issue Apr 5, 2019 · 13 comments · Fixed by #2135
Closed

wrapper.simulate() should be wrapped in ReactTestUtils.act() #2084

mikesteele opened this issue Apr 5, 2019 · 13 comments · Fixed by #2135

Comments

@mikesteele
Copy link

Is your feature request related to a problem? Please describe.
React 16.8 introduced act() to allow component interactions to be tested in a synchronous way.

Describe the solution you'd like
wrapper.simulate() should be wrapped in act() to ensure any state changes are resolved before proceeding.

Describe alternatives you've considered

act(() => {
  wrapper.simulate();
});

Though it's easy to forget act.

Additional context
I have a feeling this is already on your roadmap, but I couldn't find an issue about it.

@ljharb
Copy link
Member

ljharb commented Apr 5, 2019

This is already addressed by #2034.

@ljharb ljharb closed this as completed Apr 5, 2019
@mikesteele
Copy link
Author

Out of curiosity, does that PR mean that any time a mounted component is rendered, it's wrapped in act()? Or only on mount()?

@ljharb
Copy link
Member

ljharb commented Apr 5, 2019

Only mount mounts components, so yes to both?

Shallow rendering does not yet have an “act”, so that’s still unaddressed.

@jquense
Copy link
Contributor

jquense commented May 3, 2019

@ljharb sorry how does #2034 address this? It seems like that only wraps mount() calls in act()? simulate() should also wrap calls, which is not what's happening on the latest published enzyme

@ljharb
Copy link
Member

ljharb commented May 3, 2019

@jquense there's a bunch of unpublished stuff in enzyme, for one, but you're right that simulate probably isn't covered. A PR to fix that would be great.

act requires a DOM, and there's no act that works in a non-browser environment, so it's simply not an option for shallow at this time.

@mikesteele
Copy link
Author

Could you re-open this issue then?

@ljharb
Copy link
Member

ljharb commented May 3, 2019

It’s not fixable by enzyme until the shallow renderer provides an act, but sure.

@ljharb ljharb reopened this May 3, 2019
@mikesteele
Copy link
Author

But it's not fixable for simulate in full DOM rendering?

@ljharb
Copy link
Member

ljharb commented May 3, 2019

Ah, true, that is fixable. PRs welcome :-) even if they only have test cases.

@mikesteele
Copy link
Author

Thanks! I'll try to find time to contribute. I think this will be a huge improvement for users who need to force wrappers to update after simulating events to ensure that state updates are complete.

@jquense
Copy link
Contributor

jquense commented May 3, 2019

@mikesteele let me know if you don't have any time . i'd be happy to put something together as well

@ljharb
Copy link
Member

ljharb commented May 3, 2019

Feel free to race each other to get something up :-p thanks in advance! <3

@mikesteele
Copy link
Author

@jquense I probably won't have time for a few weeks, feel free to take this

petersendidit added a commit to petersendidit/enzyme that referenced this issue May 24, 2019
Wraps the mount simulate method in ReactTestUtils.act() to correctly
allow component interactions to be tested in a synchronous way

Fixes enzymejs#2084
ljharb pushed a commit to petersendidit/enzyme that referenced this issue May 24, 2019
…Utils.act()

Wraps the mount simulate method in ReactTestUtils.act() to correctly
allow component interactions to be tested in a synchronous way

Fixes enzymejs#2084
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

Successfully merging a pull request may close this issue.

3 participants