From 4a70b669be716417feb2e2feb603d8337070074c Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Tue, 13 Nov 2018 14:40:54 +0000 Subject: [PATCH] feat(ivy): register references from NgModule annotations (#26906) The `NgModuleDecoratorHandler` can now register all the references that it finds in the `NgModule` metadata, such as `declarations`, `imports`, `exports` etc. This information can then be used by ngcc to work out if any of these references are internal only and need to be manually exported from a library's entry-point. PR Close #26906 --- .../ngcc/src/analysis/decoration_analyzer.ts | 9 ++-- .../src/analysis/ngcc_references_registry.ts | 49 +++++++++++++++++ .../src/ngcc/src/packages/entry_point.ts | 7 +-- .../src/ngcc/src/packages/transformer.ts | 5 +- packages/compiler-cli/src/ngcc/src/utils.ts | 10 ++++ .../test/analysis/decoration_analyzer_spec.ts | 20 ++++--- .../test/analysis/references_registry_spec.ts | 52 +++++++++++++++++++ .../src/ngcc/test/rendering/renderer_spec.ts | 5 +- .../src/ngtsc/annotations/index.ts | 1 + .../src/ngtsc/annotations/src/ng_module.ts | 8 ++- .../annotations/src/references_registry.ts | 39 ++++++++++++++ packages/compiler-cli/src/ngtsc/program.ts | 6 ++- 12 files changed, 191 insertions(+), 20 deletions(-) create mode 100644 packages/compiler-cli/src/ngcc/src/analysis/ngcc_references_registry.ts create mode 100644 packages/compiler-cli/src/ngcc/test/analysis/references_registry_spec.ts create mode 100644 packages/compiler-cli/src/ngtsc/annotations/src/references_registry.ts diff --git a/packages/compiler-cli/src/ngcc/src/analysis/decoration_analyzer.ts b/packages/compiler-cli/src/ngcc/src/analysis/decoration_analyzer.ts index 910c2d9e71..4be8a9db6c 100644 --- a/packages/compiler-cli/src/ngcc/src/analysis/decoration_analyzer.ts +++ b/packages/compiler-cli/src/ngcc/src/analysis/decoration_analyzer.ts @@ -9,9 +9,8 @@ import {ConstantPool} from '@angular/compiler'; import * as fs from 'fs'; import * as ts from 'typescript'; -import {BaseDefDecoratorHandler, ComponentDecoratorHandler, DirectiveDecoratorHandler, InjectableDecoratorHandler, NgModuleDecoratorHandler, PipeDecoratorHandler, ResourceLoader, SelectorScopeRegistry} from '../../../ngtsc/annotations'; +import {BaseDefDecoratorHandler, ComponentDecoratorHandler, DirectiveDecoratorHandler, InjectableDecoratorHandler, NgModuleDecoratorHandler, PipeDecoratorHandler, ReferencesRegistry, ResourceLoader, SelectorScopeRegistry} from '../../../ngtsc/annotations'; import {CompileResult, DecoratorHandler} from '../../../ngtsc/transform'; - import {DecoratedClass} from '../host/decorated_class'; import {NgccReflectionHost} from '../host/ngcc_host'; import {isDefined} from '../utils'; @@ -63,13 +62,15 @@ export class DecorationAnalyzer { this.rootDirs, /* defaultPreserveWhitespaces */ false, /* i18nUseExternalIds */ true), new DirectiveDecoratorHandler(this.typeChecker, this.host, this.scopeRegistry, this.isCore), new InjectableDecoratorHandler(this.host, this.isCore), - new NgModuleDecoratorHandler(this.typeChecker, this.host, this.scopeRegistry, this.isCore), + new NgModuleDecoratorHandler( + this.typeChecker, this.host, this.scopeRegistry, this.referencesRegistry, this.isCore), new PipeDecoratorHandler(this.typeChecker, this.host, this.scopeRegistry, this.isCore), ]; constructor( private typeChecker: ts.TypeChecker, private host: NgccReflectionHost, - private rootDirs: string[], private isCore: boolean) {} + private referencesRegistry: ReferencesRegistry, private rootDirs: string[], + private isCore: boolean) {} /** * Analyze a program to find all the decorated files should be transformed. diff --git a/packages/compiler-cli/src/ngcc/src/analysis/ngcc_references_registry.ts b/packages/compiler-cli/src/ngcc/src/analysis/ngcc_references_registry.ts new file mode 100644 index 0000000000..f4779456b6 --- /dev/null +++ b/packages/compiler-cli/src/ngcc/src/analysis/ngcc_references_registry.ts @@ -0,0 +1,49 @@ +/** + * @license + * Copyright Google Inc. All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import * as ts from 'typescript'; +import {ReferencesRegistry} from '../../../ngtsc/annotations'; +import {Declaration, ReflectionHost} from '../../../ngtsc/host'; +import {Reference, ResolvedReference} from '../../../ngtsc/metadata'; +import {hasNameIdentifier} from '../utils'; + +/** + * This is a place for DecoratorHandlers to register references that they + * find in their analysis of the code. + * + * This registry is used to ensure that these references are publicly exported + * from libraries that are compiled by ngcc. + */ +export class NgccReferencesRegistry implements ReferencesRegistry { + private map = new Map(); + + constructor(private host: ReflectionHost) {} + + /** + * Register one or more references in the registry. + * Only `ResolveReference` references are stored. Other types are ignored. + * @param references A collection of references to register. + */ + add(...references: Reference[]): void { + references.forEach(ref => { + // Only store resolved references. We are not interested in literals. + if (ref instanceof ResolvedReference && hasNameIdentifier(ref.node)) { + const declaration = this.host.getDeclarationOfIdentifier(ref.node.name); + if (declaration && hasNameIdentifier(declaration.node)) { + this.map.set(declaration.node.name, declaration); + } + } + }); + } + + /** + * Create and return a mapping for the registered resolved references. + * @returns A map of reference identifiers to reference declarations. + */ + getDeclarationMap(): Map { return this.map; } +} diff --git a/packages/compiler-cli/src/ngcc/src/packages/entry_point.ts b/packages/compiler-cli/src/ngcc/src/packages/entry_point.ts index 74c8091f12..d2fe854c02 100644 --- a/packages/compiler-cli/src/ngcc/src/packages/entry_point.ts +++ b/packages/compiler-cli/src/ngcc/src/packages/entry_point.ts @@ -62,7 +62,7 @@ interface EntryPointPackageJson { * @param packageJsonPath the absolute path to the package.json file. * @returns JSON from the package.json file if it is valid, `null` otherwise. */ -function loadEntryPointPackage(packageJsonPath: string): {[key: string]: any}|null { +function loadEntryPointPackage(packageJsonPath: string): EntryPointPackageJson|null { try { return JSON.parse(fs.readFileSync(packageJsonPath, 'utf8')); } catch (e) { @@ -95,7 +95,7 @@ export function getEntryPointInfo(packagePath: string, entryPointPath: string): name, module: modulePath, types, - typings = types, // synonymous + typings = types, // synonymous es2015, fesm2015 = es2015, // synonymous fesm5 = modulePath, // synonymous @@ -109,7 +109,8 @@ export function getEntryPointInfo(packagePath: string, entryPointPath: string): } // Also there must exist a `metadata.json` file next to the typings entry-point. - const metadataPath = path.resolve(entryPointPath, typings.replace(/\.d\.ts$/, '') + '.metadata.json'); + const metadataPath = + path.resolve(entryPointPath, typings.replace(/\.d\.ts$/, '') + '.metadata.json'); if (!fs.existsSync(metadataPath)) { return null; } diff --git a/packages/compiler-cli/src/ngcc/src/packages/transformer.ts b/packages/compiler-cli/src/ngcc/src/packages/transformer.ts index 0b13e8a832..8bc9092034 100644 --- a/packages/compiler-cli/src/ngcc/src/packages/transformer.ts +++ b/packages/compiler-cli/src/ngcc/src/packages/transformer.ts @@ -11,6 +11,7 @@ import {mkdir, mv} from 'shelljs'; import * as ts from 'typescript'; import {DecorationAnalyzer} from '../analysis/decoration_analyzer'; +import {NgccReferencesRegistry} from '../analysis/ngcc_references_registry'; import {SwitchMarkerAnalyzer} from '../analysis/switch_marker_analyzer'; import {Esm2015ReflectionHost} from '../host/esm2015_host'; import {Esm5ReflectionHost} from '../host/esm5_host'; @@ -156,8 +157,10 @@ export class Transformer { analyzeProgram( program: ts.Program, reflectionHost: NgccReflectionHost, rootDirs: string[], isCore: boolean) { + const typeChecker = bundle.program.getTypeChecker(); + const referencesRegistry = new NgccReferencesRegistry(reflectionHost); const decorationAnalyzer = - new DecorationAnalyzer(program.getTypeChecker(), reflectionHost, rootDirs, isCore); + new DecorationAnalyzer(typeChecker, reflectionHost, referencesRegistry, rootDirs, isCore); const switchMarkerAnalyzer = new SwitchMarkerAnalyzer(reflectionHost); return { decorationAnalyses: decorationAnalyzer.analyzeProgram(program), diff --git a/packages/compiler-cli/src/ngcc/src/utils.ts b/packages/compiler-cli/src/ngcc/src/utils.ts index 94682bd8b6..ceef984d94 100644 --- a/packages/compiler-cli/src/ngcc/src/utils.ts +++ b/packages/compiler-cli/src/ngcc/src/utils.ts @@ -40,3 +40,13 @@ export function findAll(node: ts.Node, test: (node: ts.Node) => node is ts.No } } } + +/** + * Does the given declaration have a name which is an identifier? + * @param declaration The declaration to test. + * @returns true if the declaration has an identifer for a name. + */ +export function hasNameIdentifier(declaration: ts.Declaration): declaration is ts.Declaration& + {name: ts.Identifier} { + return ts.isIdentifier((declaration as any).name); +} diff --git a/packages/compiler-cli/src/ngcc/test/analysis/decoration_analyzer_spec.ts b/packages/compiler-cli/src/ngcc/test/analysis/decoration_analyzer_spec.ts index 2d435ca638..89868d3b62 100644 --- a/packages/compiler-cli/src/ngcc/test/analysis/decoration_analyzer_spec.ts +++ b/packages/compiler-cli/src/ngcc/test/analysis/decoration_analyzer_spec.ts @@ -10,6 +10,7 @@ import * as ts from 'typescript'; import {Decorator} from '../../../ngtsc/host'; import {DecoratorHandler} from '../../../ngtsc/transform'; import {DecorationAnalyses, DecorationAnalyzer} from '../../src/analysis/decoration_analyzer'; +import {NgccReferencesRegistry} from '../../src/analysis/ngcc_references_registry'; import {Esm2015ReflectionHost} from '../../src/host/esm2015_host'; import {makeProgram} from '../helpers/utils'; @@ -84,9 +85,10 @@ describe('DecorationAnalyzer', () => { beforeEach(() => { program = makeProgram(TEST_PROGRAM); - const analyzer = new DecorationAnalyzer( - program.getTypeChecker(), new Esm2015ReflectionHost(false, program.getTypeChecker()), - [''], false); + const host = new Esm2015ReflectionHost(false, program.getTypeChecker()); + const referencesRegistry = new NgccReferencesRegistry(host); + const analyzer = + new DecorationAnalyzer(program.getTypeChecker(), host, referencesRegistry, [''], false); testHandler = createTestHandler(); analyzer.handlers = [testHandler]; result = analyzer.analyzeProgram(program); @@ -126,9 +128,10 @@ describe('DecorationAnalyzer', () => { it('should analyze an internally imported component, which is not publicly exported from the entry-point', () => { const program = makeProgram(...INTERNAL_COMPONENT_PROGRAM); + const host = new Esm2015ReflectionHost(false, program.getTypeChecker()); + const referencesRegistry = new NgccReferencesRegistry(host); const analyzer = new DecorationAnalyzer( - program.getTypeChecker(), new Esm2015ReflectionHost(false, program.getTypeChecker()), - [''], false); + program.getTypeChecker(), host, referencesRegistry, [''], false); const testHandler = createTestHandler(); analyzer.handlers = [testHandler]; const result = analyzer.analyzeProgram(program); @@ -142,9 +145,10 @@ describe('DecorationAnalyzer', () => { it('should analyze an internally defined component, which is not exported at all', () => { const program = makeProgram(...INTERNAL_COMPONENT_PROGRAM); - const analyzer = new DecorationAnalyzer( - program.getTypeChecker(), new Esm2015ReflectionHost(false, program.getTypeChecker()), - [''], false); + const host = new Esm2015ReflectionHost(false, program.getTypeChecker()); + const referencesRegistry = new NgccReferencesRegistry(host); + const analyzer = + new DecorationAnalyzer(program.getTypeChecker(), host, referencesRegistry, [''], false); const testHandler = createTestHandler(); analyzer.handlers = [testHandler]; const result = analyzer.analyzeProgram(program); diff --git a/packages/compiler-cli/src/ngcc/test/analysis/references_registry_spec.ts b/packages/compiler-cli/src/ngcc/test/analysis/references_registry_spec.ts new file mode 100644 index 0000000000..b2f9ce27aa --- /dev/null +++ b/packages/compiler-cli/src/ngcc/test/analysis/references_registry_spec.ts @@ -0,0 +1,52 @@ +/** + * @license + * Copyright Google Inc. All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import * as ts from 'typescript'; + +import {Reference, TypeScriptReflectionHost, staticallyResolve} from '../../../ngtsc/metadata'; +import {getDeclaration, makeProgram} from '../../../ngtsc/testing/in_memory_typescript'; +import {NgccReferencesRegistry} from '../../src/analysis/ngcc_references_registry'; + +describe('NgccReferencesRegistry', () => { + it('should return a mapping from resolved reference identifiers to their declarations', () => { + const {program} = makeProgram([{ + name: 'index.ts', + contents: ` + export class SomeClass {} + export function someFunction() {} + export const someVariable = 42; + + export const testArray = [SomeClass, someFunction, someVariable]; + ` + }]); + + const checker = program.getTypeChecker(); + + const testArrayDeclaration = + getDeclaration(program, 'index.ts', 'testArray', ts.isVariableDeclaration); + const someClassDecl = getDeclaration(program, 'index.ts', 'SomeClass', ts.isClassDeclaration); + const someFunctionDecl = + getDeclaration(program, 'index.ts', 'someFunction', ts.isFunctionDeclaration); + const someVariableDecl = + getDeclaration(program, 'index.ts', 'someVariable', ts.isVariableDeclaration); + const testArrayExpression = testArrayDeclaration.initializer !; + + const host = new TypeScriptReflectionHost(checker); + const registry = new NgccReferencesRegistry(host); + + const references = + staticallyResolve(testArrayExpression, host, checker) as Reference[]; + registry.add(...references); + + const map = registry.getDeclarationMap(); + expect(map.size).toEqual(2); + expect(map.get(someClassDecl.name !) !.node).toBe(someClassDecl); + expect(map.get(someFunctionDecl.name !) !.node).toBe(someFunctionDecl); + expect(map.has(someVariableDecl.name as ts.Identifier)).toBe(false); + }); +}); diff --git a/packages/compiler-cli/src/ngcc/test/rendering/renderer_spec.ts b/packages/compiler-cli/src/ngcc/test/rendering/renderer_spec.ts index e63fa81f89..56806524ae 100644 --- a/packages/compiler-cli/src/ngcc/test/rendering/renderer_spec.ts +++ b/packages/compiler-cli/src/ngcc/test/rendering/renderer_spec.ts @@ -12,6 +12,7 @@ import MagicString from 'magic-string'; import {fromObject, generateMapFileComment} from 'convert-source-map'; import {makeProgram} from '../helpers/utils'; import {CompiledClass, DecorationAnalyzer} from '../../src/analysis/decoration_analyzer'; +import {NgccReferencesRegistry} from '../../src/analysis/ngcc_references_registry'; import {SwitchMarkerAnalyzer} from '../../src/analysis/switch_marker_analyzer'; import {BundleInfo, createBundleInfo} from '../../src/packages/bundle'; import {Esm2015ReflectionHost} from '../../src/host/esm2015_host'; @@ -46,8 +47,10 @@ function createTestRenderer( options.rewriteCoreImportsTo ? program.getSourceFile(options.rewriteCoreImportsTo) ! : null; const bundle = createBundleInfo(options.isCore || false, rewriteCoreImportsTo, null); const host = new Esm2015ReflectionHost(bundle.isCore, program.getTypeChecker()); + const referencesRegistry = new NgccReferencesRegistry(host); const decorationAnalyses = - new DecorationAnalyzer(program.getTypeChecker(), host, [''], bundle.isCore) + new DecorationAnalyzer( + program.getTypeChecker(), host, referencesRegistry, [''], bundle.isCore) .analyzeProgram(program); const switchMarkerAnalyses = new SwitchMarkerAnalyzer(host).analyzeProgram(program); const renderer = new TestRenderer(host, bundle); diff --git a/packages/compiler-cli/src/ngtsc/annotations/index.ts b/packages/compiler-cli/src/ngtsc/annotations/index.ts index 8902f606e8..ac736ce264 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/index.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/index.ts @@ -15,4 +15,5 @@ export {DirectiveDecoratorHandler} from './src/directive'; export {InjectableDecoratorHandler} from './src/injectable'; export {NgModuleDecoratorHandler} from './src/ng_module'; export {PipeDecoratorHandler} from './src/pipe'; +export {NoopReferencesRegistry, ReferencesRegistry} from './src/references_registry'; export {CompilationScope, SelectorScopeRegistry} from './src/selector_scope'; diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts b/packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts index 23bd83bf31..5544ddeca8 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts @@ -15,6 +15,7 @@ import {Reference, ResolvedReference, ResolvedValue, reflectObjectLiteral, stati import {AnalysisOutput, CompileResult, DecoratorHandler} from '../../transform'; import {generateSetClassMetadataCall} from './metadata'; +import {ReferencesRegistry} from './references_registry'; import {SelectorScopeRegistry} from './selector_scope'; import {getConstructorDependencies, isAngularCore, toR3Reference, unwrapExpression} from './util'; @@ -32,7 +33,8 @@ export interface NgModuleAnalysis { export class NgModuleDecoratorHandler implements DecoratorHandler { constructor( private checker: ts.TypeChecker, private reflector: ReflectionHost, - private scopeRegistry: SelectorScopeRegistry, private isCore: boolean) {} + private scopeRegistry: SelectorScopeRegistry, private referencesRegistry: ReferencesRegistry, + private isCore: boolean) {} detect(node: ts.Declaration, decorators: Decorator[]|null): Decorator|undefined { if (!decorators) { @@ -72,6 +74,7 @@ export class NgModuleDecoratorHandler implements DecoratorHandler[] = []; if (ngModule.has('imports')) { @@ -80,6 +83,7 @@ export class NgModuleDecoratorHandler implements DecoratorHandler this._extractModuleFromModuleWithProvidersFn(ref.node)); imports = this.resolveTypeList(expr, importsMeta, 'imports'); + this.referencesRegistry.add(...imports); } let exports: Reference[] = []; if (ngModule.has('exports')) { @@ -88,12 +92,14 @@ export class NgModuleDecoratorHandler implements DecoratorHandler this._extractModuleFromModuleWithProvidersFn(ref.node)); exports = this.resolveTypeList(expr, exportsMeta, 'exports'); + this.referencesRegistry.add(...exports); } let bootstrap: Reference[] = []; if (ngModule.has('bootstrap')) { const expr = ngModule.get('bootstrap') !; const bootstrapMeta = staticallyResolve(expr, this.reflector, this.checker); bootstrap = this.resolveTypeList(expr, bootstrapMeta, 'bootstrap'); + this.referencesRegistry.add(...bootstrap); } // Register this module's information with the SelectorScopeRegistry. This ensures that during diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/references_registry.ts b/packages/compiler-cli/src/ngtsc/annotations/src/references_registry.ts new file mode 100644 index 0000000000..a45ce6ad35 --- /dev/null +++ b/packages/compiler-cli/src/ngtsc/annotations/src/references_registry.ts @@ -0,0 +1,39 @@ +/** + * @license + * Copyright Google Inc. All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import * as ts from 'typescript'; +import {Declaration} from '../../host'; +import {Reference} from '../../metadata'; + +/** + * Implement this interface if you want DecoratorHandlers to register + * references that they find in their analysis of the code. + */ +export interface ReferencesRegistry { + /** + * Register one or more references in the registry. + * Only `ResolveReference` references are stored. Other types are ignored. + * @param references A collection of references to register. + */ + add(...references: Reference[]): void; + /** + * Create and return a mapping for the registered resolved references. + * @returns A map of reference identifiers to reference declarations. + */ + getDeclarationMap(): Map; +} + +/** + * This registry does nothing, since ngtsc does not currently need + * this functionality. + * The ngcc tool implements a working version for its purposes. + */ +export class NoopReferencesRegistry implements ReferencesRegistry { + add(...references: Reference[]): void {} + getDeclarationMap(): Map { return new Map(); } +} \ No newline at end of file diff --git a/packages/compiler-cli/src/ngtsc/program.ts b/packages/compiler-cli/src/ngtsc/program.ts index c9d53e44ac..82f884b009 100644 --- a/packages/compiler-cli/src/ngtsc/program.ts +++ b/packages/compiler-cli/src/ngtsc/program.ts @@ -13,7 +13,7 @@ import * as ts from 'typescript'; import * as api from '../transformers/api'; import {nocollapseHack} from '../transformers/nocollapse_hack'; -import {ComponentDecoratorHandler, DirectiveDecoratorHandler, InjectableDecoratorHandler, NgModuleDecoratorHandler, PipeDecoratorHandler, ResourceLoader, SelectorScopeRegistry} from './annotations'; +import {ComponentDecoratorHandler, DirectiveDecoratorHandler, InjectableDecoratorHandler, NgModuleDecoratorHandler, NoopReferencesRegistry, PipeDecoratorHandler, ResourceLoader, SelectorScopeRegistry} from './annotations'; import {BaseDefDecoratorHandler} from './annotations/src/base_def'; import {TypeScriptReflectionHost} from './metadata'; import {FileResourceLoader, HostResourceLoader} from './resource_loader'; @@ -214,6 +214,7 @@ export class NgtscProgram implements api.Program { private makeCompilation(): IvyCompilation { const checker = this.tsProgram.getTypeChecker(); const scopeRegistry = new SelectorScopeRegistry(checker, this.reflector); + const referencesRegistry = new NoopReferencesRegistry(); // Set up the IvyCompilation, which manages state for the Ivy transformer. const handlers = [ @@ -223,7 +224,8 @@ export class NgtscProgram implements api.Program { this.options.preserveWhitespaces || false, this.options.i18nUseExternalIds !== false), new DirectiveDecoratorHandler(checker, this.reflector, scopeRegistry, this.isCore), new InjectableDecoratorHandler(this.reflector, this.isCore), - new NgModuleDecoratorHandler(checker, this.reflector, scopeRegistry, this.isCore), + new NgModuleDecoratorHandler( + checker, this.reflector, scopeRegistry, referencesRegistry, this.isCore), new PipeDecoratorHandler(checker, this.reflector, scopeRegistry, this.isCore), ];