Unmount doesn't seem to be firing useEffect cleanup functions

Question

Unmount doesn't seem to be firing useEffect cleanup functions

kmarple1 opened this issue 3 years ago · 15 comments

react-hooks-testing-library version: 8.0.0
react version: ^17.0.2
react-dom version (if applicable): ^17.0.2
react-test-renderer version (if applicable): n/a
node version: v14.17.6
npm (or yarn) version: 6.14.15

Relevant code or config:

The component I'm testing:

import { useState, useEffect } from "react";

const useDebounce = (value, delay) => {
    const [debouncedValue, setDebouncedValue] = useState(value);

    useEffect(() => {
        const handler = setTimeout(() => {
            setDebouncedValue(value);
        }, delay);

        return () => {
            clearTimeout(handler);
        };
    }, [value]);

    return debouncedValue;
};

export default useDebounce;

The test itself:

import { renderHook } from "@testing-library/react-hooks";
import useDebounce from "../useDebounce";

jest.useFakeTimers();
jest.spyOn(global, "clearTimeout");

describe("useDebounce", () => {
    test(`it should clear the timeout when unmounting`, () => {
        const value = "test";
        const delay = 1000;
        const { unmount } = renderHook(() => useDebounce(value, delay));
        expect(clearTimeout).not.toHaveBeenCalled();
        unmount();
        expect(clearTimeout).toHaveBeenCalled(); // fails here
    });
});

What you did:

I'm attempting to test that the useEffect cleanup function in my hook fires correctly.

What happened:

While it works properly when running normally, I can't get unmount() to fire it.

Reproduction:

The above example reproduces the problem.

Problem description:

Unmount doesn't seem to be properly cleanup up useEffects.

Answer 1 · 2022-05-27T11:43:03.000Z

I imagine it’s the fake timers getting in the way. Try without that line and see if it works (I’m unable to test it myself at the moment)

Answer 2 · 2022-09-09T18:31:28.000Z

I have the same problem without any fake timers.

I have an RxJS observable which I unsubscribe in the effect cleanup.
Works in the browser, but not when running tests.

Answer 3 · 2022-10-19T10:18:18.000Z

I'm also having this issue, I call a function on the window in my effect cleanup but I can't asset that it's being called as the clean up function never get's called when unmounting.

Is anyone looking into this? Thanks.

Answer 4 · 2022-10-19T23:32:50.000Z

Hi @dahliacreative,

Noone is currently looking into this but we have tests that ensure the effects are cleaned up, so it's unlikely to be an issue with this library or react itself (I cannot stress how much of our functionality is just calling react to do it's thing) and much more likely to be in your code. A common issue I see is that the work in the cleanup is actually async and has not run in between the unmount call and assertions happening.

If you want help diagnosing the issue, I suggest sharing some code here/a new "Question" issue/discord, or putting together a small reproduction repo if you cannot share your actual code with us.

Answer 5 · 2022-11-17T04:52:57.000Z

Have same issue - not firing useEffect cleanup on unmount. Here is the code snippet I am using:

Video.tsx
---------------
React.useEffect(() => {
    video.addEventListener('mouseover', onPlay);
    video.addEventListener('mouseout', onPause);
    return () => {
      video.removeEventListener('mouseover', onPlay);
      video.removeEventListener('mouseout', onPause);
    };
  }, []);

Video.spec.tsx
----------------
it('should call addEventListener on mount', () => {
    const { unmount } = render(<Video src={src} />);
    expect(addEventListener).toHaveBeenCalled();
    unmount();
    expect(removeEventListener).toHaveBeenCalled();
});

Tried wrapping unmount around act, but unmount already does that - so that's not it.
Any help here is appreciated.

Edit:
Okay, this worked for me:

Caching a reference to the useEffect's cleanup function

let cleanupFunc;
jest.spyOn(React, 'useEffect').mockImplementation((f) => {
    cleanupFunc = f();
});

Then finally calling this reference on unmount

it('should call addEventListener on mount', () => {
    const { unmount } = render(<Video src={src} />);
    expect(addEventListener).toHaveBeenCalled();
    unmount();
    cleanupFunc();
    expect(removeEventListener).toHaveBeenCalled();
});

Answer 6 · 2022-11-30T14:11:42.000Z

try skiping one thread cycle before expecting:

it('should call addEventListener on mount', async () => {
    const { unmount } = render(<Video src={src} />);
    expect(addEventListener).toHaveBeenCalled();
    unmount();

    // wait for a moment
    await new Promise((resolve) => setTimeout(resolve, 1));

    expect(removeEventListener).toHaveBeenCalled();
});

Answer 7 · 2023-04-18T12:12:17.000Z

This did not work for me @vzaidman but what did work @kmarple1 was the following:

Component

import { useEffect } from 'react'

const Component = () => {
  useEffect(async () => {
    console.log('renders')
    return () => {
      console.log('cleanup')
    }
  }, [url])

  return <div/>
}

