From f6d442fe638186e96a374d5cad0b35155e699983 Mon Sep 17 00:00:00 2001 From: "JiaLi.Passion" Date: Sat, 21 Oct 2017 10:51:44 +0900 Subject: [PATCH 1/3] fix(timer): setInterval should not auto cancel after callback invoked --- lib/common/timers.ts | 6 ++++++ test/common/setInterval.spec.ts | 31 +++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/lib/common/timers.ts b/lib/common/timers.ts index d82f239d7..5f8d9aa1e 100644 --- a/lib/common/timers.ts +++ b/lib/common/timers.ts @@ -39,6 +39,12 @@ export function patchTimer(window: any, setName: string, cancelName: string, nam try { task.invoke.apply(this, arguments); } finally { + if (task.data && task.data.isPeriodic) { + // issue-934, task will be cancelled + // even it is a periodic task such as + // setInterval + return; + } if (typeof data.handleId === NUMBER) { // in non-nodejs env, we remove timerId // from local cache diff --git a/test/common/setInterval.spec.ts b/test/common/setInterval.spec.ts index d62376b1a..8f1fa02ab 100644 --- a/test/common/setInterval.spec.ts +++ b/test/common/setInterval.spec.ts @@ -60,4 +60,35 @@ describe('setInterval', function() { }, null, null, 'unit-test'); }); + it('should not cancel the task after invoke the setInterval callback', (done) => { + const logs: string[] = []; + const hasTaskSpy = jasmine.createSpy('hasTask'); + const zone = Zone.current.fork({ + name: 'interval', + onHasTask: + (delegate: ZoneDelegate, currentZone: Zone, targetZone: Zone, hasTask: HasTaskState) => { + hasTaskSpy(hasTask); + return delegate.hasTask(targetZone, hasTask); + } + }); + + zone.run(() => { + const timerId = setInterval(() => { + logs.push('interval invoked'); + }, 100); + (global as any)[Zone.__symbol__('setTimeout')](() => { + expect(logs.length > 0).toBeTruthy(); + expect(hasTaskSpy) + .toHaveBeenCalledWith( + {microTask: false, macroTask: true, eventTask: false, change: 'macroTask'}); + clearInterval(timerId); + expect(hasTaskSpy.calls.allArgs()).toEqual([ + [{microTask: false, macroTask: true, eventTask: false, change: 'macroTask'}], + [{microTask: false, macroTask: false, eventTask: false, change: 'macroTask'}] + ]); + done(); + }, 300); + }); + }); + }); From 5ceb5fc4fc5d1cf237cdafb8986c4a5d8ae6760a Mon Sep 17 00:00:00 2001 From: "JiaLi.Passion" Date: Sat, 21 Oct 2017 12:44:20 +0900 Subject: [PATCH 2/3] fix rxjs to 5.4.2 --- package.json | 2 +- test/common/setInterval.spec.ts | 20 ++++++++------------ 2 files changed, 9 insertions(+), 13 deletions(-) diff --git a/package.json b/package.json index 160e0c088..b92d0fd4a 100644 --- a/package.json +++ b/package.json @@ -87,7 +87,7 @@ "phantomjs": "^2.1.7", "promises-aplus-tests": "^2.1.2", "pump": "^1.0.1", - "rxjs": "^5.4.2", + "rxjs": "5.4.2", "selenium-webdriver": "^3.4.0", "systemjs": "^0.19.37", "ts-loader": "^0.6.0", diff --git a/test/common/setInterval.spec.ts b/test/common/setInterval.spec.ts index 8f1fa02ab..42aaac7b0 100644 --- a/test/common/setInterval.spec.ts +++ b/test/common/setInterval.spec.ts @@ -61,30 +61,26 @@ describe('setInterval', function() { }); it('should not cancel the task after invoke the setInterval callback', (done) => { - const logs: string[] = []; - const hasTaskSpy = jasmine.createSpy('hasTask'); + const logs: HasTaskState[] = []; const zone = Zone.current.fork({ name: 'interval', onHasTask: (delegate: ZoneDelegate, currentZone: Zone, targetZone: Zone, hasTask: HasTaskState) => { - hasTaskSpy(hasTask); + logs.push(hasTask); return delegate.hasTask(targetZone, hasTask); } }); zone.run(() => { - const timerId = setInterval(() => { - logs.push('interval invoked'); - }, 100); + const timerId = setInterval(() => {}, 100); (global as any)[Zone.__symbol__('setTimeout')](() => { expect(logs.length > 0).toBeTruthy(); - expect(hasTaskSpy) - .toHaveBeenCalledWith( - {microTask: false, macroTask: true, eventTask: false, change: 'macroTask'}); + expect(logs).toEqual( + [{microTask: false, macroTask: true, eventTask: false, change: 'macroTask'}]); clearInterval(timerId); - expect(hasTaskSpy.calls.allArgs()).toEqual([ - [{microTask: false, macroTask: true, eventTask: false, change: 'macroTask'}], - [{microTask: false, macroTask: false, eventTask: false, change: 'macroTask'}] + expect(logs).toEqual([ + {microTask: false, macroTask: true, eventTask: false, change: 'macroTask'}, + {microTask: false, macroTask: false, eventTask: false, change: 'macroTask'} ]); done(); }, 300); From daa6092669053d77bd232f44448444b8997dc826 Mon Sep 17 00:00:00 2001 From: "JiaLi.Passion" Date: Mon, 4 Dec 2017 12:59:44 +0900 Subject: [PATCH 3/3] fix(test): update rxjs to 5.5.3, update RAF test case to avoid timeout --- package.json | 2 +- test/browser/requestAnimationFrame.spec.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index b92d0fd4a..ad30355fb 100644 --- a/package.json +++ b/package.json @@ -87,7 +87,7 @@ "phantomjs": "^2.1.7", "promises-aplus-tests": "^2.1.2", "pump": "^1.0.1", - "rxjs": "5.4.2", + "rxjs": "^5.5.3", "selenium-webdriver": "^3.4.0", "systemjs": "^0.19.37", "ts-loader": "^0.6.0", diff --git a/test/browser/requestAnimationFrame.spec.ts b/test/browser/requestAnimationFrame.spec.ts index f25420d20..63a12540b 100644 --- a/test/browser/requestAnimationFrame.spec.ts +++ b/test/browser/requestAnimationFrame.spec.ts @@ -26,7 +26,7 @@ describe('requestAnimationFrame', function() { it('should bind to same zone when called recursively', function(done) { const originalTimeout: number = (jasmine).DEFAULT_TIMEOUT_INTERVAL; - (jasmine).DEFAULT_TIMEOUT_INTERVAL = 5000; + (jasmine).DEFAULT_TIMEOUT_INTERVAL = 10000; Zone.current.fork({name: 'TestZone'}).run(() => { let frames = 0; let previousTimeStamp = 0;