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

calling runAllTimers after using Lodash's _.debounce results in an infinite recursion error #3465

Closed
rimunroe opened this issue May 4, 2017 · 41 comments · Fixed by #7776
Closed

Comments

@rimunroe
Copy link
Contributor

rimunroe commented May 4, 2017

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

bug

What is the current behavior?

When using fake timers, creating a debounced function, calling it a couple times, and then calling jest.runAllTimers, an error will be printed:

Ran 100000 timers, and there are still more! Assuming we've hit an infinite recursion and bailing out...

  at FakeTimers.runAllTimers (node_modules/jest-util/build/FakeTimers.js:207:13)
  at Object.<anonymous> (__tests__/lodash-bug-test.js:12:8)

It seems that changing the second argument passed to debounce (the time in milliseconds to debounce the function for) changes whether or not this error occurs. For example: on my machine (mid-2014 MBP) it appears to always throw when the delay is above ~600ms, but only fails some of the time when it's around 500ms.

This issue has been encountered before (lodash/lodash#2893), and it seems to have been on Lodash's end. I added a comment to the issue in the Lodash repo, but @jdalton said that he's not sure why it would still be occurring with recent versions of Lodash.

If the current behavior is a bug, please provide the steps to reproduce and either a repl.it demo through https://repl.it/languages/jest or a minimal repository on GitHub that we can yarn install and yarn test.

https://github.com/rimunroe/lodash-jest-timer-issue

What is the expected behavior?

The calling jest.runAllTimers should cause the debounced function to behave as though the time it was told to debounce for elapsed.

Please provide your exact Jest configuration and mention your Jest, node, yarn/npm version and operating system.

I'm using macOS 10.12.4

No configuration other than calling jest.runAllTimers in the test.

I encountered the bug with the following versions:

@thymikee
Copy link
Collaborator

Btw, I hit this too. Turns out lodash's implementation of throttle is way more complex than it (imo) should be. Ended up with a simple helper like this one because I didn't have time to debug what's wrong.

@jdalton
Copy link
Contributor

jdalton commented Jun 20, 2017

Lodash throttles by way of debounce. It's robust and handles things like clock drift after daylights savings time. That said, IMO it's not really Lodash's burden to prop up a mock library. We do our part to be good neighbors and don't hold on to timer references like setTimeout. Beyond that it really depends on your level of mock. For example, you could mock the debounced function itself instead of the underlying timer apis.

@thymikee
Copy link
Collaborator

@jdalton thanks for your input!

@jdalton
Copy link
Contributor

jdalton commented Jun 20, 2017

Since this is a recursive issue you might look into a non-recursive mock that uses a while loop and a queue array of setTimeout invocations to walk through. I've done something similar in the past when mocking out setTimeout to be a synchronous queue.

@taystack
Copy link

taystack commented Sep 6, 2017

@rimunroe I had the same error when using jest.runAllTimers(). I switched to jest.runOnlyPendingTimers() and this fixed my problem.

@hasLandon
Copy link

jest.runOnlyPendingTimers() eliminates the error message for me, but the method is never invoked.

@alexandru-calinoiu
Copy link

I have the same issue, no error message but the method is not invoked.

@jkaipr
Copy link

jkaipr commented Oct 9, 2017

Managed to work around this with a combination of jest.useRealTimers(), setTimeout and done.

  it('debounces the prop', done => {
    wrapper.prop('debounced')('arg1', 'arg2');
    wrapper.prop('debounced')('arg3', 'arg4');
    wrapper.prop('debounced')('arg5', 'arg6');
    setTimeout(() => {
      expect(props.debounced.calls.count()).toBe(1);
      expect(props.debounced).toHaveBeenCalledWith('arg5', 'arg6');
      done();
    }, 1000);
  });

@skray
Copy link

skray commented Nov 3, 2017

I am having this same issue. Using sinon's fake timers I am able to advance the clock and test that a debounced function is called. I am trying to convert to Jest, and using Jest's fake timers, I get Ran 100000 timers, and there are still more! when using jest.runAllTimers, and the function is not invoked when using jest.runOnlyPendingTimers or jest.runTimersToTime.

I am able to use real timers and do something similar to @jkaipr above:

it('triggers debounce', (done) => {
  //set up and call debounced function

  setTimeout(() => {
    //test function was called
    done()
  }, 300 /* however long my debounce is */)
})

@alecklandgraf
Copy link

I was able to get around this by mocking lodash's debounce module

import debounce from 'lodash/debounce'
import { someFunction, someDebouncedFunction } from '../myModule';
jest.mock('lodash/debounce', () => jest.fn(fn => fn));

...

it('tests something', () => {
  debounce.mockClear();
  someFunction = jest.fn();

  someDebouncedFunction();

  expect(debounce).toHaveBeenCalledTimes(1);
  expect(someFunction).toHaveBeenCalledTimes(1);
});

@thymikee
Copy link
Collaborator

Since lodash doesn’t use timers for it’s denounce and throttle I assume this particular issue is not actionable from our side and should be closed.

@cameron-martin
Copy link

What do you mean by not using timers?

@jdalton
Copy link
Contributor

jdalton commented Dec 22, 2017

@thymikee The issue is this.

@rimunroe
Copy link
Contributor Author

rimunroe commented Dec 22, 2017

That’s fine with me

@jdalton
Copy link
Contributor

jdalton commented Dec 22, 2017

@thymikee

Since lodash doesn’t use timers for it’s denounce and throttle I assume this particular issue is not actionable from our side and should be closed.

What timers does Lodash not use?
Lodash uses setTimeout for debounce and throttle.

I'm guessing the issue with jest is it has a problem with mocking setTimeouts that themselves call setTimeout.

For those wanting alternative mocks you might look at sinon.js/lolex.

@thymikee
Copy link
Collaborator

@jdalton looks like I misunderstood what you wrote earlier, sorry about that 😅.
Fake timers support running recursive setTimeouts, but Jest bails out after 100000 calls. Although lodash may hit some edge case.

@SimenB
Copy link
Member

SimenB commented Dec 23, 2017

Should we look into just integrating lolex instead of rolling our own fake timer implementation?

@jdalton
Copy link
Contributor

jdalton commented Dec 23, 2017

@SimenB That would be rad!

@SimenB
Copy link
Member

SimenB commented Dec 23, 2017

Opened up #5165 for it.

@rimunroe
Copy link
Contributor Author

rimunroe commented Jan 25, 2018

I shouldn't have closed this in the first place, so I'm reopening it.

@jakubsadura
Copy link

jakubsadura commented Jun 20, 2018

I had a case where I wanted to test component which used _.debounce in several places and I had to mock implementation of only one usage, I've done it in following way:

const originalDebounce = _.debounce;
jest.spyOn(_, 'debounce').mockImplementation((f, delay) => {
    // can also check if f === yourDebouncedFunction
    if (delay === constants.debounceTime) {
        return f;
    }
    return originalDebounce(f, delay);
});

Hope this will help someone

@tleunen
Copy link

tleunen commented Aug 30, 2018

I'm having the same issue with the 100,000 limit reached and I've found out that the lodash implementation of debounce is creating a ton of timers.
When debouncing a fn with a wait time of 500ms, I can see multiple timers with wait values of 498, then multiple 497 and so on... So I'm not especially surprised that we can reach 100k.

@jdalton is that expected that lodash creates so many timers?
It calls leadingEdge first, and then loop until 0 in timerExpired

vdestraitt pushed a commit to ToucanToco/weaverbird that referenced this issue Jul 29, 2019
Instead of using jest faketimers that don't seem to work directly, (maybe
because jestjs/jest#3465) just mock `_.throttle`
to make it return the throttled function directly.
lizozom pushed a commit to lizozom/kibana that referenced this issue Sep 2, 2019
lizozom pushed a commit to elastic/kibana that referenced this issue Sep 2, 2019
@xevrem
Copy link

xevrem commented Oct 8, 2019

I used this to mock lodash's debounce, it works for most of my use cases:
place the following in __mocks__/lodash/debounce/index.js in your root project directory

export const debounce = jest.fn().mockImplementation((callback, timeout) => {
  let timeoutId = null;
  const debounced = jest.fn(()=>{
    window.clearTimeout(timeoutId);
    timeoutId = window.setTimeout(callback, timeout);
  });

  const cancel = jest.fn(()=>{
    window.clearTimeout(timeoutId);
  });

  debounced.cancel = cancel;
  return debounced;
});

export default debounce;

then just use it with jest's timer mocking and your tests should behave correctly. as always, extend as appropriate :)

matijs added a commit to Ultimaker/react-web-components that referenced this issue Nov 26, 2019
The <SearchBar /> component debounces searches. Normally that would be
tested using Jest's fake timers, however due to this issue
jestjs/jest#3465 it's using real timers and
a timeout of 500ms.

A placeholder prop to be passed onto the <InputText /> was also added.
matijs added a commit to Ultimaker/react-web-components that referenced this issue Nov 27, 2019
The <SearchBar /> component debounces searches. Normally that would be
tested using Jest's fake timers, however due to this issue
jestjs/jest#3465 it's using real timers and
a timeout of 500ms.

A placeholder prop to be passed onto the <InputText /> was also added.
@johnnypesola
Copy link

johnnypesola commented Dec 3, 2019

Thanks @xevrem! But the debounced function is not being passed the correct arguments, I suggest updating your example with this (accept and spread args):

const debounced = jest.fn((...args) => {
  window.clearTimeout(timeoutId);
  timeoutId = window.setTimeout(() => callback(...args), timeout);
});

@SimenB
Copy link
Member

SimenB commented May 4, 2020

Merged a fix 1 day before the issue's 3 year anniversary 😅 Available in [email protected] via jest.useFakeTimers('modern'). next docs: https://jestjs.io/docs/en/next/jest-object#jestusefaketimersimplementation-modern--legacy

@zwbetz-gh
Copy link

Got it to work with sinonjs fake timers. Here's a small sample:

import FakeTimers from '@sinonjs/fake-timers';

let clock;

describe('some tests', () => {

  beforeEach(() => {
    clock = FakeTimers.createClock();
  });

  it('should do a thing', () => {
    const fakeWaitInMillis = 5000;

    // call func that uses debounce

    clock.setTimeout(() => {
      // expect func to be called after wait
    }, fakeWaitInMillis);
  });
  
});

@dsmmcken
Copy link

dsmmcken commented Jun 8, 2020

Another basic mock similar to @xevrem answer but with a mocked .flush() as well, in case you need that.

Also note if you are just importing the lodash.debounce the mock goes in __mocks__/lodash.debounce/index.js

const debounce = jest.fn().mockImplementation((callback, delay) => {
  let timer = null;
  let pendingArgs = null;

  const cancel = jest.fn(() => {
    if (timer) {
      clearTimeout(timer);
    }
    timer = null;
    pendingArgs = null;
  });

  const flush = jest.fn(() => {
    if (timer) {
      callback(...pendingArgs);
      cancel();
    }
  });

  const wrapped = (...args) => {
    cancel();

    pendingArgs = args;
    timer = setTimeout(flush, wrapped.delay);
  };

  wrapped.cancel = cancel;
  wrapped.flush = flush;
  wrapped.delay = delay;

  return wrapped;
});

export default debounce;

HowlingEverett added a commit to everydayhero/react-widgets that referenced this issue Jun 18, 2020
raineorshine added a commit to cybersemics/em that referenced this issue Aug 2, 2020
- Fake timers cause an infinite loop on _.debounce.
- Jest v26 contains a 'modern' option for useFakeTimers, but create-react-app uses an older version of jest
jestjs/jest#3465 (comment)
- Mock debounce
- Test fails without a dummy call to the database. Why?
@SkipCat
Copy link

SkipCat commented Dec 14, 2020

Got this error when using jest.runAllTimers() or jest.advanceTimersByTime(msToRun)

Switching to jest.runOnlyPendingTimers() like @taystack suggested fixed the problem for me and did not prevent the setTimeOuts in my code from being executed

I didn't need lodash mocks or jest.useFakeTimers('modern')

@JasonMore
Copy link

I removed the jest parts from the mock, as it was conflicting with jest.resetAllMocks

src/__mocks__/lodash/debounce/index.ts

const debounce = function MOCKED_debounce(callback, delay) {
  let timer = 0
  let pendingArgs

  const cancel = () => {
    if (timer) {
      clearTimeout(timer)
    }
    timer = 0
    pendingArgs = []
  }

  const flush = () => {
    if (timer) {
      callback(...pendingArgs)
      cancel()
    }
  }

  const wrapped = (...args) => {
    cancel()

    pendingArgs = args
    timer = setTimeout(flush, wrapped.delay)
  }

  wrapped.cancel = cancel
  wrapped.flush = flush
  wrapped.delay = delay

  return wrapped
}

export default debounce

@esslamben
Copy link

So whats the actual recommended way of dealing with this?

@SimenB
Copy link
Member

SimenB commented Jan 11, 2021

#3465 (comment)

Modern timers will be the default in jest@27 (currently available as jest@next)

@esslamben
Copy link

Thanks for the reply @SimenB, seem to still run into the problem even with jest.useFakeTimers('modern'); so I've opt'd to just mock the lodash debounce function as I couldn't get it to work.

Would love to figure it out though as the point at which it fails it throws a nasty error with Ran 100000 timers, and there are still more! Assuming we've hit an infinite recursion and bailing out.... Think I have a pretty unique use case where the debounce function actually calls an async request which I'm awaiting by calling await waitFor(() => expect(apiFunc).toHaveBeenCalledTimes(1)); This never gets called but interestingly a log in the function calling the debounced version is logging so I'm sure its a timing issue with the test expecting a result.

For anybody wishing to mock it I used @alecklandgraf jest.mock('lodash/debounce', () => jest.fn(fn => fn));

@SimenB
Copy link
Member

SimenB commented Jan 11, 2021

Please open up a new issue with a minimal reproduction if you're stilling having issues

@chestozo
Copy link

chestozo commented Jan 23, 2021

little addition to @xevrem version here
(adding args support):

export const debounce = jest.fn().mockImplementation((callback, timeout) => {
  let timeoutId = null;
  const debounced = jest.fn((...args)=>{
    window.clearTimeout(timeoutId);
    timeoutId = window.setTimeout(() => callback(...args), timeout);
  });

  const cancel = jest.fn(()=>{
    window.clearTimeout(timeoutId);
  });

  debounced.cancel = cancel;
  return debounced;
});

export default debounce;

@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet