Skip to content

Commit c36c0bc

Browse files
JiaLiPassionmhevery
authored andcommitted
feat: Patch captureStackTrace/prepareStackTrace to ZoneAwareError, patch process.nextTick, fix removeAllListeners bug (angular#516)
* fix long-stack-trace zone will not render correctly when reject a promise * fix issue angular#484 and angular#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
1 parent 94c33d1 commit c36c0bc

File tree

8 files changed

+226
-16
lines changed

8 files changed

+226
-16
lines changed

lib/common/utils.ts

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ function findAllExistingRegisteredTasks(target: any, name: string, capture: bool
155155
const eventTasks: Task[] = target[EVENT_TASKS];
156156
if (eventTasks) {
157157
const result = [];
158-
for (var i = eventTasks.length - 1; i >= 0; i --) {
158+
for (let i = eventTasks.length - 1; i >= 0; i --) {
159159
const eventTask = eventTasks[i];
160160
const data = <ListenerTaskMeta>eventTask.data;
161161
if (data.eventName === name && data.useCapturing === capture) {
@@ -280,17 +280,24 @@ export function makeZoneAwareRemoveAllListeners(fnName: string, useCapturingPara
280280
const defaultUseCapturing = useCapturingParam ? false : undefined;
281281

282282
return function zoneAwareRemoveAllListener(self: any, args: any[]) {
283-
var eventName = args[0];
284-
var useCapturing = args[1] || defaultUseCapturing;
285-
var target = self || _global;
286-
var eventTasks = findAllExistingRegisteredTasks(target, eventName, useCapturing, true);
287-
if (eventTasks) {
288-
for (var i = 0; i < eventTasks.length; i ++) {
289-
var eventTask = eventTasks[i];
290-
eventTask.zone.cancelTask(eventTask);
291-
}
283+
const target = self || _global;
284+
if (args.length === 0) {
285+
// remove all listeners without eventName
286+
target[EVENT_TASKS] = [];
287+
// we don't cancel Task either, because call native eventEmitter.removeAllListeners will
288+
// will do remove listener(cancelTask) for us
289+
target[symbol]();
290+
return;
292291
}
293-
target[symbol](eventName, useCapturing);
292+
const eventName = args[0];
293+
const useCapturing = args[1] || defaultUseCapturing;
294+
// call this function just remove the related eventTask from target[EVENT_TASKS]
295+
findAllExistingRegisteredTasks(target, eventName, useCapturing, true);
296+
// we don't need useCapturing here because useCapturing is just for DOM, and
297+
// removeAllListeners should only be called by node eventEmitter
298+
// and we don't cancel Task either, because call native eventEmitter.removeAllListeners will
299+
// will do remove listener(cancelTask) for us
300+
target[symbol](eventName);
294301
}
295302
}
296303

lib/node/node.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import './events';
1111
import './fs';
1212

1313
import {patchTimer} from '../common/timers';
14+
import {patchMethod} from '../common/utils';
1415

1516
const set = 'set';
1617
const clear = 'clear';
@@ -30,6 +31,7 @@ if (shouldPatchGlobalTimers) {
3031
patchTimer(_global, set, clear, 'Immediate');
3132
}
3233

34+
patchNextTick();
3335

3436
// Crypto
3537
let crypto;
@@ -83,3 +85,28 @@ if (httpClient && httpClient.ClientRequest) {
8385
}
8486
};
8587
}
88+
89+
function patchNextTick() {
90+
let setNative = null;
91+
92+
function scheduleTask(task: Task) {
93+
const args = task.data;
94+
args[0] = function() {
95+
task.invoke.apply(this, arguments);
96+
};
97+
setNative.apply(process, args);
98+
return task;
99+
}
100+
101+
setNative =
102+
patchMethod(process, 'nextTick', (delegate: Function) => function(self: any, args: any[]) {
103+
if (typeof args[0] === 'function') {
104+
const zone = Zone.current;
105+
const task = zone.scheduleMicroTask('nextTick', args[0], args, scheduleTask);
106+
return task;
107+
} else {
108+
// cause an error by calling it directly.
109+
return delegate.apply(process, args);
110+
}
111+
});
112+
}

lib/zone.ts

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1264,7 +1264,7 @@ const Zone: ZoneType = (function(global: any) {
12641264
/// Skip this frame when printing out stack
12651265
blackList,
12661266
/// This frame marks zone transition
1267-
trasition
1267+
transition
12681268
};
12691269
const NativeError = global[__symbol__('Error')] = global.Error;
12701270
// Store the frames which should be removed from the stack frames
@@ -1304,7 +1304,7 @@ const Zone: ZoneType = (function(global: any) {
13041304
if (frameType === FrameType.blackList) {
13051305
frames.splice(i, 1);
13061306
i--;
1307-
} else if (frameType === FrameType.trasition) {
1307+
} else if (frameType === FrameType.transition) {
13081308
if (zoneFrame.parent) {
13091309
// This is the special frame where zone changed. Print and process it accordingly
13101310
frames[i] += ` [${zoneFrame.parent.zone.name} => ${zoneFrame.zone.name}]`;
@@ -1337,6 +1337,21 @@ const Zone: ZoneType = (function(global: any) {
13371337
});
13381338
}
13391339

1340+
if (NativeError.hasOwnProperty('captureStackTrace')) {
1341+
Object.defineProperty(ZoneAwareError, 'captureStackTrace', {
1342+
value: function(targetObject: Object, constructorOpt?: Function) {
1343+
NativeError.captureStackTrace(targetObject, constructorOpt);
1344+
}
1345+
});
1346+
}
1347+
1348+
Object.defineProperty(ZoneAwareError, 'prepareStackTrace', {
1349+
get: function() { return NativeError.prepareStackTrace; },
1350+
set: function(value) { return NativeError.prepareStackTrace = value; }
1351+
});
1352+
1353+
// Now we need to populet the `blacklistedStackFrames` as well as find the
1354+
13401355
// Now we need to populet the `blacklistedStackFrames` as well as find the
13411356
// run/runGuraded/runTask frames. This is done by creating a detect zone and then threading
13421357
// 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) {
13641379
// FireFox: Zone.prototype.run@http://localhost:9876/base/build/lib/zone.js:101:24
13651380
// Safari: run@http://localhost:9876/base/build/lib/zone.js:101:24
13661381
let fnName: string = frame.split('(')[0].split('@')[0];
1367-
let frameType = FrameType.trasition;
1382+
let frameType = FrameType.transition;
13681383
if (fnName.indexOf('ZoneAwareError') !== -1) {
13691384
zoneAwareFrame = frame;
13701385
}

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,6 @@
6767
"ts-loader": "^0.6.0",
6868
"tslint": "^3.15.1",
6969
"typescript": "^2.0.2",
70-
"whatwg-fetch": "https://github.com/jimmywarting/fetch.git"
70+
"whatwg-fetch": "^2.0.1"
7171
}
7272
}

test/node/Error.spec.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/**
2+
* @license
3+
* Copyright Google Inc. All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.io/license
7+
*/
8+
9+
describe('ZoneAwareError', () => {
10+
// If the environment does not supports stack rewrites, then these tests will fail
11+
// and there is no point in running them.
12+
if (!Error['stackRewrite']) return;
13+
14+
it ('should have all properties from NativeError', () => {
15+
let obj: any = new Object();
16+
Error.captureStackTrace(obj);
17+
expect(obj.stack).not.toBeUndefined();
18+
});
19+
20+
it ('should support prepareStackTrace', () => {
21+
(<any>Error).prepareStackTrace = function(error, stack) {
22+
return stack;
23+
}
24+
let obj: any = new Object();
25+
Error.captureStackTrace(obj);
26+
expect(obj.stack[0].getFileName()).not.toBeUndefined();
27+
});
28+
});
29+

test/node/events.spec.ts

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,15 @@ describe('nodejs EventEmitter', () => {
105105
expect(emitter.listeners('test').length).toEqual(0);
106106
});
107107
});
108+
it ('should remove All listeners properly even without a type parameter', () => {
109+
zoneA.run(() => {
110+
emitter.on('test', shouldNotRun);
111+
emitter.on('test1', shouldNotRun);
112+
emitter.removeAllListeners();
113+
expect(emitter.listeners('test').length).toEqual(0);
114+
expect(emitter.listeners('test1').length).toEqual(0);
115+
});
116+
});
108117
it ('should remove once listener after emit', () => {
109118
zoneA.run(() => {
110119
emitter.once('test', expectZoneA);
@@ -119,4 +128,43 @@ describe('nodejs EventEmitter', () => {
119128
emitter.emit('test');
120129
});
121130
});
131+
it ('should trigger removeListener when remove listener', () => {
132+
zoneA.run(() => {
133+
emitter.on('removeListener', function(type, handler) {
134+
zoneResults.push('remove' + type);
135+
});
136+
emitter.on('newListener', function(type, handler) {
137+
zoneResults.push('new' + type);
138+
});
139+
emitter.on('test', shouldNotRun);
140+
emitter.removeListener('test', shouldNotRun);
141+
expect(zoneResults).toEqual(['newtest', 'removetest']);
142+
});
143+
});
144+
it ('should trigger removeListener when remove all listeners with eventname ', () => {
145+
zoneA.run(() => {
146+
emitter.on('removeListener', function(type, handler) {
147+
zoneResults.push('remove' + type);
148+
});
149+
emitter.on('test', shouldNotRun);
150+
emitter.on('test1', expectZoneA);
151+
emitter.removeAllListeners('test');
152+
expect(zoneResults).toEqual(['removetest']);
153+
expect(emitter.listeners('removeListener').length).toBe(1);
154+
});
155+
});
156+
it ('should trigger removeListener when remove all listeners without eventname', () => {
157+
zoneA.run(() => {
158+
emitter.on('removeListener', function(type, handler) {
159+
zoneResults.push('remove' + type);
160+
});
161+
emitter.on('test', shouldNotRun);
162+
emitter.on('test1', shouldNotRun);
163+
emitter.removeAllListeners();
164+
expect(zoneResults).toEqual(['removetest', 'removetest1']);
165+
expect(emitter.listeners('test').length).toBe(0);
166+
expect(emitter.listeners('test1').length).toBe(0);
167+
expect(emitter.listeners('removeListener').length).toBe(0);
168+
});
169+
});
122170
});

test/node/process.spec.ts

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
/**
2+
* @license
3+
* Copyright Google Inc. All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.io/license
7+
*/
8+
9+
describe('process related test', () => {
10+
let zoneA, result;
11+
beforeEach(() => {
12+
zoneA = Zone.current.fork({name: 'zoneA'});
13+
result = [];
14+
});
15+
it('process.nextTick callback should in zone', (done) => {
16+
zoneA.run(function() {
17+
process.nextTick(() => {
18+
expect(Zone.current.name).toEqual('zoneA');
19+
done();
20+
});
21+
});
22+
});
23+
it('process.nextTick should be excuted before macroTask and promise', (done) => {
24+
zoneA.run(function() {
25+
setTimeout(() => {
26+
result.push('timeout');
27+
}, 0);
28+
process.nextTick(() => {
29+
result.push('tick');
30+
});
31+
setTimeout(() => {
32+
expect(result).toEqual(['tick', 'timeout']);
33+
done();
34+
});
35+
});
36+
});
37+
it ('process.nextTick should be treated as microTask', (done) => {
38+
let zoneTick = Zone.current.fork({
39+
name: 'zoneTick',
40+
onScheduleTask: (parentZoneDelegate: ZoneDelegate, currentZone: Zone, targetZone: Zone, task: Task): Task => {
41+
result.push(
42+
{
43+
callback: 'scheduleTask',
44+
targetZone: targetZone.name,
45+
task: task.source
46+
});
47+
return parentZoneDelegate.scheduleTask(targetZone, task);
48+
},
49+
onInvokeTask: (parentZoneDelegate: ZoneDelegate, currentZone: Zone, targetZone: Zone, task: Task, applyThis?: any, applyArgs?: any): any => {
50+
result.push(
51+
{
52+
callback: 'invokeTask',
53+
targetZone: targetZone.name,
54+
task: task.source
55+
});
56+
return parentZoneDelegate.invokeTask(targetZone, task, applyThis, applyArgs);
57+
}
58+
});
59+
zoneTick.run(() => {
60+
process.nextTick(() => {
61+
result.push('tick');
62+
});
63+
});
64+
setTimeout(() => {
65+
expect(result.length).toBe(3);
66+
expect(result[0]).toEqual(
67+
{
68+
callback: 'scheduleTask',
69+
targetZone: 'zoneTick',
70+
task: 'nextTick'
71+
});
72+
expect(result[1]).toEqual(
73+
{
74+
callback: 'invokeTask',
75+
targetZone: 'zoneTick',
76+
task: 'nextTick'
77+
}
78+
);
79+
done();
80+
});
81+
})
82+
});

test/node_tests.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,4 +7,6 @@
77
*/
88

99
import './node/events.spec';
10-
import './node/fs.spec';
10+
import './node/fs.spec';
11+
import './node/process.spec';
12+
import './node/Error.spec';

0 commit comments

Comments
 (0)