Repository: sinonjs/fake-timers
Pull request #323 (2020-05-11 13:56:22)

fix: fake all supported timers by default
#### Purpose (TL;DR) - mandatory
It's very weird to me that this method is excluded from default mocking. I've read #126 and to me the solution seems to be for users to specify a `toFake` themselves, not force other people to specify "yes, when I say fake timers I do mean _all_ timers".

Note that this should be considered a breaking change.

Whenever we start supporting some _new_ timer, it should by default not be mocked, but I think when we later make a new major release it should be mocked by default again.

<!--
#### Background (Problem in detail)  - optional
-->
<!--
> When relevant, give a more thorough description of what the problem the PR is trying to solve. Examples of good topics for this section are:
> * Link to an existing GitHub issue describing the problem
> * Describing the problem in greater detail than the TL;DR section above
> * How you discovered the issue if it's not already described as an issue on GitHub
> * Discussion of different approaches to solving this problem and why you chose your proposed solution
-->


<!--
#### Solution  - optional
-->
<!--
> When contributing code (and not just fixing typos, documentation and configuration), please describe why/how your solution works. This helps reviewers spot any mistakes in the implementation.
>
> Example:
> "This solution works by adding a `paintBlue()` method"
> Then your reviewer might spot a mistake in the implementation, if `paintBlue()` uses the colour red.
-->


