From 8d72217f11b5170e7bfd8979525b593b991ee92a Mon Sep 17 00:00:00 2001 From: Victor Berchet Date: Wed, 12 Oct 2016 10:05:32 -0700 Subject: [PATCH] refactor(core): delete unused reflector code --- .../src/static_reflection_capabilities.ts | 9 +- .../compiler-cli/src/static_reflector.ts | 9 +- .../compiler/src/lifecycle_reflector.ts | 27 +--- .../platform_reflection_capabilities.ts | 3 +- .../core/src/reflection/reflection.ts | 3 +- .../src/reflection/reflection_capabilities.ts | 7 +- .../@angular/core/src/reflection/reflector.ts | 117 +----------------- .../core/test/reflection/reflector_spec.ts | 69 +---------- modules/@angular/facade/src/collection.ts | 55 +------- 9 files changed, 22 insertions(+), 277 deletions(-) diff --git a/modules/@angular/compiler-cli/src/static_reflection_capabilities.ts b/modules/@angular/compiler-cli/src/static_reflection_capabilities.ts index 52853c48fd548..35e6c81e889a5 100644 --- a/modules/@angular/compiler-cli/src/static_reflection_capabilities.ts +++ b/modules/@angular/compiler-cli/src/static_reflection_capabilities.ts @@ -20,11 +20,10 @@ export class StaticAndDynamicReflectionCapabilities { isReflectionEnabled(): boolean { return true; } factory(type: any): Function { return this.dynamicDelegate.factory(type); } - interfaces(type: any): any[] { return this.dynamicDelegate.interfaces(type); } - hasLifecycleHook(type: any, lcInterface: /*Type*/ any, lcProperty: string): boolean { - return isStaticType(type) ? - this.staticDelegate.hasLifecycleHook(type, lcInterface, lcProperty) : - this.dynamicDelegate.hasLifecycleHook(type, lcInterface, lcProperty); + + hasLifecycleHook(type: any, lcProperty: string): boolean { + return isStaticType(type) ? this.staticDelegate.hasLifecycleHook(type, lcProperty) : + this.dynamicDelegate.hasLifecycleHook(type, lcProperty); } parameters(type: any): any[][] { return isStaticType(type) ? this.staticDelegate.parameters(type) : diff --git a/modules/@angular/compiler-cli/src/static_reflector.ts b/modules/@angular/compiler-cli/src/static_reflector.ts index 5e92309ab91a0..6af5314b981f5 100644 --- a/modules/@angular/compiler-cli/src/static_reflector.ts +++ b/modules/@angular/compiler-cli/src/static_reflector.ts @@ -159,14 +159,15 @@ export class StaticReflector implements ReflectorReader { } } - hasLifecycleHook(type: any, lcInterface: /*Type*/ any, lcProperty: string): boolean { + hasLifecycleHook(type: any, lcProperty: string): boolean { if (!(type instanceof StaticSymbol)) { throw new Error( `hasLifecycleHook received ${JSON.stringify(type)} which is not a StaticSymbol`); } - let classMetadata = this.getTypeMetadata(type); - let members = classMetadata ? classMetadata['members'] : null; - let member: any[] = members ? members[lcProperty] : null; + const classMetadata = this.getTypeMetadata(type); + const members = classMetadata ? classMetadata['members'] : null; + const member: any[] = + members && members.hasOwnProperty(lcProperty) ? members[lcProperty] : null; return member ? member.some(a => a['__symbolic'] == 'method') : false; } diff --git a/modules/@angular/compiler/src/lifecycle_reflector.ts b/modules/@angular/compiler/src/lifecycle_reflector.ts index a4849dc755554..3cecb5e3acb28 100644 --- a/modules/@angular/compiler/src/lifecycle_reflector.ts +++ b/modules/@angular/compiler/src/lifecycle_reflector.ts @@ -6,13 +6,11 @@ * found in the LICENSE file at https://angular.io/license */ -import {AfterContentChecked, AfterContentInit, AfterViewChecked, AfterViewInit, DoCheck, OnChanges, OnDestroy, OnInit, Type} from '@angular/core'; - import {LifecycleHooks, reflector} from './private_import_core'; export function hasLifecycleHook(hook: LifecycleHooks, token: any): boolean { - return reflector.hasLifecycleHook(token, getInterface(hook), getHookName(hook)); + return reflector.hasLifecycleHook(token, getHookName(hook)); } function getHookName(hook: LifecycleHooks): string { @@ -34,25 +32,4 @@ function getHookName(hook: LifecycleHooks): string { case LifecycleHooks.AfterViewChecked: return 'ngAfterViewChecked'; } -} - -function getInterface(hook: LifecycleHooks): any { - switch (hook) { - case LifecycleHooks.OnInit: - return OnInit; - case LifecycleHooks.OnDestroy: - return OnDestroy; - case LifecycleHooks.DoCheck: - return DoCheck; - case LifecycleHooks.OnChanges: - return OnChanges; - case LifecycleHooks.AfterContentInit: - return AfterContentInit; - case LifecycleHooks.AfterContentChecked: - return AfterContentChecked; - case LifecycleHooks.AfterViewInit: - return AfterViewInit; - case LifecycleHooks.AfterViewChecked: - return AfterViewChecked; - } -} +} \ No newline at end of file diff --git a/modules/@angular/core/src/reflection/platform_reflection_capabilities.ts b/modules/@angular/core/src/reflection/platform_reflection_capabilities.ts index e819b22a23419..741f790166bd1 100644 --- a/modules/@angular/core/src/reflection/platform_reflection_capabilities.ts +++ b/modules/@angular/core/src/reflection/platform_reflection_capabilities.ts @@ -12,8 +12,7 @@ import {GetterFn, MethodFn, SetterFn} from './types'; export interface PlatformReflectionCapabilities { isReflectionEnabled(): boolean; factory(type: Type): Function; - interfaces(type: Type): any[]; - hasLifecycleHook(type: any, lcInterface: Type, lcProperty: string): boolean; + hasLifecycleHook(type: any, lcProperty: string): boolean; parameters(type: Type): any[][]; annotations(type: Type): any[]; propMetadata(typeOrFunc: Type): {[key: string]: any[]}; diff --git a/modules/@angular/core/src/reflection/reflection.ts b/modules/@angular/core/src/reflection/reflection.ts index 6653c83a1c164..abbe8727b265e 100644 --- a/modules/@angular/core/src/reflection/reflection.ts +++ b/modules/@angular/core/src/reflection/reflection.ts @@ -9,8 +9,7 @@ import {ReflectionCapabilities} from './reflection_capabilities'; import {Reflector} from './reflector'; -export {ReflectionInfo, Reflector} from './reflector'; - +export {Reflector} from './reflector'; /** * The {@link Reflector} used internally in Angular to access metadata diff --git a/modules/@angular/core/src/reflection/reflection_capabilities.ts b/modules/@angular/core/src/reflection/reflection_capabilities.ts index c2369344347ae..4366c90159917 100644 --- a/modules/@angular/core/src/reflection/reflection_capabilities.ts +++ b/modules/@angular/core/src/reflection/reflection_capabilities.ts @@ -128,12 +128,7 @@ export class ReflectionCapabilities implements PlatformReflectionCapabilities { return {}; } - // Note: JavaScript does not support to query for interfaces during runtime. - // However, we can't throw here as the reflector will always call this method - // when asked for a lifecycle interface as this is what we check in Dart. - interfaces(type: Type): any[] { return []; } - - hasLifecycleHook(type: any, lcInterface: Type, lcProperty: string): boolean { + hasLifecycleHook(type: any, lcProperty: string): boolean { return type instanceof Type && lcProperty in type.prototype; } diff --git a/modules/@angular/core/src/reflection/reflector.ts b/modules/@angular/core/src/reflection/reflector.ts index 49ad411f70420..10bd0c8df158c 100644 --- a/modules/@angular/core/src/reflection/reflector.ts +++ b/modules/@angular/core/src/reflection/reflector.ts @@ -6,7 +6,6 @@ * found in the LICENSE file at https://angular.io/license */ -import {MapWrapper} from '../facade/collection'; import {Type} from '../type'; import {PlatformReflectionCapabilities} from './platform_reflection_capabilities'; import {ReflectorReader} from './reflector_reader'; @@ -15,138 +14,38 @@ import {GetterFn, MethodFn, SetterFn} from './types'; export {PlatformReflectionCapabilities} from './platform_reflection_capabilities'; export {GetterFn, MethodFn, SetterFn} from './types'; -/** - * Reflective information about a symbol, including annotations, interfaces, and other metadata. - */ -export class ReflectionInfo { - constructor( - public annotations?: any[], public parameters?: any[][], public factory?: Function, - public interfaces?: any[], public propMetadata?: {[name: string]: any[]}) {} -} - /** * Provides access to reflection data about symbols. Used internally by Angular * to power dependency injection and compilation. */ export class Reflector extends ReflectorReader { - /** @internal */ - _injectableInfo = new Map(); - /** @internal */ - _getters = new Map(); - /** @internal */ - _setters = new Map(); - /** @internal */ - _methods = new Map(); - /** @internal */ - _usedKeys: Set = null; - constructor(public reflectionCapabilities: PlatformReflectionCapabilities) { super(); } updateCapabilities(caps: PlatformReflectionCapabilities) { this.reflectionCapabilities = caps; } - /** - * Causes `this` reflector to track keys used to access - * {@link ReflectionInfo} objects. - */ - trackUsage(): void { this._usedKeys = new Set(); } - - /** - * Lists types for which reflection information was not requested since - * {@link #trackUsage} was called. This list could later be audited as - * potential dead code. - */ - listUnusedKeys(): any[] { - if (!this._usedKeys) { - throw new Error('Usage tracking is disabled'); - } - const allTypes = MapWrapper.keys(this._injectableInfo); - return allTypes.filter(key => !this._usedKeys.has(key)); - } - - registerType(type: Type, typeInfo: ReflectionInfo): void { - this._injectableInfo.set(type, typeInfo); - } - - registerGetters(getters: {[key: string]: GetterFn}): void { _mergeMaps(this._getters, getters); } - - registerSetters(setters: {[key: string]: SetterFn}): void { _mergeMaps(this._setters, setters); } - - registerMethods(methods: {[key: string]: MethodFn}): void { _mergeMaps(this._methods, methods); } - - factory(type: Type): Function { - if (this._containsReflectionInfo(type)) { - return this._getReflectionInfo(type).factory || null; - } - - return this.reflectionCapabilities.factory(type); - } + factory(type: Type): Function { return this.reflectionCapabilities.factory(type); } parameters(typeOrFunc: Type): any[][] { - if (this._injectableInfo.has(typeOrFunc)) { - return this._getReflectionInfo(typeOrFunc).parameters || []; - } - return this.reflectionCapabilities.parameters(typeOrFunc); } annotations(typeOrFunc: Type): any[] { - if (this._injectableInfo.has(typeOrFunc)) { - return this._getReflectionInfo(typeOrFunc).annotations || []; - } - return this.reflectionCapabilities.annotations(typeOrFunc); } propMetadata(typeOrFunc: Type): {[key: string]: any[]} { - if (this._injectableInfo.has(typeOrFunc)) { - return this._getReflectionInfo(typeOrFunc).propMetadata || {}; - } - return this.reflectionCapabilities.propMetadata(typeOrFunc); } - interfaces(type: Type): any[] { - if (this._injectableInfo.has(type)) { - return this._getReflectionInfo(type).interfaces || []; - } - - return this.reflectionCapabilities.interfaces(type); + hasLifecycleHook(type: any, lcProperty: string): boolean { + return this.reflectionCapabilities.hasLifecycleHook(type, lcProperty); } - hasLifecycleHook(type: any, lcInterface: Type, lcProperty: string): boolean { - if (this.interfaces(type).indexOf(lcInterface) !== -1) { - return true; - } - - return this.reflectionCapabilities.hasLifecycleHook(type, lcInterface, lcProperty); - } + getter(name: string): GetterFn { return this.reflectionCapabilities.getter(name); } - getter(name: string): GetterFn { - return this._getters.has(name) ? this._getters.get(name) : - this.reflectionCapabilities.getter(name); - } + setter(name: string): SetterFn { return this.reflectionCapabilities.setter(name); } - setter(name: string): SetterFn { - return this._setters.has(name) ? this._setters.get(name) : - this.reflectionCapabilities.setter(name); - } - - method(name: string): MethodFn { - return this._methods.has(name) ? this._methods.get(name) : - this.reflectionCapabilities.method(name); - } - - /** @internal */ - _getReflectionInfo(typeOrFunc: any): ReflectionInfo { - if (this._usedKeys) { - this._usedKeys.add(typeOrFunc); - } - - return this._injectableInfo.get(typeOrFunc); - } - - /** @internal */ - _containsReflectionInfo(typeOrFunc: any) { return this._injectableInfo.has(typeOrFunc); } + method(name: string): MethodFn { return this.reflectionCapabilities.method(name); } importUri(type: any): string { return this.reflectionCapabilities.importUri(type); } @@ -158,7 +57,3 @@ export class Reflector extends ReflectorReader { return this.reflectionCapabilities.resolveEnum(identifier, name); } } - -function _mergeMaps(target: Map, config: {[key: string]: Function}): void { - Object.keys(config).forEach(k => { target.set(k, config[k]); }); -} diff --git a/modules/@angular/core/test/reflection/reflector_spec.ts b/modules/@angular/core/test/reflection/reflector_spec.ts index 0b46e6b03139c..b186c1083d2ac 100644 --- a/modules/@angular/core/test/reflection/reflector_spec.ts +++ b/modules/@angular/core/test/reflection/reflector_spec.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {ReflectionInfo, Reflector} from '@angular/core/src/reflection/reflection'; +import {Reflector} from '@angular/core/src/reflection/reflection'; import {ReflectionCapabilities} from '@angular/core/src/reflection/reflection_capabilities'; import {makeDecorator, makeParamDecorator, makePropDecorator} from '@angular/core/src/util/decorators'; @@ -71,40 +71,12 @@ export function main() { beforeEach(() => { reflector = new Reflector(new ReflectionCapabilities()); }); - describe('usage tracking', () => { - beforeEach(() => { reflector = new Reflector(null); }); - - it('should be disabled by default', () => { - expect(() => reflector.listUnusedKeys()).toThrowError('Usage tracking is disabled'); - }); - - it('should report unused keys', () => { - reflector.trackUsage(); - expect(reflector.listUnusedKeys()).toEqual([]); - - reflector.registerType(AType, new ReflectionInfo(null, null, () => 'AType')); - reflector.registerType(TestObj, new ReflectionInfo(null, null, () => 'TestObj')); - expect(reflector.listUnusedKeys()).toEqual([AType, TestObj]); - - reflector.factory(AType); - expect(reflector.listUnusedKeys()).toEqual([TestObj]); - - reflector.factory(TestObj); - expect(reflector.listUnusedKeys()).toEqual([]); - }); - }); - describe('factory', () => { it('should create a factory for the given type', () => { const obj = reflector.factory(TestObj)(1, 2); expect(obj.a).toEqual(1); expect(obj.b).toEqual(2); }); - - it('should return a registered factory if available', () => { - reflector.registerType(TestObj, new ReflectionInfo(null, null, () => 'fake')); - expect(reflector.factory(TestObj)()).toEqual('fake'); - }); }); describe('parameters', () => { @@ -117,16 +89,6 @@ export function main() { const p = reflector.parameters(ClassWithoutDecorators); expect(p.length).toEqual(2); }); - - it('should return registered parameters if available', () => { - reflector.registerType(TestObj, new ReflectionInfo(null, [[1], [2]])); - expect(reflector.parameters(TestObj)).toEqual([[1], [2]]); - }); - - it('should return an empty list when no parameters field in the stored type info', () => { - reflector.registerType(TestObj, new ReflectionInfo()); - expect(reflector.parameters(TestObj)).toEqual([]); - }); }); describe('propMetadata', () => { @@ -136,11 +98,6 @@ export function main() { expect(p['c']).toEqual([new PropDecorator('p3')]); expect(p['someMethod']).toEqual([new PropDecorator('p4')]); }); - - it('should return registered meta if available', () => { - reflector.registerType(TestObj, new ReflectionInfo(null, null, null, null, {'a': [1, 2]})); - expect(reflector.propMetadata(TestObj)).toEqual({'a': [1, 2]}); - }); }); describe('annotations', () => { @@ -149,11 +106,6 @@ export function main() { expect(p).toEqual([new ClassDecorator({value: 'class'})]); }); - it('should return registered annotations if available', () => { - reflector.registerType(TestObj, new ReflectionInfo([1, 2])); - expect(reflector.annotations(TestObj)).toEqual([1, 2]); - }); - it('should work for a class without annotations', () => { const p = reflector.annotations(ClassWithoutDecorators); expect(p).toEqual([]); @@ -165,11 +117,6 @@ export function main() { const getA = reflector.getter('a'); expect(getA(new TestObj(1, 2))).toEqual(1); }); - - it('should return a registered getter if available', () => { - reflector.registerGetters({'abc': (obj: any) => 'fake'}); - expect(reflector.getter('abc')('anything')).toEqual('fake'); - }); }); describe('setter', () => { @@ -179,15 +126,6 @@ export function main() { setA(obj, 100); expect(obj.a).toEqual(100); }); - - it('should return a registered setter if available', () => { - let updateMe: any; - - reflector.registerSetters({'abc': (obj: any, value: any) => { updateMe = value; }}); - reflector.setter('abc')('anything', 'fake'); - - expect(updateMe).toEqual('fake'); - }); }); describe('method', () => { @@ -196,11 +134,6 @@ export function main() { const obj = new TestObj(1, 2); expect(func(obj, ['value'])).toEqual('value'); }); - - it('should return a registered method if available', () => { - reflector.registerMethods({'abc': (obj: any, args: any) => args}); - expect(reflector.method('abc')('anything', ['fake'])).toEqual(['fake']); - }); }); }); } diff --git a/modules/@angular/facade/src/collection.ts b/modules/@angular/facade/src/collection.ts index c8838decbbd72..4f7906edceb22 100644 --- a/modules/@angular/facade/src/collection.ts +++ b/modules/@angular/facade/src/collection.ts @@ -8,55 +8,9 @@ import {getSymbolIterator, isArray, isBlank, isJsObject, isPresent} from './lang'; -// Safari and Internet Explorer do not support the iterable parameter to the -// Map constructor. We work around that by manually adding the items. -const createMapFromPairs: {(pairs: any[]): Map} = (function() { - try { - if (new Map([[1, 2]]).size === 1) { - return function createMapFromPairs(pairs: any[]): Map { return new Map(pairs); }; - } - } catch (e) { - } - return function createMapAndPopulateFromPairs(pairs: any[]): Map { - var map = new Map(); - for (var i = 0; i < pairs.length; i++) { - var pair = pairs[i]; - map.set(pair[0], pair[1]); - } - return map; - }; -})(); -const createMapFromMap: {(m: Map): Map} = (function() { - try { - if (new Map(new Map())) { - return function createMapFromMap(m: Map): Map { return new Map(m); }; - } - } catch (e) { - } - return function createMapAndPopulateFromMap(m: Map): Map { - var map = new Map(); - m.forEach((v, k) => { map.set(k, v); }); - return map; - }; -})(); -const _clearValues: {(m: Map): void} = (function() { - if (((new Map()).keys()).next) { - return function _clearValues(m: Map) { - var keyIterator = m.keys(); - var k: any /** TODO #???? */; - while (!((k = (keyIterator).next()).done)) { - m.set(k.value, null); - } - }; - } else { - return function _clearValuesWithForeEach(m: Map) { - m.forEach((v, k) => { m.set(k, null); }); - }; - } -})(); // Safari doesn't implement MapIterator.next(), which is used is Traceur's polyfill of Array.from // TODO(mlaval): remove the work around once we have a working polyfill of Array.from -var _arrayFromMap: {(m: Map, getValues: boolean): any[]} = (function() { +const _arrayFromMap: {(m: Map, getValues: boolean): any[]} = (function() { try { if (((new Map()).values()).next) { return function createArrayFromMap(m: Map, getValues: boolean): any[] { @@ -83,13 +37,6 @@ export class MapWrapper { } return result; } - static toStringMap(m: Map): {[key: string]: T} { - var r: {[key: string]: T} = {}; - m.forEach((v, k) => r[k] = v); - return r; - } - static createFromPairs(pairs: any[]): Map { return createMapFromPairs(pairs); } - static iterable(m: T): T { return m; } static keys(m: Map): K[] { return _arrayFromMap(m, false); } static values(m: Map): V[] { return _arrayFromMap(m, true); } }