From c2a904da09165609cf408ceee26611f3a8df5326 Mon Sep 17 00:00:00 2001 From: crisbeto Date: Wed, 11 Dec 2019 22:18:38 +0100 Subject: [PATCH] fix(ivy): inheriting injectable definition from undecorated class not working on IE10 in JIT mode (#34305) The way definitions are added in JIT mode is through `Object.defineProperty`, but the problem is that in IE10 properties defined through `defineProperty` won't be inherited which means that inheriting injectable definitions no longer works. These changes add a workaround only for JIT mode where we define a fallback method for retrieving the definition. This isn't ideal, but it should only be required until v10 where we'll no longer support inheriting injectable definitions from undecorated classes. PR Close #34305 --- packages/core/src/di/interface/defs.ts | 13 ++++++++++++- packages/core/src/di/jit/injectable.ts | 12 +++++++++++- .../bundling/injection/bundle.golden_symbols.json | 3 +++ 3 files changed, 26 insertions(+), 2 deletions(-) diff --git a/packages/core/src/di/interface/defs.ts b/packages/core/src/di/interface/defs.ts index 7a6662371b..01e5261da4 100644 --- a/packages/core/src/di/interface/defs.ts +++ b/packages/core/src/di/interface/defs.ts @@ -218,7 +218,10 @@ function getOwnDefinition(type: any, def: ɵɵInjectableDef): ɵɵInjectab * `ɵprov` on an ancestor only. */ export function getInheritedInjectableDef(type: any): ɵɵInjectableDef|null { - const def = type && (type[NG_PROV_DEF] || type[NG_INJECTABLE_DEF]); + // See `jit/injectable.ts#compileInjectable` for context on NG_PROV_DEF_FALLBACK. + const def = type && (type[NG_PROV_DEF] || type[NG_INJECTABLE_DEF] || + (type[NG_PROV_DEF_FALLBACK] && type[NG_PROV_DEF_FALLBACK]())); + if (def) { // TODO(FW-1307): Re-add ngDevMode when closure can handle it // ngDevMode && @@ -245,6 +248,14 @@ export function getInjectorDef(type: any): ɵɵInjectorDef|null { export const NG_PROV_DEF = getClosureSafeProperty({ɵprov: getClosureSafeProperty}); export const NG_INJ_DEF = getClosureSafeProperty({ɵinj: getClosureSafeProperty}); +// On IE10 properties defined via `defineProperty` won't be inherited by child classes, +// which will break inheriting the injectable definition from a grandparent through an +// undecorated parent class. We work around it by defining a fallback method which will be +// used to retrieve the definition. This should only be a problem in JIT mode, because in +// AOT TypeScript seems to have a workaround for static properties. When inheriting from an +// undecorated parent is no longer supported in v10, this can safely be removed. +export const NG_PROV_DEF_FALLBACK = getClosureSafeProperty({ɵprovFallback: getClosureSafeProperty}); + // We need to keep these around so we can read off old defs if new defs are unavailable export const NG_INJECTABLE_DEF = getClosureSafeProperty({ngInjectableDef: getClosureSafeProperty}); export const NG_INJECTOR_DEF = getClosureSafeProperty({ngInjectorDef: getClosureSafeProperty}); diff --git a/packages/core/src/di/jit/injectable.ts b/packages/core/src/di/jit/injectable.ts index c01ad1b121..ee18d1a577 100644 --- a/packages/core/src/di/jit/injectable.ts +++ b/packages/core/src/di/jit/injectable.ts @@ -12,7 +12,7 @@ import {NG_FACTORY_DEF} from '../../render3/fields'; import {getClosureSafeProperty} from '../../util/property'; import {resolveForwardRef} from '../forward_ref'; import {Injectable} from '../injectable'; -import {NG_PROV_DEF} from '../interface/defs'; +import {NG_PROV_DEF, NG_PROV_DEF_FALLBACK} from '../interface/defs'; import {ClassSansProvider, ExistingSansProvider, FactorySansProvider, ValueProvider, ValueSansProvider} from '../interface/provider'; import {angularCoreDiEnv} from './environment'; @@ -40,6 +40,16 @@ export function compileInjectable(type: Type, srcMeta?: Injectable): void { return ngInjectableDef; }, }); + + // On IE10 properties defined via `defineProperty` won't be inherited by child classes, + // which will break inheriting the injectable definition from a grandparent through an + // undecorated parent class. We work around it by defining a method which should be used + // as a fallback. This should only be a problem in JIT mode, because in AOT TypeScript + // seems to have a workaround for static properties. When inheriting from an undecorated + // parent is no longer supported in v10, this can safely be removed. + if (!type.hasOwnProperty(NG_PROV_DEF_FALLBACK)) { + (type as any)[NG_PROV_DEF_FALLBACK] = () => (type as any)[NG_PROV_DEF]; + } } // if NG_FACTORY_DEF is already defined on this class then don't overwrite it diff --git a/packages/core/test/bundling/injection/bundle.golden_symbols.json b/packages/core/test/bundling/injection/bundle.golden_symbols.json index 76e6fcdb57..b8cef691a6 100644 --- a/packages/core/test/bundling/injection/bundle.golden_symbols.json +++ b/packages/core/test/bundling/injection/bundle.golden_symbols.json @@ -38,6 +38,9 @@ { "name": "NG_PROV_DEF" }, + { + "name": "NG_PROV_DEF_FALLBACK" + }, { "name": "NG_TEMP_TOKEN_PATH" },