From 6994ea705751dea50308fb76c72012efe0a3680a Mon Sep 17 00:00:00 2001 From: Alan Agius Date: Fri, 2 Jun 2023 10:53:30 +0000 Subject: [PATCH] fix(zone.js): path entire promise in node In https://github.com/angular/angular/pull/49144 we introduced a change to only path `Promise.prototype.then` due to Node.js `SafePromise` complaining about `Promise.prototype.then` called on incompatible receiver. This however introduced a number of regressions. This commit reverts this change and re-introduces the changes to path the entire promise on Node. The original problem `SafePromise` problem is no longer reproducible as of Node.js version 18.13+ as it was addressed as part of https://github.com/nodejs/node/pull/45175 While the Angular CLI does not yet generate ESM server bundles, users using ESM with dynamic imports will require using Node.js 18.13 or later. Closes #50513, closes #50457, closes #50414 and closes #49930 --- packages/zone.js/bundles.bzl | 2 +- packages/zone.js/lib/node/promise.ts | 117 ------------------ .../lib/node/rollup-main-node-bundle.ts | 12 -- packages/zone.js/lib/node/rollup-test-main.ts | 2 +- .../zone.js/test/promise/promise-adapter.mjs | 4 - .../zone.js/test/promise/promise-test.mjs | 2 +- 6 files changed, 3 insertions(+), 136 deletions(-) delete mode 100644 packages/zone.js/lib/node/promise.ts delete mode 100644 packages/zone.js/lib/node/rollup-main-node-bundle.ts diff --git a/packages/zone.js/bundles.bzl b/packages/zone.js/bundles.bzl index b889647523a4db..90ecfac59a29fa 100644 --- a/packages/zone.js/bundles.bzl +++ b/packages/zone.js/bundles.bzl @@ -14,7 +14,7 @@ BUNDLES_ENTRY_POINTS = { "entrypoint": _DIR + "mix/rollup-mix", }, "zone-node": { - "entrypoint": _DIR + "node/rollup-main-node-bundle", + "entrypoint": _DIR + "node/rollup-main", }, "async-test": { "entrypoint": _DIR + "testing/async-testing", diff --git a/packages/zone.js/lib/node/promise.ts b/packages/zone.js/lib/node/promise.ts deleted file mode 100644 index 701c5eefe5d296..00000000000000 --- a/packages/zone.js/lib/node/promise.ts +++ /dev/null @@ -1,117 +0,0 @@ -/** - * @license - * Copyright Google LLC 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 - */ - -/** - * A different implementation for monkey patching Promise. - * Currently Zone.js patches Promise itself with ZoneAwarePromise and also Promise.prototype.then - * The reason is: - * - * 1. Promise.prototype.then should trigger ZoneSpec.scheduleTask and acts as a microTask - * 2. Promise should be able to controlled by fakeAsync(), so Promise.prototype.then can work as - * sync operation in fakeAsync() - * 3. Promise unhandledRejection can be handled by ZoneSpec.onHandleError hook - * - * And this implementation also has it's disadvantage. - * - * 1. We need to implement a full Promise spec by ourselves. - * 2. We need to implement the new APIs for Promise such as (all, allSettled, any...) when the new - * APIs are available. - * 3. Promise behavior is different with the native one, such as the timing of then callback. - * 4. Can not handle the some vm operation requires native Promise such as async/await or - * SafePromise. - * - * And this new implementation try to address most of these disadvantages. - * 1. We don't monkey patch Promise itself any longer. - * 2. We only monkey patches Promise.prototype.then and schedule microTask from there. - * 3. The Promise APIs are all using native ones. - * 4. SafePromise issues are gone, and the timing will be the same with the native one. - * - * Also this new implementation introduces breaking changes. - * - * 1. Promise can not be easily handled by fakeAsync(), and since we will deprecate fakeAsync() in - * the future, this is the first step. - * 2. Promise unhandled rejection happened inside new Promise(callback) will not be handled by - * ZoneSpec.onHandleError(thenCallback error will not be be impacted). - * - * So now we only introduces this change to `zone-node` bundle, since the breaking change will be - * minor for NodeJS environment, - * @TODO: JiaLiPassion, we will introduce this change to browser later. - */ -Zone.__load_patch('ZoneAwarePromise', (global: any, Zone: ZoneType, api: _ZonePrivate) => { - const __symbol__ = api.symbol; - const symbolThen = __symbol__('then'); - - api.onUnhandledError = (e: any) => { - if (api.showUncaughtError()) { - const rejection = e && e.rejection; - if (rejection) { - console.error( - 'Unhandled Promise rejection:', - rejection instanceof Error ? rejection?.message : rejection, - '; Zone:', (e.zone).name, '; Task:', e.task && (e.task).source, - '; Value:', rejection, rejection instanceof Error ? rejection.stack : undefined); - } else { - console.error(e); - } - } - }; - - api.microtaskDrainDone = () => {}; - - const symbolThenPatched = __symbol__('thenPatched'); - - function patchThen(Ctor: Function) { - const proto = Ctor.prototype; - if ((Ctor as any)[symbolThenPatched] === true) { - return; - } - - const prop = Object.getOwnPropertyDescriptor(proto, 'then'); - if (prop && (prop.writable === false || !prop.configurable)) { - // check Ctor.prototype.then propertyDescriptor is writable or not - // in meteor env, writable is false, we should ignore such case - return; - } - - const originalThen = proto.then; - // Keep a reference to the original method. - proto[symbolThen] = originalThen; - - const makeResolver = function(resolveFunc: any, zone: Zone, source: string, isReject: boolean) { - if (!resolveFunc) { - return resolveFunc; - } - return (val: any) => { - const task = zone.scheduleMicroTask(source, () => { - return typeof resolveFunc === 'function' ? resolveFunc(val) : - (isReject ? Promise.reject(val) : val); - }, undefined, () => {}); - return zone.runGuarded(() => { - return task.invoke(); - }, undefined, []); - }; - }; - - Ctor.prototype.then = function(onResolve: any, onReject: any) { - const zone = Zone.current; - return originalThen.call( - this, makeResolver(onResolve, zone, 'Promise.prototype.then', false), - makeResolver(onReject, zone, 'Promise.prototype.reject', true)); - }; - (Ctor as any)[symbolThenPatched] = true; - } - - api.patchThen = patchThen; - - if (Promise) { - patchThen(Promise); - } - - global[api.symbol('Promise')] = Promise; - return Promise; -}); diff --git a/packages/zone.js/lib/node/rollup-main-node-bundle.ts b/packages/zone.js/lib/node/rollup-main-node-bundle.ts deleted file mode 100644 index a151e1df4b56c2..00000000000000 --- a/packages/zone.js/lib/node/rollup-main-node-bundle.ts +++ /dev/null @@ -1,12 +0,0 @@ -/** - * @license - * Copyright Google LLC 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 - */ - -import '../zone'; -import './promise'; -import '../common/to-string'; -import './node'; diff --git a/packages/zone.js/lib/node/rollup-test-main.ts b/packages/zone.js/lib/node/rollup-test-main.ts index c27da6c208b7ef..ff2eb1be174428 100644 --- a/packages/zone.js/lib/node/rollup-test-main.ts +++ b/packages/zone.js/lib/node/rollup-test-main.ts @@ -6,6 +6,6 @@ * found in the LICENSE file at https://angular.io/license */ -import './rollup-main-node-bundle'; +import './rollup-main'; // load test related files into bundle import '../testing/zone-testing'; diff --git a/packages/zone.js/test/promise/promise-adapter.mjs b/packages/zone.js/test/promise/promise-adapter.mjs index 426ff2fba25fef..e901283a0261ff 100644 --- a/packages/zone.js/test/promise/promise-adapter.mjs +++ b/packages/zone.js/test/promise/promise-adapter.mjs @@ -18,8 +18,4 @@ const rejected = (reason) => { return Promise.reject(reason); }; -process.on('unhandledRejection', (error) => { - console.log('unhandled', error); -}); - export default {deferred, resolved, rejected}; diff --git a/packages/zone.js/test/promise/promise-test.mjs b/packages/zone.js/test/promise/promise-test.mjs index e86bb25fd5ea5b..88cc2ec241ee35 100644 --- a/packages/zone.js/test/promise/promise-test.mjs +++ b/packages/zone.js/test/promise/promise-test.mjs @@ -1,7 +1,7 @@ import promisesAplusTests from 'promises-aplus-tests'; import adapter from './promise-adapter.mjs'; -promisesAplusTests(adapter, {reporter: 'dot', timeout: 500}, function (err) { +promisesAplusTests(adapter, {reporter: 'dot'}, function (err) { if (err) { console.error(err); process.exit(1);