From 31796e8e2fa78fb55ae0c8bf461c4ad742898363 Mon Sep 17 00:00:00 2001 From: JiaLiPassion Date: Wed, 29 Apr 2020 17:08:28 +0900 Subject: [PATCH] fix(zone.js): remove unused Promise overwritten setter logic (#36851) In the early Zone.js versions (< 0.10.3), `ZoneAwarePromise` did not support `Symbol.species`, so when user used a 3rd party `Promise` such as `es6-promise`, and try to load the promise library after import of `zone.js`, the loading promise library will overwrite the patched `Promise` from `zone.js` and will break `Promise` semantics with respect to `zone.js`. Starting with `zone.js` 0.10.3, `Symbol.species` is supported therefore this will not longer be an issue. (https://github.com//pull/34533) Before 0.10.3, the logic in zone.js tried to handle the case in the wrong way. It did so by overriding the descriptor of `global.Promise`, to allow the 3rd party libraries to override native `Promise` instead of `ZoneAwarePromise`. This is not the correct solution, and since the `Promise.species` is now supported, the 3rd party solution of overriding `global.Promise` is no longer needed. PR removes the wrong work around logic. (This will improve the bundle size.) PR Close #36851 --- .../size-tracking/integration-payloads.json | 6 +-- packages/zone.js/lib/common/promise.ts | 38 ------------------- packages/zone.js/lib/extra/bluebird.ts | 2 +- packages/zone.js/lib/zone.ts | 9 ----- packages/zone.js/test/common/zone.spec.ts | 11 ++---- 5 files changed, 8 insertions(+), 58 deletions(-) diff --git a/goldens/size-tracking/integration-payloads.json b/goldens/size-tracking/integration-payloads.json index 4d088f4c80..fce7abd8f5 100644 --- a/goldens/size-tracking/integration-payloads.json +++ b/goldens/size-tracking/integration-payloads.json @@ -3,8 +3,8 @@ "master": { "uncompressed": { "runtime-es2015": 1485, - "main-es2015": 141394, - "polyfills-es2015": 36963 + "main-es2015": 141151, + "polyfills-es2015": 36571 } } }, @@ -22,7 +22,7 @@ "uncompressed": { "runtime-es2015": 1485, "main-es2015": 147314, - "polyfills-es2015": 36963 + "polyfills-es2015": 36571 } } }, diff --git a/packages/zone.js/lib/common/promise.ts b/packages/zone.js/lib/common/promise.ts index e14a4c2ad9..6f4b59ff80 100644 --- a/packages/zone.js/lib/common/promise.ts +++ b/packages/zone.js/lib/common/promise.ts @@ -459,44 +459,6 @@ Zone.__load_patch('ZoneAwarePromise', (global: any, Zone: ZoneType, api: _ZonePr ZoneAwarePromise['all'] = ZoneAwarePromise.all; const NativePromise = global[symbolPromise] = global['Promise']; - const ZONE_AWARE_PROMISE = Zone.__symbol__('ZoneAwarePromise'); - - let desc = ObjectGetOwnPropertyDescriptor(global, 'Promise'); - if (!desc || desc.configurable) { - desc && delete desc.writable; - desc && delete desc.value; - if (!desc) { - desc = {configurable: true, enumerable: true}; - } - desc.get = function() { - // if we already set ZoneAwarePromise, use patched one - // otherwise return native one. - return global[ZONE_AWARE_PROMISE] ? global[ZONE_AWARE_PROMISE] : global[symbolPromise]; - }; - desc.set = function(NewNativePromise) { - if (NewNativePromise === ZoneAwarePromise) { - // if the NewNativePromise is ZoneAwarePromise - // save to global - global[ZONE_AWARE_PROMISE] = NewNativePromise; - } else { - // if the NewNativePromise is not ZoneAwarePromise - // for example: after load zone.js, some library just - // set es6-promise to global, if we set it to global - // directly, assertZonePatched will fail and angular - // will not loaded, so we just set the NewNativePromise - // to global[symbolPromise], so the result is just like - // we load ES6 Promise before zone.js - global[symbolPromise] = NewNativePromise; - if (!NewNativePromise.prototype[symbolThen]) { - patchThen(NewNativePromise); - } - api.setNativePromise(NewNativePromise); - } - }; - - ObjectDefineProperty(global, 'Promise', desc); - } - global['Promise'] = ZoneAwarePromise; const symbolThenPatched = __symbol__('thenPatched'); diff --git a/packages/zone.js/lib/extra/bluebird.ts b/packages/zone.js/lib/extra/bluebird.ts index 9405db6851..e06b2ce0c6 100644 --- a/packages/zone.js/lib/extra/bluebird.ts +++ b/packages/zone.js/lib/extra/bluebird.ts @@ -80,6 +80,6 @@ Zone.__load_patch('bluebird', (global: any, Zone: ZoneType, api: _ZonePrivate) = }); // override global promise - global[api.symbol('ZoneAwarePromise')] = Bluebird; + global.Promise = Bluebird; }; }); diff --git a/packages/zone.js/lib/zone.ts b/packages/zone.js/lib/zone.ts index 201590dbcc..7d6b3ebaff 100644 --- a/packages/zone.js/lib/zone.ts +++ b/packages/zone.js/lib/zone.ts @@ -340,7 +340,6 @@ interface _ZonePrivate { patchEventTarget: (global: any, apis: any[], options?: any) => boolean[]; patchOnProperties: (obj: any, properties: string[]|null, prototype?: any) => void; patchThen: (ctro: Function) => void; - setNativePromise: (nativePromise: any) => void; patchMethod: (target: any, name: string, patchFn: (delegate: Function, delegateName: string, name: string) => @@ -1419,14 +1418,6 @@ const Zone: ZoneType = (function(global: any) { bindArguments: () => [], patchThen: () => noop, patchMacroTask: () => noop, - setNativePromise: (NativePromise: any) => { - // sometimes NativePromise.resolve static function - // is not ready yet, (such as core-js/es6.promise) - // so we need to check here. - if (NativePromise && typeof NativePromise.resolve === 'function') { - nativeMicroTaskQueuePromise = NativePromise.resolve(0); - } - }, patchEventPrototype: () => noop, isIEOrEdge: () => false, getGlobalObjects: () => undefined, diff --git a/packages/zone.js/test/common/zone.spec.ts b/packages/zone.js/test/common/zone.spec.ts index d701069949..a18d2da306 100644 --- a/packages/zone.js/test/common/zone.spec.ts +++ b/packages/zone.js/test/common/zone.spec.ts @@ -334,7 +334,7 @@ describe('Zone', function() { Zone.assertZonePatched(); }); - it('should keep ZoneAwarePromise has been patched', () => { + xit('should throw error if ZoneAwarePromise has been overwritten', () => { class WrongPromise { static resolve(value: any) {} @@ -342,15 +342,12 @@ describe('Zone', function() { } const ZoneAwarePromise = global.Promise; - const NativePromise = (global as any)[zoneSymbol('Promise')]; - global.Promise = WrongPromise; try { - expect(ZoneAwarePromise).toBeTruthy(); - Zone.assertZonePatched(); - expect(global.Promise).toBe(ZoneAwarePromise); + global.Promise = WrongPromise; + expect(Zone.assertZonePatched()).toThrow(); } finally { // restore it. - global.Promise = NativePromise; + global.Promise = ZoneAwarePromise; } Zone.assertZonePatched(); });