diff --git a/packages/zone.js/lib/common/timers.ts b/packages/zone.js/lib/common/timers.ts index e1d68b5a6d..2e8a6d7b79 100644 --- a/packages/zone.js/lib/common/timers.ts +++ b/packages/zone.js/lib/common/timers.ts @@ -29,27 +29,9 @@ export function patchTimer(window: any, setName: string, cancelName: string, nam function scheduleTask(task: Task) { const data = task.data; - function timer(this: unknown) { - try { - task.invoke.apply(this, arguments); - } finally { - // issue-934, task will be cancelled - // even it is a periodic task such as - // setInterval - if (!(task.data && task.data.isPeriodic)) { - if (typeof data.handleId === 'number') { - // in non-nodejs env, we remove timerId - // from local cache - delete tasksByHandleId[data.handleId]; - } else if (data.handleId) { - // Node returns complex objects as handleIds - // we remove task reference from timer object - (data.handleId as any)[taskSymbol] = null; - } - } - } - } - data.args[0] = timer; + data.args[0] = function() { + return task.invoke.apply(this, arguments); + }; data.handleId = setNative!.apply(window, data.args); return task; } @@ -67,6 +49,32 @@ export function patchTimer(window: any, setName: string, cancelName: string, nam undefined, args: args }; + const callback = args[0]; + args[0] = function timer(this: unknown) { + try { + return callback.apply(this, arguments); + } finally { + // issue-934, task will be cancelled + // even it is a periodic task such as + // setInterval + + // https://github.com/angular/angular/issues/40387 + // Cleanup tasksByHandleId should be handled before scheduleTask + // Since some zoneSpec may intercept and doesn't trigger + // scheduleFn(scheduleTask) provided here. + if (!(options.isPeriodic)) { + if (typeof options.handleId === 'number') { + // in non-nodejs env, we remove timerId + // from local cache + delete tasksByHandleId[options.handleId]; + } else if (options.handleId) { + // Node returns complex objects as handleIds + // we remove task reference from timer object + (options.handleId as any)[taskSymbol] = null; + } + } + } + }; const task = scheduleMacroTaskWithCurrentZone(setName, args[0], options, scheduleTask, clearTask); if (!task) { diff --git a/packages/zone.js/test/zone-spec/fake-async-test.spec.ts b/packages/zone.js/test/zone-spec/fake-async-test.spec.ts index 22724223a2..249b871f8e 100644 --- a/packages/zone.js/test/zone-spec/fake-async-test.spec.ts +++ b/packages/zone.js/test/zone-spec/fake-async-test.spec.ts @@ -289,6 +289,40 @@ describe('FakeAsyncTestZoneSpec', () => { }); }); + it('should clear internal timerId cache', () => { + let taskSpy: jasmine.Spy = jasmine.createSpy('taskGetState'); + fakeAsyncTestZone + .fork({ + name: 'scheduleZone', + onScheduleTask: (delegate: ZoneDelegate, curr: Zone, target: Zone, task: Task) => { + (task as any)._state = task.state; + Object.defineProperty(task, 'state', { + configurable: true, + enumerable: true, + get: () => { + taskSpy(); + return (task as any)._state; + }, + set: (newState: string) => { + (task as any)._state = newState; + } + }); + return delegate.scheduleTask(target, task); + } + }) + .run(() => { + const id = setTimeout(() => {}, 0); + testZoneSpec.tick(); + clearTimeout(id); + // This is a hack way to test the timerId cache is cleaned or not + // since the tasksByHandleId cache is an internal variable held by + // zone.js timer patch, if the cache is not cleared, the code in `timer.ts` + // will call `task.state` one more time to check whether to clear the + // task or not, so here we use this count to check the issue is fixed or not + // For details, please refer to https://github.com/angular/angular/issues/40387 + expect(taskSpy.calls.count()).toEqual(5); + }); + }); it('should pass arguments to setImmediate', ifEnvSupports('setImmediate', () => { fakeAsyncTestZone.run(() => { let value = 'genuine value';