Skip to content

Add act to update call to support hooks #137

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

Merged
merged 3 commits into from
Mar 25, 2019
Merged

Add act to update call to support hooks #137

merged 3 commits into from
Mar 25, 2019

Conversation

brmenchl
Copy link
Contributor

Summary

Currently render and fireEvent are wrapped in act to support hooks. This pull request wraps the update call in act to support testing component updates (e.g. prop changes).

Test plan

  • I have added a test for update in __tests__/act.test.js.
  • A useEffect hook should run again on update calls (if the hook dependency array is satisfied).

Copy link
Member

@thymikee thymikee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is awesome, thank you! I left some small nits, but no biggie

@@ -28,6 +28,14 @@ test('render should trigger useEffect', () => {
expect(effectCallback).toHaveBeenCalledTimes(1);
});

test('updaate should trigger useEffect', () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: update

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oof good catch

src/render.js Outdated
@@ -46,6 +46,15 @@ function renderWithAct(
return ((renderer: any): ReactTestRenderer);
}

function updateWithAct(renderer: ReactTestRenderer) {
function updateImpl(component: React.Element<any>) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can return here :)

@thymikee thymikee requested a review from Esemesek March 23, 2019 21:58
@brmenchl
Copy link
Contributor Author

Thanks for the response! I made a couple updates, let me know if there's anything else outstanding.

Copy link
Member

@thymikee thymikee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good from my side, let's wait for @Esemesek though

@Esemesek Esemesek merged commit f96d782 into callstack:master Mar 25, 2019
@Esemesek
Copy link
Collaborator

Esemesek commented Mar 25, 2019

Thank you for this PR ❤️ ❤️ ❤️

@brmenchl brmenchl deleted the updateWithAct branch March 25, 2019 14:33
@brmenchl
Copy link
Contributor Author

Of course! I've got a couple more coming up in the next few days, more type inference for the typescript typings file and a hook specific API a la react-hooks-testing-library (this might require some discussion).

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 this pull request may close these issues.

3 participants