fix(zone.js): Promise.resolve(subPromise) should return subPromise

In the original `Promise` impelmentation, zone.js follow the spec from https://promisesaplus.com/#point-51. ``` const p1 = Promise.resolve(1); const p2 = Promise.resolve(p1); p1 === p2; // false ``` in this case, `p2` should be the same status with `p1` but they are still different instances. And for some edge case. ``` class MyPromise extends Promise { constructor(sub) { super((res) => res(null)); this.sub = sub; } then(onFufilled, onRejected) { this.sub.then(onFufilled, onRejected); } } const p1 = new Promise(setTimeout(res), 100); const myP = new MyPromise(p1); const r = await myP; r === 1; // false ``` So in the above code, `myP` is not the same instance with `p1`, and since `myP` is resolved in constructor, so `await myP` will just pass without waiting for `p1`. And in the current `tc39` spec here https://tc39.es/ecma262/multipage/control-abstraction-objects.html#sec-promise-resolve `Promise.resolve(subP)` should return `subP`. ``` const p1 = Promise.resolve(1); const p2 = Promise.resolve(p1); p1 === p2; // true ``` So the above `MyPromise` can wait for the `p1` correctly.
JiaLiPassion · Dec 11, 2023 · a2a89a5 · a2a89a5
1 parent 5f73608
commit a2a89a5
Show file tree

Hide file tree

Showing 7 changed files with 69 additions and 349 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -234,7 +234,6 @@ jobs:
         # Install
       - run: yarn --cwd packages/zone.js install --frozen-lockfile --non-interactive
         # Run zone.js tools tests
-      - run: yarn --cwd packages/zone.js promisetest
       - run: yarn --cwd packages/zone.js promisefinallytest
       - run: yarn --cwd packages/zone.js jest:test
       - run: yarn --cwd packages/zone.js jest:nodetest

