From aebf165359ad6de5a8bacd9cb91651fc4175aaad Mon Sep 17 00:00:00 2001 From: JiaLiPassion Date: Mon, 7 Mar 2022 18:24:24 +0900 Subject: [PATCH] fix(zone.js): should ignore multiple resolve call (#45283) Close #44913 The following case is not handled correctly by `zone.js`. ``` const delayedPromise = new Promise((resolve) => { setTimeout(resolve, 1, 'timeout'); }); new Promise((resolve) => { resolve(delayedPromise); resolve('second call'); }).then(console.log); ``` It should output `timeout`, since the promise is resolved by the 1st resolve, the `second call` should be ignored. So this is a bug that the original implementation not ensure the `resolve` is only called once. PR Close #45283 --- packages/zone.js/lib/common/promise.ts | 8 ++- packages/zone.js/test/common/Promise.spec.ts | 68 ++++++++++++++++++++ 2 files changed, 74 insertions(+), 2 deletions(-) diff --git a/packages/zone.js/lib/common/promise.ts b/packages/zone.js/lib/common/promise.ts index edac6ee2c12..215646a1645 100644 --- a/packages/zone.js/lib/common/promise.ts +++ b/packages/zone.js/lib/common/promise.ts @@ -96,7 +96,7 @@ Zone.__load_patch('ZoneAwarePromise', (global: any, Zone: ZoneType, api: _ZonePr const REJECTED_NO_CATCH = 0; function makeResolver(promise: ZoneAwarePromise, state: boolean): (value: any) => void { - return (v) => { + return (v: any) => { try { resolvePromise(promise, state, v); } catch (err) { @@ -445,7 +445,11 @@ Zone.__load_patch('ZoneAwarePromise', (global: any, Zone: ZoneType, api: _ZonePr (promise as any)[symbolState] = UNRESOLVED; (promise as any)[symbolValue] = []; // queue; try { - executor && executor(makeResolver(promise, RESOLVED), makeResolver(promise, REJECTED)); + const onceWrapper = once(); + executor && + executor( + onceWrapper(makeResolver(promise, RESOLVED)), + onceWrapper(makeResolver(promise, REJECTED))); } catch (error) { resolvePromise(promise, false, error); } diff --git a/packages/zone.js/test/common/Promise.spec.ts b/packages/zone.js/test/common/Promise.spec.ts index 86ce614c7ac..7fd34dcff8a 100644 --- a/packages/zone.js/test/common/Promise.spec.ts +++ b/packages/zone.js/test/common/Promise.spec.ts @@ -836,5 +836,73 @@ describe( expect(Subclass.thenArgs.length).toBe(1); }); }); + + describe('resolve/reject multiple times', () => { + it('should ignore second resolve', (done) => { + const nested = new Promise(res => setTimeout(() => res('nested'))); + const p = new Promise(res => { + res(nested); + res(1); + }); + p.then(v => { + expect(v).toBe('nested'); + done(); + }); + }); + it('should ignore second resolve', (done) => { + const nested = new Promise(res => setTimeout(() => res('nested'))); + const p = new Promise(res => { + res(1); + res(nested); + }); + p.then(v => { + expect(v).toBe(1); + done(); + }); + }); + it('should ignore second reject', (done) => { + const p = new Promise((res, rej) => { + rej(1); + rej(2); + }); + p.then( + v => { + fail('should not get here'); + }, + err => { + expect(err).toBe(1); + done(); + }); + }); + it('should ignore resolve after reject', (done) => { + const p = new Promise((res, rej) => { + rej(1); + res(2); + }); + p.then( + v => { + fail('should not get here'); + }, + err => { + expect(err).toBe(1); + done(); + }); + }); + it('should ignore reject after resolve', (done) => { + const nested = new Promise(res => setTimeout(() => res('nested'))); + const p = new Promise((res, rej) => { + res(nested); + rej(1); + }); + p.then( + v => { + expect(v).toBe('nested'); + done(); + }, + err => { + fail('should not be here'); + }); + }); + }); }); }));