codecov[bot]: # [Codecov](https://codecov.io/gh/sinonjs/fake-timers/pull/323?src=pr&el=h1) Report
> Merging [#323](https://codecov.io/gh/sinonjs/fake-timers/pull/323?src=pr&el=desc) into [master](https://codecov.io/gh/sinonjs/fake-timers/commit/7f8322effa73a94dc43896cdfb49d63e821b32d1&el=desc) will **decrease** coverage by `0.01%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/sinonjs/fake-timers/pull/323/graphs/tree.svg?width=650&height=150&src=pr&token=YubuW6TySz)](https://codecov.io/gh/sinonjs/fake-timers/pull/323?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #323      +/-   ##
==========================================
- Coverage   92.75%   92.74%   -0.02%     
==========================================
  Files           1        1              
  Lines         552      551       -1     
==========================================
- Hits          512      511       -1     
  Misses         40       40              
```

| Flag | Coverage Δ | |
|---|---|---|
| #unit | `92.74% <100.00%> (-0.02%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/sinonjs/fake-timers/pull/323?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/fake-timers-src.js](https://codecov.io/gh/sinonjs/fake-timers/pull/323/diff?src=pr&el=tree#diff-c3JjL2Zha2UtdGltZXJzLXNyYy5qcw==) | `92.74% <100.00%> (-0.02%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/sinonjs/fake-timers/pull/323?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/sinonjs/fake-timers/pull/323?src=pr&el=footer). Last update [7f8322e...a8a8f7b](https://codecov.io/gh/sinonjs/fake-timers/pull/323?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

benjamingr: So here's the thing: promise libraries and other libraries use nextTick and microticks for scheduling so this is a pretty big breaking change.

In principle I agree that we should mock this by default.
SimenB: > So here's the thing: promise libraries and other libraries use nextTick and microticks for scheduling so this is a pretty big breaking change.

Good point, should probably call it out in the readme.

> In principle I agree that we should mock this by default.

👍 
SimenB: @benjamingr added a section to the readme, feedback appreciated 😀 
benjamingr: @SimenB I am +1 on this change in principle but I know it will break pretty much every fake-timers install "in the wild" that uses the current API.
SimenB: Should work if people use the builtin `Promise` and `async-await`, no? If it breaks builtin `Promise` global I'd consider that a bug in node.

My main issue is that to enable "fake everything" I have to enumerate all timers from the outside, which is not obvious I think. Maybe we could rather add a `fakeNextTick` explicit option (defaulting to `false` I guess) instead of piggy-backing `toFake`?
benjamingr: > Should work if people use the builtin Promise and async-await, no? If it breaks builtin Promise global I'd consider that a bug in node.

It doesn't break the built in promises but mostly because we don't mock its timings at the moment (we probably should with async_hooks). In practice this means that it's a lot harder than it has to to test promises with fake-timers (I recall a lot of discussions about this + async versions of timers somewhere?)

I don't mind seeing what breaks with mocking nextTick on some large codebases I maintain - though I assume I know the answer is "every place that relies on nextTick executing and doesn't .tick or runMicrotasks before that :] 
SimenB: In a work project where I migrated to use Jest's fake timers now that Jest ships with Lolex it broke `node-fetch` (and/or integration with `nock`, not entirely sure)
stale[bot]: This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

mroderick: Stale bot?!
fatso83: This makes my head hurt and you guys know best anyhow. Not sure why it has stalled, though?
fatso83: B,B,B,B,bump? It's not super problematic to release a major version, as long as you also document how to deal with the fallout. Like using node-fetch as an example case, show what needs to be done to fix the test cases. @SimenB 
mroderick: This needs a rebase to resolve the conflicts.

@SimenB do you have time to take this across the finish line? If not, would you be ok with someone else doing it?
fatso83: **I want to finish this (and do #490) and release a new breaking major. Soonish** 

> I don't mind seeing what breaks with mocking nextTick on some large codebases I maintain

There's been 4 years. I guess the number of places that rely on non-native Promises are a lot fewer and further between. Maybe it's not that bad? I think we would just need to add something to the changelog.
fatso83: Since I closed #490 and I haven't heard anything in months, I think I'll just pick this up and release a new breaking major.

Patch:
diff --git a/README.md b/README.md
index ac97210..a7f5ab6 100644
--- a/README.md
+++ b/README.md
@@ -3,19 +3,27 @@
 [![codecov](https://codecov.io/gh/sinonjs/fake-timers/branch/main/graph/badge.svg)](https://codecov.io/gh/sinonjs/fake-timers)
 <a href="CODE_OF_CONDUCT.md"><img src="https://img.shields.io/badge/Contributor%20Covenant-v2.0%20adopted-ff69b4.svg" alt="Contributor Covenant" /></a>
 
-JavaScript implementation of the timer APIs; `setTimeout`, `clearTimeout`, `setImmediate`, `clearImmediate`, `setInterval`, `clearInterval`, `requestAnimationFrame`, `cancelAnimationFrame`, `requestIdleCallback`, and `cancelIdleCallback`, along with a clock instance that controls the flow of time. FakeTimers also provides a `Date` implementation that gets its time from the clock.
+JavaScript implementation of the timer
+APIs; `setTimeout`, `clearTimeout`, `setImmediate`, `clearImmediate`, `setInterval`, `clearInterval`, `requestAnimationFrame`, `cancelAnimationFrame`, `requestIdleCallback`,
+and `cancelIdleCallback`, along with a clock instance that controls the flow of time. FakeTimers also provides a `Date`
+implementation that gets its time from the clock.
 
-In addition in browser environment `@sinonjs/fake-timers` provides a `performance` implementation that gets its time from the clock. In Node environments FakeTimers provides a `nextTick` implementation that is synchronized with the clock - and a `process.hrtime` shim that works with the clock.
+In addition in browser environment `@sinonjs/fake-timers` provides a `performance` implementation that gets its time
+from the clock. In Node environments FakeTimers provides a `nextTick` implementation that is synchronized with the
+clock - and a `process.hrtime` shim that works with the clock.
 
 `@sinonjs/fake-timers` can be used to simulate passing time in automated tests and other
 situations where you want the scheduling semantics, but don't want to actually
 wait.
 
-`@sinonjs/fake-timers` is extracted from [Sinon.JS](https://github.com/sinonjs/sinon.js) and targets the [same runtimes](https://sinonjs.org/releases/latest/#supported-runtimes).
+`@sinonjs/fake-timers` is extracted from [Sinon.JS](https://github.com/sinonjs/sinon.js) and targets
+the [same runtimes](https://sinonjs.org/releases/latest/#supported-runtimes).
 
 ## Autocomplete, IntelliSense and TypeScript definitions
 
-Version 7 introduced JSDoc to the codebase. This should provide autocomplete and type suggestions in supporting IDEs. If you need more elaborate type support, TypeScript definitions for the Sinon projects are independently maintained by the Definitely Types community:
+Version 7 introduced JSDoc to the codebase. This should provide autocomplete and type suggestions in supporting IDEs. If
+you need more elaborate type support, TypeScript definitions for the Sinon projects are independently maintained by the
+Definitely Types community:
 
 ```
 npm install -D @types/sinonjs__fake-timers
@@ -29,7 +37,8 @@ npm install -D @types/sinonjs__fake-timers
 npm install @sinonjs/fake-timers
 ```
 
-If you want to use `@sinonjs/fake-timers` in a browser you can either build your own bundle or use [Skypack](https://www.skypack.dev).
+If you want to use `@sinonjs/fake-timers` in a browser you can either build your own bundle or
+use [Skypack](https://www.skypack.dev).
 
 ## Usage
 
@@ -54,7 +63,8 @@ clock.tick(15);
 
 Upon executing the last line, an interesting fact about the
 [Poblano](https://en.wikipedia.org/wiki/Poblano) will be printed synchronously to
-the screen. If you want to simulate asynchronous behavior, please see the `async` function variants (eg `clock.tick(time)` vs `await clock.tickAsync(time)`).
+the screen. If you want to simulate asynchronous behavior, please see the `async` function variants (
+eg `clock.tick(time)` vs `await clock.tickAsync(time)`).
 
 The `next`, `runAll`, `runToFrame`, and `runToLast` methods are available to advance the clock. See the
 API Reference for more details.
@@ -67,7 +77,9 @@ clock instance, not the browser's internals.
 
 Calling `install` with no arguments achieves this. You can call `uninstall`
 later to restore things as they were again.
-Note that in NodeJS the [timers](https://nodejs.org/api/timers.html) and [timers/promises](https://nodejs.org/api/timers.html#timers-promises-api) modules will also receive fake timers when using global scope.
+Note that in NodeJS the [timers](https://nodejs.org/api/timers.html)
+and [timers/promises](https://nodejs.org/api/timers.html#timers-promises-api) modules will also receive fake timers when
+using global scope.
 
 ```js
 // In the browser distribution, a global `FakeTimers` is already available
@@ -143,22 +155,26 @@ Creates a clock. The default
 
 The `now` argument may be a number (in milliseconds) or a Date object.
 
-The `loopLimit` argument sets the maximum number of timers that will be run when calling `runAll()` before assuming that we have an infinite loop and throwing an error. The default is `1000`.
+The `loopLimit` argument sets the maximum number of timers that will be run when calling `runAll()` before assuming that
+we have an infinite loop and throwing an error. The default is `1000`.
 
 ### `var clock = FakeTimers.install([config])`
 
 Installs FakeTimers using the specified config (otherwise with epoch `0` on the global scope).
-Note that in NodeJS the [timers](https://nodejs.org/api/timers.html) and [timers/promises](https://nodejs.org/api/timers.html#timers-promises-api) modules will also receive fake timers when using global scope.
+Note that in NodeJS the [timers](https://nodejs.org/api/timers.html)
+and [timers/promises](https://nodejs.org/api/timers.html#timers-promises-api) modules will also receive fake timers when
+using global scope.
 The following configuration options are available
 
-| Parameter                        | Type        | Default                                                                                                                                                                                                                        | Description                                                                                                                                                                                                                                                                                  |
-| -------------------------------- | ----------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
-| `config.now`                     | Number/Date | 0                                                                                                                                                                                                                              | installs FakeTimers with the specified unix epoch                                                                                                                                                                                                                                            |
-| `config.toFake`                  | String[]    | ["setTimeout", "clearTimeout", "setImmediate", "clearImmediate","setInterval", "clearInterval", "Date", "requestAnimationFrame", "cancelAnimationFrame", "requestIdleCallback", "cancelIdleCallback", "hrtime", "performance"] | an array with explicit function names (or objects, in the case of "performance") to hijack. _When not set, FakeTimers will automatically fake all methods **except** `nextTick`_ e.g., `FakeTimers.install({ toFake: ["setTimeout","nextTick"]})` will fake only `setTimeout` and `nextTick` |
-| `config.loopLimit`               | Number      | 1000                                                                                                                                                                                                                           | the maximum number of timers that will be run when calling runAll()                                                                                                                                                                                                                          |
-| `config.shouldAdvanceTime`       | Boolean     | false                                                                                                                                                                                                                          | tells FakeTimers to increment mocked time automatically based on the real system time shift (e.g. the mocked time will be incremented by 20ms for every 20ms change in the real system time)                                                                                                 |
-| `config.advanceTimeDelta`        | Number      | 20                                                                                                                                                                                                                             | relevant only when using with `shouldAdvanceTime: true`. increment mocked time by `advanceTimeDelta` ms every `advanceTimeDelta` ms change in the real system time.                                                                                                                          |
-| `config.shouldClearNativeTimers` | Boolean     | false                                                                                                                                                                                                                          | tells FakeTimers to clear 'native' (i.e. not fake) timers by delegating to their respective handlers. These are not cleared by default, leading to potentially unexpected behavior if timers existed prior to installing FakeTimers.                                                         |
+| Parameter                        | Type        | Default                                                                                                                                                                                                                        | Description                                                                                                                                                                                                                                                            |
+| -------------------------------- | ----------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
+| `config.now`                     | Number/Date | 0                                                                                                                                                                                                                              | installs FakeTimers with the specified unix epoch                                                                                                                                                                                                                      |
+| `config.toFake`                  | String[]    | ["setTimeout", "clearTimeout", "setImmediate", "clearImmediate","setInterval", "clearInterval", "Date", "requestAnimationFrame", "cancelAnimationFrame", "requestIdleCallback", "cancelIdleCallback", "hrtime", "performance"] | an array with explicit function names (or objects, in the case of "performance") to hijack. \_When not set, FakeTimers will automatically fake all methods e.g., `FakeTimers.install({ toFake: ["setTimeout","nextTick"]})` will fake only `setTimeout` and `nextTick` |
+| `config.loopLimit`               | Number      | 1000                                                                                                                                                                                                                           | the maximum number of timers that will be run when calling runAll()                                                                                                                                                                                                    |
+| `config.shouldAdvanceTime`       | Boolean     | false                                                                                                                                                                                                                          | tells FakeTimers to increment mocked time automatically based on the real system time shift (e.g. the mocked time will be incremented by 20ms for every 20ms change in the real system time)                                                                           |
+| `config.advanceTimeDelta`        | Number      | 20                                                                                                                                                                                                                             | relevant only when using with `shouldAdvanceTime: true`. increment mocked time by `advanceTimeDelta` ms every `advanceTimeDelta` ms change in the real system time.                                                                                                    |
+| `config.shouldClearNativeTimers` | Boolean     | false                                                                                                                                                                                                                          | tells FakeTimers to clear 'native' (i.e. not fake) timers by delegating to their respective handlers. These are not cleared by default, leading to potentially unexpected behavior if timers existed prior to installing FakeTimers.                                   |
+| `config.ignoreMissingTimers`     | Boolean     | false                                                                                                                                                                                                                          | tells FakeTimers to ignore missing timers that might not exist in the given environment                                                                                                                                                                                |
 
 ### `var id = clock.setTimeout(callback, timeout)`
 
@@ -218,7 +234,9 @@ Cancels the callback scheduled by the provided id.
 
 ### `clock.requestIdleCallback(callback[, timeout])`
 
-Queued the callback to be fired during idle periods to perform background and low priority work on the main event loop. Callbacks which have a timeout option will be fired no later than time in milliseconds. Returns an `id` which can be used to cancel the callback.
+Queued the callback to be fired during idle periods to perform background and low priority work on the main event loop.
+Callbacks which have a timeout option will be fired no later than time in milliseconds. Returns an `id` which can be
+used to cancel the callback.
 
 ### `clock.cancelIdleCallback(id)`
 
@@ -263,7 +281,8 @@ callbacks to execute _before_ running the timers.
 Advance the clock by jumping forward in time, firing callbacks at most once.
 `time` takes the same formats as [`clock.tick`](#clockticktime--await-clocktickasynctime).
 
-This can be used to simulate the JS engine (such as a browser) being put to sleep and resumed later, skipping intermediary timers.
+This can be used to simulate the JS engine (such as a browser) being put to sleep and resumed later, skipping
+intermediary timers.
 
 ### `clock.reset()`
 
@@ -273,9 +292,11 @@ Useful to reset the state of the clock without having to `uninstall` and `instal
 
 ### `clock.runAll()` / `await clock.runAllAsync()`
 
-This runs all pending timers until there are none remaining. If new timers are added while it is executing they will be run as well.
+This runs all pending timers until there are none remaining. If new timers are added while it is executing they will be
+run as well.
 
-This makes it easier to run asynchronous tests to completion without worrying about the number of timers they use, or the delays in those timers.
+This makes it easier to run asynchronous tests to completion without worrying about the number of timers they use, or
+the delays in those timers.
 
 It runs a maximum of `loopLimit` times after which it assumes there is an infinite loop of timers and throws an error.
 
@@ -284,7 +305,8 @@ callbacks to execute _before_ running the timers.
 
 ### `clock.runMicrotasks()`
 
-This runs all pending microtasks scheduled with `nextTick` but none of the timers and is mostly useful for libraries using FakeTimers underneath and for running `nextTick` items without any timers.
+This runs all pending microtasks scheduled with `nextTick` but none of the timers and is mostly useful for libraries
+using FakeTimers underneath and for running `nextTick` items without any timers.
 
 ### `clock.runToFrame()`
 
@@ -323,11 +345,22 @@ Implements the `Date` object but using the clock to provide the correct time.
 
 ### `Performance`
 
-Implements the `now` method of the [`Performance`](https://developer.mozilla.org/en-US/docs/Web/API/Performance/now) object but using the clock to provide the correct time. Only available in environments that support the Performance object (browsers mostly).
+Implements the `now` method of the [`Performance`](https://developer.mozilla.org/en-US/docs/Web/API/Performance/now)
+object but using the clock to provide the correct time. Only available in environments that support the Performance
+object (browsers mostly).
 
 ### `FakeTimers.withGlobal`
 
-In order to support creating clocks based on separate or sandboxed environments (such as JSDOM), FakeTimers exports a factory method which takes single argument `global`, which it inspects to figure out what to mock and what features to support. When invoking this function with a global, you will get back an object with `timers`, `createClock` and `install` - same as the regular FakeTimers exports only based on the passed in global instead of the global environment.
+In order to support creating clocks based on separate or sandboxed environments (such as JSDOM), FakeTimers exports a
+factory method which takes single argument `global`, which it inspects to figure out what to mock and what features to
+support. When invoking this function with a global, you will get back an object with `timers`, `createClock`
+and `install` - same as the regular FakeTimers exports only based on the passed in global instead of the global
+environment.
+
+## Promises and fake time
+
+If you use a Promise library like Bluebird, note that you should either call `clock.runMicrotasks()` or make sure to
+_not_ mock `nextTick`.
 
 ## Running tests
 
diff --git a/src/fake-timers-src.js b/src/fake-timers-src.js
index b7cb1b1..c389223 100644
--- a/src/fake-timers-src.js
+++ b/src/fake-timers-src.js
@@ -1776,10 +1776,7 @@ function withGlobal(_global) {
         clock.methods = config.toFake || [];
 
         if (clock.methods.length === 0) {
-            // do not fake nextTick by default - GitHub#126
-            clock.methods = Object.keys(timers).filter(function (key) {
-                return key !== "nextTick" && key !== "queueMicrotask";
-            });
+            clock.methods = Object.keys(timers);
         }
 
         if (config.shouldAdvanceTime === true) {


Test patch:
diff --git a/test/fake-timers-test.js b/test/fake-timers-test.js
index 80e73eb..c876ab7 100644
--- a/test/fake-timers-test.js
+++ b/test/fake-timers-test.js
@@ -4658,8 +4658,8 @@ describe("FakeTimers", function () {
             assert(typeof timers[0].id !== "undefined");
         });
 
-        it("passes arguments when installed - GitHub#122", function () {
-            const clock = FakeTimers.install({ toFake: ["nextTick"] });
+        it("nextTick passes arguments", function () {
+            const clock = FakeTimers.install();
             let called = false;
             process.nextTick(function (value) {
                 called = value;
@@ -4668,19 +4668,6 @@ describe("FakeTimers", function () {
             assert(called);
             clock.uninstall();
         });
-
-        it("does not install by default - GitHub#126", function (done) {
-            const clock = FakeTimers.install();
-            const spy = sinon.spy(clock, "nextTick");
-            let called = false;
-            process.nextTick(function (value) {
-                called = value;
-                assert(called);
-                assert(!spy.called);
-                clock.uninstall();
-                done();
-            }, true);
-        });
     });
 
     describe("requestIdleCallback", function () {