wrapper.unmount not calling useEffect cleanup

[Johannes-Andersen]

Hey, hope all is well.

Hopefully, this is the right place, but feel free to let me know if I am doing something stupid.

So I have attempted to upgrade to react 17, but found all my tests related to useEffect cleanup's are not passing anymore. It would seem like they are triggered.

Example below is an example where the console log never gets called, and the toHaveBeenCalledTimes is 0.

Here is the test:

  it('should call ruleModel.resetState when unmounted', () => {
    const resetState = jest.fn()
    const ruleModel = {
      resetState: action(resetState),
    }
    const wrapper = renderWrapper({ ruleModel })
    wrapper.unmount()
    expect(resetState).toHaveBeenCalledTimes(1)
  })

And here is the hook:

  useEffect(() => {
    if (ruleId) getRule(ruleId)
    return () => {
      console.log('hello')
      resetState()
    }
  }, [ruleId, getRule, resetState])

Here is a snippet of the renderWrapper function:

import { mount, ReactWrapper } from 'enzyme'

const renderWrapper = ({[...]}: any): ReactWrapper => {

  const store = createStore({[...]})

  return mount(
    <StoreProvider store={store}>
      <NewRuleConditionsPage
        onDiscard={onDiscard}
        ruleId={ruleId}
      />
    </StoreProvider>
  )
}

[ChristianRuiz]

Hi! We are facing the same issue, did you find a workaround it?

[wojtekmaj]

Does it work with React 16 and enzyme-adapter-react-16?

[Johannes-Andersen]

Yeah, used to work with React 16 + enzyme-adapter-react-16.

It would just seem to be a React 17 thing, where the changes when it runs the cleanup: https://reactjs.org/blog/2020/08/10/react-v17-rc.html#effect-cleanup-timing

I was able to get it working with:

it('should xxx when unmounted', (done) => {
    const wrapper = renderWrapper()
    wrapper.unmount()
     setTimeout(() => {
        expect(resetState).toHaveBeenCalledTimes(1)
        done()
     })
  })

But you can also see if this works for you:

it('should xxx when unmounted', async () => {
    const wrapper = renderWrapper()
    wrapper.unmount()
    await flushPromise()
    expect(resetState).toHaveBeenCalledTimes(1)
  })

[wojtekmaj]

I'm afraid this will need to wait for an official package to be resolved

[eps1lon]

You want to wrap the unmount in act just like you would wrap any other update that schedules effects.

[mattcarlotta]

Just a heads up, flushPromises() and setTimeout() don't seem to work consistently with testing stacked asynchronous actions (like a change in state that then calls an API). Enzyme doesn't seem to handle side effects very well. I stumbled across a blog that came up with a polling solution:

import { act } from "react-dom/test-utils";

/**
 * A testing helper function to wait for stacked promises to resolve
 *
 * @param callback - a callback function to invoke after resolving promises
 * @param timeout - amount of time in milliseconds to wait before throwing an error
 * @returns promise
 */
const waitFor = (callback: () => void, timeOut = 1000): Promise<any> =>
  act(
    () =>
      new Promise((resolve, reject) => {
        const startTime = Date.now();

        const tick = () => {
          setTimeout(() => {
            try {
              callback();
              resolve();
            } catch (err) {
              if (Date.now() - startTime > timeOut) {
                reject(err);
              } else {
                tick();
              }
            }
          }, 10);
        };

        tick();
      })
  );

export default waitFor;

An example use case might be...

Demo (kind of janky on codesandbox, but works fine locally):

App.tsx

/* eslint-disable react-hooks/exhaustive-deps */
import * as React from "react";
import isEmpty from "lodash.isempty";
import axios from "./utils/axios";

import "./styles.css";

export interface IExampleState {
  data: Array<{ id: string; name: string }>;
  error: boolean;
  isLoading: boolean;
}

const initialState: IExampleState = {
  data: [],
  error: false,
  isLoading: true
};

