From c36c0bcf23ddbc9a5a17eb9d5ed19ee82f7e5d12 Mon Sep 17 00:00:00 2001 From: JiaLiPassion Date: Tue, 6 Dec 2016 04:46:05 +0900 Subject: [PATCH] feat: Patch captureStackTrace/prepareStackTrace to ZoneAwareError, patch process.nextTick, fix removeAllListeners bug (#516) * fix long-stack-trace zone will not render correctly when reject a promise * fix issue #484 and #491, patch EventEmitter.once and removeAllListeners * add event emitter once test case * prependlistener should add listener at the beginning of the listener array. add test cases for prepend listener and once. * use newest fetch to test * patch process.nextTick * restore ZoneAwareError captureStackTrace function * move captureStackTrace test into node * use hasOwnProperty for check captureStackTrace exist or not * change var to const * add process.spec.ts into node_tests.ts target * add done in process.spec.ts * change var to let * add nexttick order case * add prepareStackTrace callback to ZoneAwareError * fix when EventEmitter removeAllListeners has no event type parameter, should remove all listeners * change some var to let/const, remove unnecessary cancelTask call * modify testcases * remove typo * use zone.scheduleMicrotask to patch process.nextTick * forget use let/const again * add comment to removeAllListeners patch, and remove useCapturing parameter cause it is not needed * update fetch to 2.0.1 --- lib/common/utils.ts | 29 ++++++++------ lib/node/node.ts | 27 +++++++++++++ lib/zone.ts | 21 ++++++++-- package.json | 2 +- test/node/Error.spec.ts | 29 ++++++++++++++ test/node/events.spec.ts | 48 +++++++++++++++++++++++ test/node/process.spec.ts | 82 +++++++++++++++++++++++++++++++++++++++ test/node_tests.ts | 4 +- 8 files changed, 226 insertions(+), 16 deletions(-) create mode 100644 test/node/Error.spec.ts create mode 100644 test/node/process.spec.ts diff --git a/lib/common/utils.ts b/lib/common/utils.ts index 50bd721a4..a7b893d09 100644 --- a/lib/common/utils.ts +++ b/lib/common/utils.ts @@ -155,7 +155,7 @@ function findAllExistingRegisteredTasks(target: any, name: string, capture: bool const eventTasks: Task[] = target[EVENT_TASKS]; if (eventTasks) { const result = []; - for (var i = eventTasks.length - 1; i >= 0; i --) { + for (let i = eventTasks.length - 1; i >= 0; i --) { const eventTask = eventTasks[i]; const data = eventTask.data; if (data.eventName === name && data.useCapturing === capture) { @@ -280,17 +280,24 @@ export function makeZoneAwareRemoveAllListeners(fnName: string, useCapturingPara const defaultUseCapturing = useCapturingParam ? false : undefined; return function zoneAwareRemoveAllListener(self: any, args: any[]) { - var eventName = args[0]; - var useCapturing = args[1] || defaultUseCapturing; - var target = self || _global; - var eventTasks = findAllExistingRegisteredTasks(target, eventName, useCapturing, true); - if (eventTasks) { - for (var i = 0; i < eventTasks.length; i ++) { - var eventTask = eventTasks[i]; - eventTask.zone.cancelTask(eventTask); - } + const target = self || _global; + if (args.length === 0) { + // remove all listeners without eventName + target[EVENT_TASKS] = []; + // we don't cancel Task either, because call native eventEmitter.removeAllListeners will + // will do remove listener(cancelTask) for us + target[symbol](); + return; } - target[symbol](eventName, useCapturing); + const eventName = args[0]; + const useCapturing = args[1] || defaultUseCapturing; + // call this function just remove the related eventTask from target[EVENT_TASKS] + findAllExistingRegisteredTasks(target, eventName, useCapturing, true); + // we don't need useCapturing here because useCapturing is just for DOM, and + // removeAllListeners should only be called by node eventEmitter + // and we don't cancel Task either, because call native eventEmitter.removeAllListeners will + // will do remove listener(cancelTask) for us + target[symbol](eventName); } } diff --git a/lib/node/node.ts b/lib/node/node.ts index 92f69b6d0..1c6d6e5d7 100644 --- a/lib/node/node.ts +++ b/lib/node/node.ts @@ -11,6 +11,7 @@ import './events'; import './fs'; import {patchTimer} from '../common/timers'; +import {patchMethod} from '../common/utils'; const set = 'set'; const clear = 'clear'; @@ -30,6 +31,7 @@ if (shouldPatchGlobalTimers) { patchTimer(_global, set, clear, 'Immediate'); } +patchNextTick(); // Crypto let crypto; @@ -83,3 +85,28 @@ if (httpClient && httpClient.ClientRequest) { } }; } + +function patchNextTick() { + let setNative = null; + + function scheduleTask(task: Task) { + const args = task.data; + args[0] = function() { + task.invoke.apply(this, arguments); + }; + setNative.apply(process, args); + return task; + } + + setNative = + patchMethod(process, 'nextTick', (delegate: Function) => function(self: any, args: any[]) { + if (typeof args[0] === 'function') { + const zone = Zone.current; + const task = zone.scheduleMicroTask('nextTick', args[0], args, scheduleTask); + return task; + } else { + // cause an error by calling it directly. + return delegate.apply(process, args); + } + }); +} \ No newline at end of file diff --git a/lib/zone.ts b/lib/zone.ts index 81d065166..8c2787b24 100644 --- a/lib/zone.ts +++ b/lib/zone.ts @@ -1264,7 +1264,7 @@ const Zone: ZoneType = (function(global: any) { /// Skip this frame when printing out stack blackList, /// This frame marks zone transition - trasition + transition }; const NativeError = global[__symbol__('Error')] = global.Error; // Store the frames which should be removed from the stack frames @@ -1304,7 +1304,7 @@ const Zone: ZoneType = (function(global: any) { if (frameType === FrameType.blackList) { frames.splice(i, 1); i--; - } else if (frameType === FrameType.trasition) { + } else if (frameType === FrameType.transition) { if (zoneFrame.parent) { // This is the special frame where zone changed. Print and process it accordingly frames[i] += ` [${zoneFrame.parent.zone.name} => ${zoneFrame.zone.name}]`; @@ -1337,6 +1337,21 @@ const Zone: ZoneType = (function(global: any) { }); } + if (NativeError.hasOwnProperty('captureStackTrace')) { + Object.defineProperty(ZoneAwareError, 'captureStackTrace', { + value: function(targetObject: Object, constructorOpt?: Function) { + NativeError.captureStackTrace(targetObject, constructorOpt); + } + }); + } + + Object.defineProperty(ZoneAwareError, 'prepareStackTrace', { + get: function() { return NativeError.prepareStackTrace; }, + set: function(value) { return NativeError.prepareStackTrace = value; } + }); + + // Now we need to populet the `blacklistedStackFrames` as well as find the + // Now we need to populet the `blacklistedStackFrames` as well as find the // run/runGuraded/runTask frames. This is done by creating a detect zone and then threading // the execution through all of the above methods so that we can look at the stack trace and @@ -1364,7 +1379,7 @@ const Zone: ZoneType = (function(global: any) { // FireFox: Zone.prototype.run@http://localhost:9876/base/build/lib/zone.js:101:24 // Safari: run@http://localhost:9876/base/build/lib/zone.js:101:24 let fnName: string = frame.split('(')[0].split('@')[0]; - let frameType = FrameType.trasition; + let frameType = FrameType.transition; if (fnName.indexOf('ZoneAwareError') !== -1) { zoneAwareFrame = frame; } diff --git a/package.json b/package.json index ff8e4a88f..c86b10d95 100644 --- a/package.json +++ b/package.json @@ -67,6 +67,6 @@ "ts-loader": "^0.6.0", "tslint": "^3.15.1", "typescript": "^2.0.2", - "whatwg-fetch": "https://github.com/jimmywarting/fetch.git" + "whatwg-fetch": "^2.0.1" } } diff --git a/test/node/Error.spec.ts b/test/node/Error.spec.ts new file mode 100644 index 000000000..410acdf6a --- /dev/null +++ b/test/node/Error.spec.ts @@ -0,0 +1,29 @@ +/** + * @license + * Copyright Google Inc. All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +describe('ZoneAwareError', () => { + // If the environment does not supports stack rewrites, then these tests will fail + // and there is no point in running them. + if (!Error['stackRewrite']) return; + + it ('should have all properties from NativeError', () => { + let obj: any = new Object(); + Error.captureStackTrace(obj); + expect(obj.stack).not.toBeUndefined(); + }); + + it ('should support prepareStackTrace', () => { + (Error).prepareStackTrace = function(error, stack) { + return stack; + } + let obj: any = new Object(); + Error.captureStackTrace(obj); + expect(obj.stack[0].getFileName()).not.toBeUndefined(); + }); +}); + diff --git a/test/node/events.spec.ts b/test/node/events.spec.ts index 270131683..ce8c00de2 100644 --- a/test/node/events.spec.ts +++ b/test/node/events.spec.ts @@ -105,6 +105,15 @@ describe('nodejs EventEmitter', () => { expect(emitter.listeners('test').length).toEqual(0); }); }); + it ('should remove All listeners properly even without a type parameter', () => { + zoneA.run(() => { + emitter.on('test', shouldNotRun); + emitter.on('test1', shouldNotRun); + emitter.removeAllListeners(); + expect(emitter.listeners('test').length).toEqual(0); + expect(emitter.listeners('test1').length).toEqual(0); + }); + }); it ('should remove once listener after emit', () => { zoneA.run(() => { emitter.once('test', expectZoneA); @@ -119,4 +128,43 @@ describe('nodejs EventEmitter', () => { emitter.emit('test'); }); }); + it ('should trigger removeListener when remove listener', () => { + zoneA.run(() => { + emitter.on('removeListener', function(type, handler) { + zoneResults.push('remove' + type); + }); + emitter.on('newListener', function(type, handler) { + zoneResults.push('new' + type); + }); + emitter.on('test', shouldNotRun); + emitter.removeListener('test', shouldNotRun); + expect(zoneResults).toEqual(['newtest', 'removetest']); + }); + }); + it ('should trigger removeListener when remove all listeners with eventname ', () => { + zoneA.run(() => { + emitter.on('removeListener', function(type, handler) { + zoneResults.push('remove' + type); + }); + emitter.on('test', shouldNotRun); + emitter.on('test1', expectZoneA); + emitter.removeAllListeners('test'); + expect(zoneResults).toEqual(['removetest']); + expect(emitter.listeners('removeListener').length).toBe(1); + }); + }); + it ('should trigger removeListener when remove all listeners without eventname', () => { + zoneA.run(() => { + emitter.on('removeListener', function(type, handler) { + zoneResults.push('remove' + type); + }); + emitter.on('test', shouldNotRun); + emitter.on('test1', shouldNotRun); + emitter.removeAllListeners(); + expect(zoneResults).toEqual(['removetest', 'removetest1']); + expect(emitter.listeners('test').length).toBe(0); + expect(emitter.listeners('test1').length).toBe(0); + expect(emitter.listeners('removeListener').length).toBe(0); + }); + }); }); \ No newline at end of file diff --git a/test/node/process.spec.ts b/test/node/process.spec.ts new file mode 100644 index 000000000..7e0119608 --- /dev/null +++ b/test/node/process.spec.ts @@ -0,0 +1,82 @@ +/** + * @license + * Copyright Google Inc. All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +describe('process related test', () => { + let zoneA, result; + beforeEach(() => { + zoneA = Zone.current.fork({name: 'zoneA'}); + result = []; + }); + it('process.nextTick callback should in zone', (done) => { + zoneA.run(function() { + process.nextTick(() => { + expect(Zone.current.name).toEqual('zoneA'); + done(); + }); + }); + }); + it('process.nextTick should be excuted before macroTask and promise', (done) => { + zoneA.run(function() { + setTimeout(() => { + result.push('timeout'); + }, 0); + process.nextTick(() => { + result.push('tick'); + }); + setTimeout(() => { + expect(result).toEqual(['tick', 'timeout']); + done(); + }); + }); + }); + it ('process.nextTick should be treated as microTask', (done) => { + let zoneTick = Zone.current.fork({ + name: 'zoneTick', + onScheduleTask: (parentZoneDelegate: ZoneDelegate, currentZone: Zone, targetZone: Zone, task: Task): Task => { + result.push( + { + callback: 'scheduleTask', + targetZone: targetZone.name, + task: task.source + }); + return parentZoneDelegate.scheduleTask(targetZone, task); + }, + onInvokeTask: (parentZoneDelegate: ZoneDelegate, currentZone: Zone, targetZone: Zone, task: Task, applyThis?: any, applyArgs?: any): any => { + result.push( + { + callback: 'invokeTask', + targetZone: targetZone.name, + task: task.source + }); + return parentZoneDelegate.invokeTask(targetZone, task, applyThis, applyArgs); + } + }); + zoneTick.run(() => { + process.nextTick(() => { + result.push('tick'); + }); + }); + setTimeout(() => { + expect(result.length).toBe(3); + expect(result[0]).toEqual( + { + callback: 'scheduleTask', + targetZone: 'zoneTick', + task: 'nextTick' + }); + expect(result[1]).toEqual( + { + callback: 'invokeTask', + targetZone: 'zoneTick', + task: 'nextTick' + } + ); + done(); + }); + }) +}); \ No newline at end of file diff --git a/test/node_tests.ts b/test/node_tests.ts index f8baf9fc8..a70c1c118 100644 --- a/test/node_tests.ts +++ b/test/node_tests.ts @@ -7,4 +7,6 @@ */ import './node/events.spec'; -import './node/fs.spec'; \ No newline at end of file +import './node/fs.spec'; +import './node/process.spec'; +import './node/Error.spec';