Skip to content

Commit d1ac3aa

Browse files
JiaLiPassionAndrewKushnir
authored andcommitted
fix(zone.js): zone-node only patch Promise.prototype.then (#49144)
Close #47872 zone-node only patches `Promise.prototype.then` instead of patch `Promise` itself. So the new NodeJS `SafePromise` will not complain the Promise.prototype.then called on incompatible receiver. We should also do this change on browser zone.js patch, but it will be a big breaking change, because Promise.prototype.then will not work with `fakeAsync` any longer. PR Close #49144
1 parent 74258f3 commit d1ac3aa

File tree

6 files changed

+136
-4
lines changed

6 files changed

+136
-4
lines changed

packages/zone.js/bundles.bzl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ BUNDLES_ENTRY_POINTS = {
1414
"entrypoint": _DIR + "mix/rollup-mix",
1515
},
1616
"zone-node": {
17-
"entrypoint": _DIR + "node/rollup-main",
17+
"entrypoint": _DIR + "node/rollup-main-node-bundle",
1818
},
1919
"async-test": {
2020
"entrypoint": _DIR + "testing/async-testing",
Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC 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+
/**
10+
* A different implementation for monkey patching Promise.
11+
* Currently Zone.js patches Promise itself with ZoneAwarePromise and also Promise.prototype.then
12+
* The reason is:
13+
*
14+
* 1. Promise.prototype.then should trigger ZoneSpec.scheduleTask and acts as a microTask
15+
* 2. Promise should be able to controlled by fakeAsync(), so Promise.prototype.then can work as
16+
* sync operation in fakeAsync()
17+
* 3. Promise unhandledRejection can be handled by ZoneSpec.onHandleError hook
18+
*
19+
* And this implementation also has it's disadvantage.
20+
*
21+
* 1. We need to implement a full Promise spec by ourselves.
22+
* 2. We need to implement the new APIs for Promise such as (all, allSettled, any...) when the new
23+
* APIs are available.
24+
* 3. Promise behavior is different with the native one, such as the timing of then callback.
25+
* 4. Can not handle the some vm operation requires native Promise such as async/await or
26+
* SafePromise.
27+
*
28+
* And this new implementation try to address most of these disadvantages.
29+
* 1. We don't monkey patch Promise itself any longer.
30+
* 2. We only monkey patches Promise.prototype.then and schedule microTask from there.
31+
* 3. The Promise APIs are all using native ones.
32+
* 4. SafePromise issues are gone, and the timing will be the same with the native one.
33+
*
34+
* Also this new implementation introduces breaking changes.
35+
*
36+
* 1. Promise can not be easily handled by fakeAsync(), and since we will deprecate fakeAsync() in
37+
* the future, this is the first step.
38+
* 2. Promise unhandled rejection happened inside new Promise(callback) will not be handled by
39+
* ZoneSpec.onHandleError(thenCallback error will not be be impacted).
40+
*
41+
* So now we only introduces this change to `zone-node` bundle, since the breaking change will be
42+
* minor for NodeJS environment,
43+
* @TODO: JiaLiPassion, we will introduce this change to browser later.
44+
*/
45+
Zone.__load_patch('ZoneAwarePromise', (global: any, Zone: ZoneType, api: _ZonePrivate) => {
46+
const __symbol__ = api.symbol;
47+
const symbolThen = __symbol__('then');
48+
49+
api.onUnhandledError = (e: any) => {
50+
if (api.showUncaughtError()) {
51+
const rejection = e && e.rejection;
52+
if (rejection) {
53+
console.error(
54+
'Unhandled Promise rejection:',
55+
rejection instanceof Error ? rejection?.message : rejection,
56+
'; Zone:', (<Zone>e.zone).name, '; Task:', e.task && (<Task>e.task).source,
57+
'; Value:', rejection, rejection instanceof Error ? rejection.stack : undefined);
58+
} else {
59+
console.error(e);
60+
}
61+
}
62+
};
63+
64+
api.microtaskDrainDone = () => {};
65+
66+
const symbolThenPatched = __symbol__('thenPatched');
67+
68+
function patchThen(Ctor: Function) {
69+
const proto = Ctor.prototype;
70+
if ((Ctor as any)[symbolThenPatched] === true) {
71+
return;
72+
}
73+
74+
const prop = Object.getOwnPropertyDescriptor(proto, 'then');
75+
if (prop && (prop.writable === false || !prop.configurable)) {
76+
// check Ctor.prototype.then propertyDescriptor is writable or not
77+
// in meteor env, writable is false, we should ignore such case
78+
return;
79+
}
80+
81+
const originalThen = proto.then;
82+
// Keep a reference to the original method.
83+
proto[symbolThen] = originalThen;
84+
85+
const makeResolver = function(resolveFunc: any, zone: Zone, source: string, isReject: boolean) {
86+
if (!resolveFunc) {
87+
return resolveFunc;
88+
}
89+
return (val: any) => {
90+
const task = zone.scheduleMicroTask(source, () => {
91+
return typeof resolveFunc === 'function' ? resolveFunc(val) :
92+
(isReject ? Promise.reject(val) : val);
93+
}, undefined, () => {});
94+
return zone.runGuarded(() => {
95+
return task.invoke();
96+
}, undefined, []);
97+
};
98+
};
99+
100+
Ctor.prototype.then = function(onResolve: any, onReject: any) {
101+
const zone = Zone.current;
102+
return originalThen.call(
103+
this, makeResolver(onResolve, zone, 'Promise.prototype.then', false),
104+
makeResolver(onReject, zone, 'Promise.prototype.reject', true));
105+
};
106+
(Ctor as any)[symbolThenPatched] = true;
107+
}
108+
109+
api.patchThen = patchThen;
110+
111+
if (Promise) {
112+
patchThen(Promise);
113+
}
114+
115+
global[api.symbol('Promise')] = Promise;
116+
return Promise;
117+
});
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC 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+
import '../zone';
10+
import './promise';
11+
import '../common/to-string';
12+
import './node';

packages/zone.js/lib/node/rollup-test-main.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import './rollup-main';
10-
9+
import './rollup-main-node-bundle';
1110
// load test related files into bundle
1211
import '../testing/zone-testing';

packages/zone.js/test/promise/promise-adapter.mjs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,4 +18,8 @@ const rejected = (reason) => {
1818
return Promise.reject(reason);
1919
};
2020

21+
process.on('unhandledRejection', (error) => {
22+
console.log('unhandled', error);
23+
});
24+
2125
export default {deferred, resolved, rejected};

packages/zone.js/test/promise/promise-test.mjs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import promisesAplusTests from 'promises-aplus-tests';
22
import adapter from './promise-adapter.mjs';
33

4-
promisesAplusTests(adapter, {reporter: 'dot'}, function (err) {
4+
promisesAplusTests(adapter, {reporter: 'dot', timeout: 500}, function (err) {
55
if (err) {
66
console.error(err);
77
process.exit(1);

0 commit comments

Comments
 (0)