export default function App() {
  const [state, setState] = React.useState(initialState);
  const { data, error, isLoading } = state;

  const fetchData = React.useCallback(async (): Promise<void> => {
    try {
      const res = await axios.get("users");

      await new Promise((res) => {
        setTimeout(() => {
          res("");
        }, 1000);
      });

      setState({
        data: res.data,
        error: false,
        isLoading: false
      });
    } catch (err) {
      setState((prevState) => ({
        ...prevState,
        error: true,
        isLoading: false
      }));
    }
  }, []);

  const handleReload = React.useCallback(() => {
    setState(initialState);
  }, []);

  React.useEffect(() => {
    if (isLoading) fetchData();
  }, [isLoading, fetchData]);

  return (
    <>
      <button type="button" onClick={handleReload}>
        Reload
      </button>
      {isLoading ? (
        <p className="loading-data">Loading...</p>
      ) : error ? (
        <p className="error">{error}</p>
      ) : (
        <div className="data-list">
          {!isEmpty(data) &&
            data.map(({ id, name }) => (
              <div className="user" key={id}>
                <h1>Id: {id}</h1>
                <p>Name: {name}</p>
              </div>
            ))}
        </div>
      )}
    </>
  );
}

App.test.tsx

import { mount, ReactWrapper, configure } from "enzyme";
import Adapter from "@wojtekmaj/enzyme-adapter-react-17";
import mockAxios from "./utils/mockAxios";
import waitFor from "./utils/waitFor";
import App from "./App";

configure({ adapter: new Adapter() });

const fakeData = [{ id: "1", name: "Test User" }];
const APIURL = "users";

describe("App", () => {
  let wrapper: ReactWrapper;
  beforeEach(() => {
    wrapper = mount(<App />);
  });

  it("initially renders a loading placeholder", async () => {
    mockAxios.onGet(APIURL).replyOnce(200, fakeData);

    await waitFor(() => {
      expect(wrapper.find("button").exists()).toBeTruthy();
      expect(wrapper.find(".loading-data").exists()).toBeTruthy();
    });
  });

  it("displays an error when API call is unsuccessful and shows data when reloaded", async () => {
    mockAxios.onGet(APIURL).replyOnce(400).onGet(APIURL).reply(200, fakeData);

    await waitFor(() => {
      wrapper.update();
      expect(wrapper.find(".error").exists()).toBeTruthy();
      wrapper.find("button").simulate("click");
    });

    await waitFor(() => {
      wrapper.update();
      expect(wrapper.find(".data-list").exists()).toBeTruthy();
    });
  });

  it("displays data when successfully fetched from API", async () => {
    mockAxios.onGet(APIURL).replyOnce(200, fakeData);

    await waitFor(() => {
      wrapper.update();
      expect(wrapper.find(".data-list").exists()).toBeTruthy();
    }, 2000);
  });
});

[J-Son89]

You want to wrap the unmount in act just like you would wrap any other update that schedules effects.

is there any reason act can't be put in the internal code? wrapper.unmount was working without it before React17.

[eps1lon]

is there any reason act can't be put in the internal code?

That's what I would suggest. It was probably just an oversight in the initial implementation of the 17 adapter. We added act() inside unmount in testing-library/react as well later.

wrapper.unmount was working without it before React17.

React 17 changed the timing of useEffect cleanup functions: https://reactjs.org/blog/2020/08/10/react-v17-rc.html#effect-cleanup-timing

[J-Son89]

is there any reason act can't be put in the internal code?

That's what I would suggest. It was probably just an oversight in the initial implementation of the 17 adapter. We added act() inside unmount in testing-library/react as well later.

wrapper.unmount was working without it before React17.

React 17 changed the timing of useEffect cleanup functions: https://reactjs.org/blog/2020/08/10/react-v17-rc.html#effect-cleanup-timing

I tried this and it worked for me :) @wojtekmaj maybe you can add this in?