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

Feature request: accept custom setTimeout implementation #16

Closed
papb opened this issue Nov 28, 2020 · 1 comment · Fixed by #17
Closed

Feature request: accept custom setTimeout implementation #16

papb opened this issue Nov 28, 2020 · 1 comment · Fixed by #17

Comments

@papb
Copy link
Contributor

papb commented Nov 28, 2020

Hello. I am refactoring an existing codebase that uses a testing utility called sinon.useFakeTimers(). Calling this method basically replaces functions like setTimeout and clearTimeout in the global scope. I don't like this, but it's unfeasible to change this at this point.

The problem is: to work around another unrelated issue, I want to wrap some of these tests using p-timeout. Since p-timeout (obviously) uses setTimeout and clearTimeout from the global scope, which got replaced by sinon.useFakeTimers(), nothing times out correctly anymore. I would like to be able to make p-timeout keep using the original versions of setTimeout and clearTimeout.

Some ideas I had:

  • Option 1: A new customTimers option could be added to allow the caller to specify custom setTimeout and clearTimeout implementations (and in my case, I would provide the original implementations to it). I would use it like this:
    // my-module.js
    const { setTimeout, clearTimeout } = global;
    // ...
    await pTimeout(foo, 2000, undefined, { setTimeout, clearTimeout });
  • Option 2: A new preventTimerStubbing option that, if true, would use the implementations of setTimeout and clearTimeout that were cached when the module was loaded (instead of reading the global scope directly), making it immune to future replacements of the global implementations. This has the downside that if require('p-timeout') is called too late, the replacements will already have taken place anyway. This wouldn't be a problem for my specific case though.
  • Option 3: Just add const { setTimeout, clearTimeout } = global to the source code directly, making p-timeout immune to this type of mocking (unless the module is required too late, in which case there wouldn't be any difference). The downside is that it might break lots of people's codes, if they are also using sinon.useFakeTimers() and their code (or even any transitive dependency) uses p-timeout internally.

Would you be willing to accept any of these? If yes, which one? Let me know and I will open a PR. I prefer option 1, but it's your call.

Othewise, I understand - I might then just create and use a fork, or patch it with pirates.

Thank you!

@sindresorhus
Copy link
Owner

I'm ok with option 1.

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 a pull request may close this issue.

2 participants