export default Component

  it('unmounts', async () => {
    console.log = jest.fn()
    let cleanupFuncPromise
    jest.spyOn(React, 'useEffect').mockImplementationOnce(func => cleanupFuncPromise = func())
    const subject = await render(<Component/>)
    await subject.unmount()
    const cleanUpFunction = await cleanupFuncPromise
    cleanUpFunction()
    expect(console.log).toHaveBeenCalledWith('cleanup)
  })

Answer 8 · 2023-05-29T05:10:59.000Z

Any updates on this?
I don't want to mock my useEffect as my useEffect is actually making changes in my redux store, and That would turn into alot of work, Im mocking each step just for this.

Answer 9 · 2023-05-29T05:28:14.000Z

Hi @Fawwad-Khan No update from my end. If you want to share some code (or ideally a reproduction repo) I'm may be able to spot something obvious, but I don't have a lot of time for deep debugging on issues like this (I'll reiterate that it's unlikely to be this library causing the issue here).

Also, @ignatospadov, I've just noticed that your usage of useEffect here does not look valid to me. You cannot pass a async function to useEffect like that as far as I'm aware. To be valid it should looks more like:

const Component = () => {
  useEffect(() => {
    const run = async () => {
      console.log('renders')
    }

    run()

    return () => {
      console.log('cleanup')
    }
  }, [url])

  return <div/>
}

Hope that helps.

Answer 10 · 2023-05-29T06:04:29.000Z

I cant really share the code as the organization I work at doesnt let me.
This is what it pretty much looks like.

Im unsetting the UIState on unmount but im still getting the "something" value after unmount in my test.
It works fine in the UI/Implementation.

const MyHook = () => {

const dispatch = useDispatch()
const {UIState} = useSelector(state => ({ UIState: undefined }))

  useEffect(() => {
   dispatch(setUIState("something"))

    return () => {
     dispatch(setUIState(undefined))
    }
  }, [dispatch, setUIState])

  return {
  UIState
  }
}

Answer 11 · 2023-06-23T07:59:04.000Z

Hello, if this can be of any help, here is a reproduction code:

// src/useMount.ts

import { useEffect, useState } from 'react'

export function useMount() {
  const [mounted, setMounted] = useState(false)

  useEffect(() => {
    setMounted(true)
    return () => {
      setMounted(false)
    }
  }, [])

  return mounted
}

// tests/unit/useMount.test.ts

import { renderHook } from '@testing-library/react'
import { useMount } from '../../src/useMount'

describe('useMount', () => {
  it('should be mounted', () => {
    const { result } = renderHook(useMount)
    expect(result.current).toBe(true)
  })

  it('should not be mounted after unmount', () => {
    const { result, unmount } = renderHook(useMount)
    unmount()
    expect(result.current).toBe(false)
  })
})

Result:

Dependencies:

    "@testing-library/dom": "^9.3.1",
    "@testing-library/react": "^14.0.0",
    "@types/jest": "^29.5.2",
    "@types/react": "^18.2.13",
    "jest": "^29.5.0",
    "jest-environment-jsdom": "^29.5.0",
    "react": "^18.2.0",
    "react-dom": "^18.2.0",
    "ts-jest": "^29.1.0",
    "typescript": "^5.1.3"

This code is hosted here: https://github.com/saramorillon/hooks/tree/reproduction-847

Answer 12 · 2024-02-20T18:13:13.000Z

Hello, if this can be of any help, here is a reproduction code:

// src/useMount.ts

import { useEffect, useState } from 'react'

export function useMount() {
  const [mounted, setMounted] = useState(false)

  useEffect(() => {
    setMounted(true)
    return () => {
      setMounted(false)
    }
  }, [])

  return mounted
}

// tests/unit/useMount.test.ts

import { renderHook } from '@testing-library/react'
import { useMount } from '../../src/useMount'

describe('useMount', () => {
  it('should be mounted', () => {
    const { result } = renderHook(useMount)
    expect(result.current).toBe(true)
  })

  it('should not be mounted after unmount', () => {
    const { result, unmount } = renderHook(useMount)
    unmount()
    expect(result.current).toBe(false)
  })
})

Result:

Dependencies:

    "@testing-library/dom": "^9.3.1",
    "@testing-library/react": "^14.0.0",
    "@types/jest": "^29.5.2",
    "@types/react": "^18.2.13",
    "jest": "^29.5.0",
    "jest-environment-jsdom": "^29.5.0",
    "react": "^18.2.0",
    "react-dom": "^18.2.0",
    "ts-jest": "^29.1.0",
    "typescript": "^5.1.3"

This code is hosted here: https://github.com/saramorillon/hooks/tree/reproduction-847

Here is how this hook is implemented on the usehooks-ts library. I hope this example can provide some helpful insights! 👍

Answer 13 · 2024-04-02T15:52:04.000Z

Anyone found the solution, here is my code where issue is happening.

  React.useEffect(() => {
    emitter.on(MY_EVENT,callback);

    return () => { 
      console.log('unsubscribing'); // <<--- Its not getting console logged.
      emitter.off(MY_EVENT, callback);
    };
  }, []);

  it.only('should add and remove event listeners', () => {
    const { unmount } = render(<Component {...defaultProps} />);
    expect(emitter.on).toHaveBeenCalledTimes(1);
    expect(observers[MY_EVENT).toBeDefined();


    unmount();
    expect(emitter.off).toHaveBeenCalledTimes(1);
    expect(observers[MY_EVENT]).toBeUndefined();
  });
});

    expect(jest.fn()).toHaveBeenCalledTimes(expected)

    Expected number of calls: 1
    Received number of calls: 0

      23 |
      24 |     unmount();
    > 25 |     expect(emitter.off).toHaveBeenCalledTimes(1);
          |                         ^
      26 |     expect(observers[MY_EVENT]).toBeUndefined();

Answer 14 · 2024-04-02T16:09:09.000Z

@bhushanlaware it looks like you are rendering with react-testing-library, not react-hooks-testing-library so I suggest raising an issue with them for your case.

As a general reminder to others coming into this issue, if you are importing renderHook from @testing-library/react and not @testing-library/react-hooks then you should also be raising an issue with react-testing-library instead of tacking on a +1 to this issue.

Also a general suspicion that fake timers, unhandled async or mocked useEffect implementations are the likely cause here rather than the testing libraries involved. We generally defer the actual rendering to react itself, so if effects are not being cleanup up, a lot more people would be complaining about it.