diff --git a/packages/zone.js/README.md b/packages/zone.js/README.md
@@ -7,9 +7,9 @@ Implements _Zones_ for JavaScript, inspired by [Dart](https://dart.dev/articles/
 > If you're using zone.js via unpkg (i.e. using `https://unpkg.com/zone.js`)
 > and you're using any of the following libraries, make sure you import them first
 
-> * 'newrelic' as it patches global.Promise before zone.js does
-> * 'async-listener' as it patches global.setTimeout, global.setInterval before zone.js does
-> * 'continuation-local-storage' as it uses async-listener
+> - 'newrelic' as it patches global.Promise before zone.js does
+> - 'async-listener' as it patches global.setTimeout, global.setInterval before zone.js does
+> - 'continuation-local-storage' as it uses async-listener
 
 # NEW Zone.js POST-v0.6.0
 
@@ -41,7 +41,7 @@ Starting with Zone.js `v0.11.1+` the import changes to:
 import 'zone.js';
 ```
 
-Prior to `v0.11.1` the import would load the `ES5` bundle in `UMD` format from `dist/zone.js`. 
+Prior to `v0.11.1` the import would load the `ES5` bundle in `UMD` format from `dist/zone.js`.
 Starting with `v0.11.1` the import loads the `ES2015` bundle in `ESM` format instead.
 
 This is a breaking change for legacy browsers such as `IE11`.
@@ -65,10 +65,11 @@ See this video from ng-conf 2014 for a detailed explanation:
 [![screenshot of the zone.js presentation and ng-conf 2014](./presentation.png)](//www.youtube.com/watch?v=3IqtmUscE_U&t=150)
 
 ## See also
-* [async-listener](https://github.com/othiym23/async-listener) - a similar library for node
-* [Async stack traces in Chrome](https://www.html5rocks.com/en/tutorials/developertools/async-call-stack/)
-* [strongloop/zone](https://github.com/strongloop/zone) (Deprecated)
-* [vizone](https://github.com/gilbox/vizone) - control flow visualizer that uses zone.js
+
+- [async-listener](https://github.com/othiym23/async-listener) - a similar library for node
+- [Async stack traces in Chrome](https://www.html5rocks.com/en/tutorials/developertools/async-call-stack/)
+- [strongloop/zone](https://github.com/strongloop/zone) (Deprecated)
+- [vizone](https://github.com/gilbox/vizone) - control flow visualizer that uses zone.js
 
 ## Standard API support
 
@@ -93,19 +94,19 @@ see [MODULE.md](MODULE.md).
 
 ## Bundles
 
-Starting with `v0.11.0`, `zone.js`  uses `Angular Package Format` for bundle distribution.
+Starting with `v0.11.0`, `zone.js` uses `Angular Package Format` for bundle distribution.
 (For backwards compatibility, all bundles can still be accessed from `dist` folder.)
 
-|Bundle|Summary|
-|---|---|
-|`zone.js`| The default bundle. Contains the most used APIs such as `setTimeout/Promise/EventTarget...`, it also supports differential loading by importing  this bundle using `import zone.js`.  In legacy browsers it includes some additional patches such as `registerElement` and `EventTarget` like APIs.|
-|`zone-testing.js`| The bundle for zone testing support of `jasmine` / `mocha` / `jest`. Also includes test utility functions `async` / `fakeAsync` / `sync`.|
-|`zone-node.js`|The NodeJS support bundle.|
-|`zone-mix.js`|A mixed bundle which supports both browser and NodeJS. Useful for mixed environment such as Electron.|
-|`zone-externs.js`|the API definitions for `closure compiler`.|
+| Bundle            | Summary                                                                                                                                                                                                                                                                                           |
+| ----------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
+| `zone.js`         | The default bundle. Contains the most used APIs such as `setTimeout/Promise/EventTarget...`, it also supports differential loading by importing this bundle using `import zone.js`. In legacy browsers it includes some additional patches such as `registerElement` and `EventTarget` like APIs. |
+| `zone-testing.js` | The bundle for zone testing support of `jasmine` / `mocha` / `jest`. Also includes test utility functions `async` / `fakeAsync` / `sync`.                                                                                                                                                         |
+| `zone-node.js`    | The NodeJS support bundle.                                                                                                                                                                                                                                                                        |
+| `zone-mix.js`     | A mixed bundle which supports both browser and NodeJS. Useful for mixed environment such as Electron.                                                                                                                                                                                             |
+| `zone-externs.js` | the API definitions for `closure compiler`.                                                                                                                                                                                                                                                       |
 
 Additional optional patches not included in the `zone.js` bundles which extend functionality.
-The additional bundles can  be found under `zone.js/plugins` folder.
+The additional bundles can be found under `zone.js/plugins` folder.
 To use these bundles, add the following code after importing zone.js bundle.
 
 ```
@@ -114,28 +115,26 @@ import 'zone.js';
 import 'zone.js/plugins/zone-patch-canvas';
 ```
 
-|Patch|Summary|
-|---|---|
-|`webapis-media-query.js`|patch for `MediaQuery APIs`|
-|`webapis-notification.js`|patch for `Notification APIs`|
-|`webapis-rtc-peer-connection.js`|patch for `RTCPeerConnection APIs`|
-|`webapis-shadydom.js`|patch for `Shady DOM APIs`|
-|`zone-bluebird.js`|patch for `Bluebird APIs`|
-|`zone-error.js`|patch for `Error Global Object`, supports adding zone information to stack frame, and also removing unrelated stack frames from `zone.js` internally|
-|`zone-patch-canvas.js`|patch for `Canvas API`|
-|`zone-patch-cordova.js`|patch for `Cordova API`|
-|`zone-patch-electron.js`|patch for `Electron API`|
-|`zone-patch-fetch.js`|patch for `Fetch API`|
-|`zone-patch-jsonp.js`|helper utility for `jsonp API`|
-|`zone-patch-resize-observer.js`|patch for `ResizeObserver API`|
-|`zone-patch-rxjs.js`|patch for `rxjs API`|
-|`zone-patch-rxjs-fake-async.js`|patch for `rxjs fakeasync test`|
-|`zone-patch-socket-io.js`|patch for `socket-io`|
-|`zone-patch-user-media.js`|patch for `UserMedia API`|
-|`zone-patch-message-port.js`|patch for `MessagePort API`|
-
-## Promise A+ test passed
-[![Promises/A+ 1.1 compliant](https://promisesaplus.com/assets/logo-small.png)](https://promisesaplus.com/)
+| Patch                            | Summary                                                                                                                                              |
+| -------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------- |
+| `webapis-media-query.js`         | patch for `MediaQuery APIs`                                                                                                                          |
+| `webapis-notification.js`        | patch for `Notification APIs`                                                                                                                        |
+| `webapis-rtc-peer-connection.js` | patch for `RTCPeerConnection APIs`                                                                                                                   |
+| `webapis-shadydom.js`            | patch for `Shady DOM APIs`                                                                                                                           |
+| `zone-bluebird.js`               | patch for `Bluebird APIs`                                                                                                                            |
+| `zone-error.js`                  | patch for `Error Global Object`, supports adding zone information to stack frame, and also removing unrelated stack frames from `zone.js` internally |
+| `zone-patch-canvas.js`           | patch for `Canvas API`                                                                                                                               |
+| `zone-patch-cordova.js`          | patch for `Cordova API`                                                                                                                              |
+| `zone-patch-electron.js`         | patch for `Electron API`                                                                                                                             |
+| `zone-patch-fetch.js`            | patch for `Fetch API`                                                                                                                                |
+| `zone-patch-jsonp.js`            | helper utility for `jsonp API`                                                                                                                       |
+| `zone-patch-resize-observer.js`  | patch for `ResizeObserver API`                                                                                                                       |
+| `zone-patch-rxjs.js`             | patch for `rxjs API`                                                                                                                                 |
+| `zone-patch-rxjs-fake-async.js`  | patch for `rxjs fakeasync test`                                                                                                                      |
+| `zone-patch-socket-io.js`        | patch for `socket-io`                                                                                                                                |
+| `zone-patch-user-media.js`       | patch for `UserMedia API`                                                                                                                            |
+| `zone-patch-message-port.js`     | patch for `MessagePort API`                                                                                                                          |
 
 ## License
-MIT
+
+MIT
diff --git a/packages/zone.js/lib/common/promise.ts b/packages/zone.js/lib/common/promise.ts
@@ -292,6 +292,9 @@ Zone.__load_patch('ZoneAwarePromise', (global: any, Zone: ZoneType, api: _ZonePr
     }
 
     static resolve<R>(value: R): Promise<R> {
+      if (value instanceof ZoneAwarePromise) {
+        return value;
+      }
       return resolvePromise(<ZoneAwarePromise<R>>new this(null as any), RESOLVED, value);
     }
 

diff --git a/packages/zone.js/package.json b/packages/zone.js/package.json
@@ -19,15 +19,13 @@
     "jest-environment-jsdom": "^29.0.3",
     "jest-environment-node": "^29.0.3",
     "mocha": "^10.2.0",
-    "mock-require": "3.0.3",
-    "promises-aplus-tests": "^2.1.2"
+    "mock-require": "3.0.3"
   },
   "scripts": {
     "closuretest": "./scripts/closure/closure_compiler.sh",
     "electrontest": "cd test/extra && node electron.js",
     "jest:test": "jest --config ./test/jest/jest.config.js ./test/jest/jest.spec.js",
     "jest:nodetest": "jest --config ./test/jest/jest.node.config.js ./test/jest/jest.spec.js",
-    "promisetest": "node ./test/promise/promise-test.mjs",
     "promisefinallytest": "mocha ./test/promise/promise.finally.spec.mjs"
   },
   "repository": {

diff --git a/packages/zone.js/test/common/Promise.spec.ts b/packages/zone.js/test/common/Promise.spec.ts
@@ -6,7 +6,7 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {isNode, zoneSymbol} from '../../lib/common/utils';
+import {isNode} from '../../lib/common/utils';
 import {ifEnvSupports} from '../test-util';
 
 declare const global: any;
@@ -76,6 +76,12 @@ describe(
         expect(new Promise(() => null) instanceof Promise).toBe(true);
       });
 
+      it('Promise.resolve(subPromise) should equal to subPromise', () => {
+        const p1 = Promise.resolve(1);
+        const p2 = Promise.resolve(p1);
+        expect(p1).toBe(p2);
+      });
+
       xit('should ensure that Promise this is instanceof Promise', () => {
         expect(() => {
           Promise.call({} as any, () => null);
@@ -130,6 +136,23 @@ describe(
         expect(new MyPromise(() => {}).then(() => null) instanceof MyPromise).toBe(true);
       });
 
+      it('should allow subclassing with own then', (done: DoneFn) => {
+        class MyPromise extends Promise<any> {
+          constructor(private sub: Promise<any>) {
+            super((resolve) => {resolve(null)});
+          }
+
+          override then(onFulfilled: any, onRejected: any) {
+            return this.sub.then(onFulfilled, onRejected);
+          }
+        }
+        const p = Promise.resolve(1);
+        new MyPromise(p).then((v: any) => {
+          expect(v).toBe(1);
+          done();
+        }, () => done());
+      });
+
       it('Symbol.species should return ZoneAwarePromise', () => {
         const empty = function() {};
         const promise = Promise.resolve(1);

diff --git a/packages/zone.js/test/promise/promise-test.mjs b/packages/zone.js/test/promise/promise-test.mjs