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

Add note that mount() requires cleanup #1043

Merged
merged 1 commit into from
Sep 16, 2017
Merged

Add note that mount() requires cleanup #1043

merged 1 commit into from
Sep 16, 2017

Conversation

silvenon
Copy link
Contributor

I was following the discussion at #911 and thought that this might be a useful note to add. I can also incorporate it in the example below if you want.

@lelandrichardson
Copy link
Collaborator

Love it. Thanks!

@lelandrichardson lelandrichardson merged commit 10b9234 into enzymejs:master Sep 16, 2017
@silvenon silvenon deleted the docs-mount branch September 18, 2017 07:09
@Hillshum
Copy link

Should this be extended to the docs for shallow as well? I've found that even using shallow rendering I had tests affecting each other when doing things like window.setInterval

@silvenon
Copy link
Contributor Author

Example?

@Hillshum
Copy link

Here's what I ran into

//component
componentDidMount() {
  this.intervalId = window.setInterval(()=> {this.setState({...})}, 100)
}

componentWillUnmount() {
  window.clearInterval(this.intervalId)
}

// test (this is using lolex for timer control)
it('should clear the interval on timeout', ()=> {
   const updateSpy = jest.spyOn(Timer.prototype, 'updateTimer')
      wrapper = enzyme.shallow(<Timer />)
      clock.tick(200)
      wrapper.unmount()
      updateSpy.mockClear()

      clock.tick(350)

      expect(updateSpy).toHaveBeenCalledTimes(0) // if other shallow wraps are unmounted this fails
      updateSpy.mockRestore()
}

@silvenon
Copy link
Contributor Author

silvenon commented Jun 27, 2018

More context might be more useful, but even looking at this leads me to believe that this test is flaky because you're spying on the prototype, which is shared among all tests using this component. You should be spying on the instance, i.e. the wrapper.

Shallow rendering doesn't output anything to the DOM (it doesn't require DOM at all), therefore it can't require a cleanup.

@Hillshum
Copy link

My thinking was that spying on the instance was better, but I was unable to find a way to access the instance to attach my spy on the proper method before componentDidMount fired, therefore the function registered with setInterval was not spied. Maybe I'm missing something obvious however.

Regardless, even with shallow rendering this test suite registers a function with the setInterval API once for each test that never gets cleaned up without a call to .unmount.

@silvenon
Copy link
Contributor Author

silvenon commented Jun 27, 2018

I haven't been using Enzyme for quite a while so I completely forgot that lifecycle methods work with shallow rendering. This is a good reason to emphasize unmounting in the docs, you should submit a PR. 😉

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