From f67f0809e6c2590d7ccd1f2e677e4c1260c5b7d0 Mon Sep 17 00:00:00 2001 From: "JiaLi.Passion" Date: Sat, 3 Jun 2017 16:24:25 +0900 Subject: [PATCH] fix(xhr): inner onreadystatechange should not triigger Zone callback fix https://github.com/angular/angular/issues/17167, zone.js will patch xhr, and add onreadystatechange internally, this onreadystatechange just for invoke the xhr MacroTask, so it should not trigger ZoneSpec's callback. in this commit, use original native addEventListener/removeEventListener to add internal onreadystatechange event listener. --- lib/browser/browser.ts | 7 +++-- test/browser/XMLHttpRequest.spec.ts | 40 +++++++++++++++++++++++----- test/test-util.ts | 11 ++++++++ test/zone-spec/task-tracking.spec.ts | 6 ++++- 4 files changed, 54 insertions(+), 10 deletions(-) diff --git a/lib/browser/browser.ts b/lib/browser/browser.ts index 63f045df0..397295953 100644 --- a/lib/browser/browser.ts +++ b/lib/browser/browser.ts @@ -91,8 +91,11 @@ Zone.__load_patch('XHR', (global: any, Zone: ZoneType, api: _ZonePrivate) => { const data = task.data; // remove existing event listener const listener = data.target[XHR_LISTENER]; + const oriAddListener = data.target[zoneSymbol('addEventListener')]; + const oriRemoveListener = data.target[zoneSymbol('removeEventListener')]; + if (listener) { - data.target.removeEventListener('readystatechange', listener); + oriRemoveListener.apply(data.target, ['readystatechange', listener]); } const newListener = data.target[XHR_LISTENER] = () => { if (data.target.readyState === data.target.DONE) { @@ -104,7 +107,7 @@ Zone.__load_patch('XHR', (global: any, Zone: ZoneType, api: _ZonePrivate) => { } } }; - data.target.addEventListener('readystatechange', newListener); + oriAddListener.apply(data.target, ['readystatechange', newListener]); const storedTask: Task = data.target[XHR_TASK]; if (!storedTask) { diff --git a/test/browser/XMLHttpRequest.spec.ts b/test/browser/XMLHttpRequest.spec.ts index 67df6d7c1..41f5d62c9 100644 --- a/test/browser/XMLHttpRequest.spec.ts +++ b/test/browser/XMLHttpRequest.spec.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {ifEnvSupports} from '../test-util'; +import {ifEnvSupports, supportPatchXHROnProperty} from '../test-util'; describe('XMLHttpRequest', function() { let testZone: Zone; @@ -26,14 +26,14 @@ describe('XMLHttpRequest', function() { // The last entry in the log should be the invocation for the current onload, // which will vary depending on browser environment. The prior entries // should be the invocation of the send macrotask. - expect(wtfMock.log[wtfMock.log.length - 5]) - .toMatch(/\> Zone\:invokeTask.*addEventListener\:readystatechange/); - expect(wtfMock.log[wtfMock.log.length - 4]) - .toEqual('> Zone:invokeTask:XMLHttpRequest.send("::ProxyZone::WTF::TestZone")'); expect(wtfMock.log[wtfMock.log.length - 3]) - .toEqual('< Zone:invokeTask:XMLHttpRequest.send'); + .toEqual('> Zone:invokeTask:XMLHttpRequest.send("::ProxyZone::WTF::TestZone")'); expect(wtfMock.log[wtfMock.log.length - 2]) - .toMatch(/\< Zone\:invokeTask.*addEventListener\:readystatechange/); + .toEqual('< Zone:invokeTask:XMLHttpRequest.send'); + if (supportPatchXHROnProperty()) { + expect(wtfMock.log[wtfMock.log.length - 1]) + .toMatch(/\> Zone\:invokeTask.*addEventListener\:load/); + } done(); }; @@ -44,6 +44,32 @@ describe('XMLHttpRequest', function() { }, null, null, 'unit-test'); }); + it('should not trigger Zone callback of internal onreadystatechange', function(done) { + const scheduleSpy = jasmine.createSpy('schedule'); + const xhrZone = Zone.current.fork({ + name: 'xhr', + onScheduleTask: (delegate: ZoneDelegate, currentZone: Zone, targetZone, task: Task) => { + if (task.type === 'eventTask') { + scheduleSpy(task.source); + } + return delegate.scheduleTask(targetZone, task); + } + }); + + xhrZone.run(() => { + const req = new XMLHttpRequest(); + req.onload = function() { + expect(Zone.current.name).toEqual('xhr'); + if (supportPatchXHROnProperty()) { + expect(scheduleSpy).toHaveBeenCalled(); + } + done(); + }; + req.open('get', '/', true); + req.send(); + }); + }); + it('should work with onreadystatechange', function(done) { let req: XMLHttpRequest; diff --git a/test/test-util.ts b/test/test-util.ts index 5ff204eb8..180c350f1 100644 --- a/test/test-util.ts +++ b/test/test-util.ts @@ -57,3 +57,14 @@ function _runTest(test: any, block: Function, done: Function) { }); } } + +export function supportPatchXHROnProperty() { + let desc = Object.getOwnPropertyDescriptor(XMLHttpRequest.prototype, 'onload'); + if (!desc && (window as any)['XMLHttpRequestEventTarget']) { + desc = Object.getOwnPropertyDescriptor(global['XMLHttpRequestEventTarget'].prototype, 'onload'); + } + if (!desc || !desc.configurable) { + return false; + } + return true; +} \ No newline at end of file diff --git a/test/zone-spec/task-tracking.spec.ts b/test/zone-spec/task-tracking.spec.ts index 27c1d6c5c..fb4aae36b 100644 --- a/test/zone-spec/task-tracking.spec.ts +++ b/test/zone-spec/task-tracking.spec.ts @@ -8,6 +8,8 @@ import '../../lib/zone-spec/task-tracking'; +import {supportPatchXHROnProperty} from '../test-util'; + declare const global: any; describe('TaskTrackingZone', function() { @@ -47,7 +49,9 @@ describe('TaskTrackingZone', function() { setTimeout(() => { expect(taskTrackingZoneSpec.macroTasks.length).toBe(0); expect(taskTrackingZoneSpec.microTasks.length).toBe(0); - expect(taskTrackingZoneSpec.eventTasks.length).not.toBe(0); + if (supportPatchXHROnProperty()) { + expect(taskTrackingZoneSpec.eventTasks.length).not.toBe(0); + } taskTrackingZoneSpec.clearEvents(); expect(taskTrackingZoneSpec.eventTasks.length).toBe(0); done();