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

getTimerCount will not include cancelled immediates #8764

Merged
merged 9 commits into from Aug 18, 2019
17 changes: 17 additions & 0 deletions packages/jest-fake-timers/src/__tests__/jestFakeTimers.test.ts
Expand Up @@ -1303,5 +1303,22 @@ describe('FakeTimers', () => {

expect(timers.getTimerCount()).toEqual(3);
});

it('not includes cancelled immediates', () => {
const timers = new FakeTimers({
config,
global,
moduleMocker,
timerConfig,
});

timers.useFakeTimers();

global.setImmediate(() => {});
expect(timers.getTimerCount()).toEqual(1);
timers.clearAllTimers();

expect(timers.getTimerCount()).toEqual(0);
});
});
});
34 changes: 16 additions & 18 deletions packages/jest-fake-timers/src/jestFakeTimers.ts
Expand Up @@ -53,7 +53,6 @@ const setGlobal = (
};

export default class FakeTimers<TimerRef> {
private _cancelledImmediates!: Record<string, boolean>;
private _cancelledTicks!: Record<string, boolean>;
private _config: StackTraceConfig;
private _disposed?: boolean;
Expand Down Expand Up @@ -105,9 +104,7 @@ export default class FakeTimers<TimerRef> {
}

clearAllTimers() {
this._immediates.forEach(immediate =>
this._fakeClearImmediate(immediate.uuid),
);
this._immediates = [];
this._timers.clear();
}

Expand All @@ -118,7 +115,6 @@ export default class FakeTimers<TimerRef> {

reset() {
this._cancelledTicks = {};
this._cancelledImmediates = {};
this._now = 0;
this._ticks = [];
this._immediates = [];
Expand Down Expand Up @@ -177,11 +173,8 @@ export default class FakeTimers<TimerRef> {
}

private _runImmediate(immediate: Tick) {
if (!this._cancelledImmediates.hasOwnProperty(immediate.uuid)) {
// Callback may throw, so update the map prior calling.
this._cancelledImmediates[immediate.uuid] = true;
immediate.callback();
}
immediate.callback();
jeysal marked this conversation as resolved.
Show resolved Hide resolved
this._fakeClearImmediate(immediate.uuid);
}

runAllTimers() {
Expand Down Expand Up @@ -402,7 +395,9 @@ export default class FakeTimers<TimerRef> {
}

private _fakeClearImmediate(uuid: TimerID) {
this._cancelledImmediates[uuid] = true;
this._immediates = this._immediates.filter(
immediate => immediate.uuid !== uuid,
);
}

private _fakeNextTick(callback: Callback, ...args: Array<any>) {
Expand Down Expand Up @@ -432,19 +427,22 @@ export default class FakeTimers<TimerRef> {
return null;
}

const uuid = this._uuidCounter++;
const uuid = String(this._uuidCounter++);

this._immediates.push({
callback: () => callback.apply(null, args),
uuid: String(uuid),
uuid,
});

const cancelledImmediates = this._cancelledImmediates;
const immediates = this._immediates;
this._timerAPIs.setImmediate(() => {
if (!cancelledImmediates.hasOwnProperty(uuid)) {
// Callback may throw, so update the map prior calling.
cancelledImmediates[String(uuid)] = true;
callback.apply(null, args);
if (immediates.find(x => x.uuid === uuid)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need this if?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please refer to this comment: #8764 (comment)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean, why do we have find at all?

Copy link
Contributor Author

@eranshabi eranshabi Aug 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so we don't run immediates more than once.
If it was cleared (not in the immediates array) then it shouldn't run even if runAllImmediates is called

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, makes sense

try {
callback.apply(null, args);
} catch (error) {
eranshabi marked this conversation as resolved.
Show resolved Hide resolved
} finally {
this._fakeClearImmediate(uuid);
}
}
});

Expand Down