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

[react-test-renderer] componentWillUnmount is not called for inner component instance #8459

Closed
gre opened this issue Nov 30, 2016 · 3 comments
Assignees

Comments

@gre
Copy link
Contributor

gre commented Nov 30, 2016

Do you want to request a feature or report a bug?

bug

What is the current behavior?

componentWillUnmount for inner instances is never called.

Here are 2 (jest) tests, the first works, not the second.

import React from "react";
import renderer from "react-test-renderer";

test("top level componentWillUnmount works", () => {
  let count = 0;
  class A extends React.Component {
    componentWillUnmount() {
      count ++;
    }
    render() {
      return <div />;
    }
  }
  const inst = renderer.create(<A />);
  inst.unmount();
  expect(count).toEqual(1);
});

test("inner componentWillUnmount works", () => {
  let count = 0;
  class A extends React.Component {
    componentWillUnmount() {
      count ++;
    }
    render() {
      return <div />;
    }
  }
  const inst = renderer.create(
    <div>
      <A />
    </div>
  );
  inst.unmount();
  expect(count).toEqual(1);
});

What is the expected behavior?

The two test above should pass. All component that are unmounted should have the proper lifecycle, whatever the nesting is.

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?

React 15.4.0

@sophiebits
Copy link
Collaborator

Ah, unmountComponent in ReactTestRenderer needs to call this.unmountChildren.

@aweary
Copy link
Contributor

aweary commented Dec 3, 2016

I can take a look at this

@aweary aweary self-assigned this Dec 3, 2016
gre added a commit to gre/react that referenced this issue Dec 7, 2016
gre added a commit to gre/react that referenced this issue Dec 7, 2016
@gre
Copy link
Contributor Author

gre commented Dec 7, 2016

This PR should fix it. I also added a test so there is no regression on this.

aweary pushed a commit that referenced this issue Dec 14, 2016
* [react-test-renderer] unmount the inner instances

Fixes #8459

* add a test for #8459

* add new test in tests-passing.txt
bvaughn pushed a commit that referenced this issue Mar 29, 2017
* [react-test-renderer] unmount the inner instances

Fixes #8459

* add a test for #8459

* add new test in tests-passing.txt
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

No branches or pull requests

3 participants