From c1392ce61871b294edddbd6277146c4c81d929c9 Mon Sep 17 00:00:00 2001 From: Alex Rickabaugh Date: Tue, 19 Feb 2019 17:36:26 -0800 Subject: [PATCH] feat(ivy): produce and consume ES2015 re-exports for NgModule re-exports (#28852) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In certain configurations (such as the g3 repository) which have lots of small compilation units as well as strict dependency checking on generated code, ngtsc's default strategy of directly importing directives/pipes into components will not work. To handle these cases, an additional mode is introduced, and is enabled when using the FileToModuleHost provided by such compilation environments. In this mode, when ngtsc encounters an NgModule which re-exports another from a different file, it will re-export all the directives it contains at the ES2015 level. The exports will have a predictable name based on the FileToModuleHost. For example, if the host says that a directive Foo is from the 'root/external/foo' module, ngtsc will add: ``` export {Foo as ɵng$root$external$foo$$Foo} from 'root/external/foo'; ``` Consumers of the re-exported directive will then import it via this path instead of directly from root/external/foo, preserving strict dependency semantics. PR Close #28852 --- .../ngcc/src/analysis/decoration_analyzer.ts | 7 +- .../src/ngtsc/annotations/src/component.ts | 5 +- .../src/ngtsc/annotations/src/ng_module.ts | 13 +- .../ngtsc/annotations/test/component_spec.ts | 5 +- .../ngtsc/annotations/test/directive_spec.ts | 6 +- .../compiler-cli/src/ngtsc/imports/index.ts | 2 + .../src/ngtsc/imports/src/alias.ts | 46 ++++++ .../src/ngtsc/imports/src/reexport.ts | 13 ++ .../src/ngtsc/imports/src/references.ts | 20 +++ packages/compiler-cli/src/ngtsc/program.ts | 28 +++- .../src/ngtsc/scope/src/dependency.ts | 72 ++++++-- .../compiler-cli/src/ngtsc/scope/src/local.ts | 71 ++++++-- .../src/ngtsc/scope/test/BUILD.bazel | 1 + .../src/ngtsc/scope/test/dependency_spec.ts | 155 +++++++++++++++++- .../src/ngtsc/scope/test/local_spec.ts | 7 +- .../src/ngtsc/transform/src/alias.ts | 36 ++++ .../src/ngtsc/transform/src/api.ts | 5 +- .../src/ngtsc/transform/src/compilation.ts | 18 +- .../src/transformers/compiler_host.ts | 11 ++ packages/compiler-cli/test/ngtsc/env.ts | 11 ++ .../compiler-cli/test/ngtsc/ngtsc_spec.ts | 136 ++++++++++++++- 21 files changed, 608 insertions(+), 60 deletions(-) create mode 100644 packages/compiler-cli/src/ngtsc/imports/src/alias.ts create mode 100644 packages/compiler-cli/src/ngtsc/imports/src/reexport.ts create mode 100644 packages/compiler-cli/src/ngtsc/transform/src/alias.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 7b0d5338eb..bbc4cfb50c 100644 --- a/packages/compiler-cli/src/ngcc/src/analysis/decoration_analyzer.ts +++ b/packages/compiler-cli/src/ngcc/src/analysis/decoration_analyzer.ts @@ -72,9 +72,10 @@ export class DecorationAnalyzer { // on whether a bestGuessOwningModule is present in the Reference. new LogicalProjectStrategy(this.typeChecker, new LogicalFileSystem(this.rootDirs)), ]); - dtsModuleScopeResolver = - new MetadataDtsModuleScopeResolver(this.typeChecker, this.reflectionHost); - scopeRegistry = new LocalModuleScopeRegistry(this.dtsModuleScopeResolver); + dtsModuleScopeResolver = new MetadataDtsModuleScopeResolver( + this.typeChecker, this.reflectionHost, /* aliasGenerator */ null); + scopeRegistry = new LocalModuleScopeRegistry( + this.dtsModuleScopeResolver, this.refEmitter, /* aliasGenerator */ null); evaluator = new PartialEvaluator(this.reflectionHost, this.typeChecker); moduleResolver = new ModuleResolver(this.program, this.options, this.host); importGraph = new ImportGraph(this.moduleResolver); diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/component.ts b/packages/compiler-cli/src/ngtsc/annotations/src/component.ts index aefaf5205a..18b0a8ac8f 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/component.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/component.ts @@ -16,7 +16,7 @@ import {ModuleResolver, Reference, ReferenceEmitter} from '../../imports'; import {EnumValue, PartialEvaluator} from '../../partial_evaluator'; import {Decorator, ReflectionHost, filterToMembersWithDecorator, reflectObjectLiteral} from '../../reflection'; import {LocalModuleScopeRegistry, ScopeDirective, extractDirectiveGuards} from '../../scope'; -import {AnalysisOutput, CompileResult, DecoratorHandler, DetectResult, HandlerPrecedence} from '../../transform'; +import {AnalysisOutput, CompileResult, DecoratorHandler, DetectResult, HandlerPrecedence, ResolveResult} from '../../transform'; import {TypeCheckContext} from '../../typecheck'; import {tsSourceMapBug29300Fixed} from '../../util/src/ts_source_map_bug_29300'; @@ -326,7 +326,7 @@ export class ComponentDecoratorHandler implements } } - resolve(node: ts.ClassDeclaration, analysis: ComponentHandlerData): void { + resolve(node: ts.ClassDeclaration, analysis: ComponentHandlerData): ResolveResult { const context = node.getSourceFile(); // Check whether this component was registered with an NgModule. If so, it should be compiled // under that module's compilation scope. @@ -361,6 +361,7 @@ export class ComponentDecoratorHandler implements this.scopeRegistry.setComponentAsRequiringRemoteScoping(node); } } + return {}; } compile(node: ts.ClassDeclaration, analysis: ComponentHandlerData, pool: ConstantPool): 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 6b5510bdcd..de2ae6649b 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts @@ -15,7 +15,7 @@ import {PartialEvaluator, ResolvedValue} from '../../partial_evaluator'; import {Decorator, ReflectionHost, reflectObjectLiteral, typeNodeToValueExpr} from '../../reflection'; import {NgModuleRouteAnalyzer} from '../../routing'; import {LocalModuleScopeRegistry} from '../../scope'; -import {AnalysisOutput, CompileResult, DecoratorHandler, DetectResult, HandlerPrecedence} from '../../transform'; +import {AnalysisOutput, CompileResult, DecoratorHandler, DetectResult, HandlerPrecedence, ResolveResult} from '../../transform'; import {getSourceFile} from '../../util/src/typescript'; import {generateSetClassMetadataCall} from './metadata'; @@ -175,6 +175,17 @@ export class NgModuleDecoratorHandler implements DecoratorHandler { const moduleResolver = new ModuleResolver(program, options, host); const importGraph = new ImportGraph(moduleResolver); const cycleAnalyzer = new CycleAnalyzer(importGraph); - const scopeRegistry = - new LocalModuleScopeRegistry(new MetadataDtsModuleScopeResolver(checker, reflectionHost)); + const scopeRegistry = new LocalModuleScopeRegistry( + new MetadataDtsModuleScopeResolver(checker, reflectionHost, null), new ReferenceEmitter([]), + null); const refEmitter = new ReferenceEmitter([]); const handler = new ComponentDecoratorHandler( diff --git a/packages/compiler-cli/src/ngtsc/annotations/test/directive_spec.ts b/packages/compiler-cli/src/ngtsc/annotations/test/directive_spec.ts index 87f8ca8c90..8dea509ab5 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/test/directive_spec.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/test/directive_spec.ts @@ -8,6 +8,7 @@ import * as ts from 'typescript'; +import {ReferenceEmitter} from '../../imports'; import {PartialEvaluator} from '../../partial_evaluator'; import {TypeScriptReflectionHost} from '../../reflection'; import {LocalModuleScopeRegistry, MetadataDtsModuleScopeResolver} from '../../scope'; @@ -39,8 +40,9 @@ describe('DirectiveDecoratorHandler', () => { const checker = program.getTypeChecker(); const reflectionHost = new TestReflectionHost(checker); const evaluator = new PartialEvaluator(reflectionHost, checker); - const scopeRegistry = - new LocalModuleScopeRegistry(new MetadataDtsModuleScopeResolver(checker, reflectionHost)); + const scopeRegistry = new LocalModuleScopeRegistry( + new MetadataDtsModuleScopeResolver(checker, reflectionHost, null), new ReferenceEmitter([]), + null); const handler = new DirectiveDecoratorHandler(reflectionHost, evaluator, scopeRegistry, false); const analyzeDirective = (dirName: string) => { diff --git a/packages/compiler-cli/src/ngtsc/imports/index.ts b/packages/compiler-cli/src/ngtsc/imports/index.ts index 50af9d405d..ce541d85ae 100644 --- a/packages/compiler-cli/src/ngtsc/imports/index.ts +++ b/packages/compiler-cli/src/ngtsc/imports/index.ts @@ -6,7 +6,9 @@ * found in the LICENSE file at https://angular.io/license */ +export {AliasGenerator, AliasStrategy} from './src/alias'; export {ImportRewriter, NoopImportRewriter, R3SymbolsImportRewriter, validateAndRewriteCoreSymbol} from './src/core'; export {AbsoluteModuleStrategy, FileToModuleHost, FileToModuleStrategy, LocalIdentifierStrategy, LogicalProjectStrategy, ReferenceEmitStrategy, ReferenceEmitter} from './src/emitter'; +export {Reexport} from './src/reexport'; export {ImportMode, OwningModule, Reference} from './src/references'; export {ModuleResolver} from './src/resolver'; diff --git a/packages/compiler-cli/src/ngtsc/imports/src/alias.ts b/packages/compiler-cli/src/ngtsc/imports/src/alias.ts new file mode 100644 index 0000000000..89b2e54b83 --- /dev/null +++ b/packages/compiler-cli/src/ngtsc/imports/src/alias.ts @@ -0,0 +1,46 @@ +/** + * @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 {Expression, ExternalExpr} from '@angular/compiler'; +import * as ts from 'typescript'; + +import {FileToModuleHost, ReferenceEmitStrategy} from './emitter'; +import {ImportMode, Reference} from './references'; + +// Escape anything that isn't alphanumeric, '/', '_', '.', or '$'. +const CHARS_TO_ESCAPE = /[^a-zA-Z0-9/_\.$]/g; + +export class AliasGenerator { + constructor(private fileToModuleHost: FileToModuleHost) {} + + aliasSymbolName(decl: ts.Declaration, context: ts.SourceFile): string { + if (!ts.isClassDeclaration(decl)) { + throw new Error(`Attempt to write an alias to something which isn't a class`); + } + + // The declared module is used to get the name of the alias. + const declModule = + this.fileToModuleHost.fileNameToModuleName(decl.getSourceFile().fileName, context.fileName); + + const replaced = declModule.replace(CHARS_TO_ESCAPE, '_').replace(/\//g, '$'); + return 'ɵng$' + replaced + '$$' + decl.name !.text; + } + + aliasTo(decl: ts.Declaration, via: ts.SourceFile): Expression { + const name = this.aliasSymbolName(decl, via); + // viaModule is the module it'll actually be imported from. + const moduleName = this.fileToModuleHost.fileNameToModuleName(via.fileName, via.fileName); + return new ExternalExpr({moduleName, name}); + } +} + +export class AliasStrategy implements ReferenceEmitStrategy { + emit(ref: Reference, context: ts.SourceFile, importMode: ImportMode): Expression|null { + return ref.alias; + } +} diff --git a/packages/compiler-cli/src/ngtsc/imports/src/reexport.ts b/packages/compiler-cli/src/ngtsc/imports/src/reexport.ts new file mode 100644 index 0000000000..cd850ab73e --- /dev/null +++ b/packages/compiler-cli/src/ngtsc/imports/src/reexport.ts @@ -0,0 +1,13 @@ +/** + * @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 + */ + +export interface Reexport { + symbolName: string; + asAlias: string; + fromModule: string; +} diff --git a/packages/compiler-cli/src/ngtsc/imports/src/references.ts b/packages/compiler-cli/src/ngtsc/imports/src/references.ts index 348f77fb32..c597e75b95 100644 --- a/packages/compiler-cli/src/ngtsc/imports/src/references.ts +++ b/packages/compiler-cli/src/ngtsc/imports/src/references.ts @@ -6,6 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ +import {Expression} from '@angular/compiler'; import * as ts from 'typescript'; import {identifierOfNode} from '../../util/src/typescript'; @@ -48,6 +49,8 @@ export class Reference { private identifiers: ts.Identifier[] = []; + private _alias: Expression|null = null; + constructor(readonly node: T, bestGuessOwningModule: OwningModule|null = null) { this.bestGuessOwningModule = bestGuessOwningModule; @@ -87,6 +90,9 @@ export class Reference { return id !== null ? id.text : null; } + get alias(): Expression|null { return this._alias; } + + /** * Record a `ts.Identifier` by which it's valid to refer to this node, within the context of this * `Reference`. @@ -100,4 +106,18 @@ export class Reference { getIdentityIn(context: ts.SourceFile): ts.Identifier|null { return this.identifiers.find(id => id.getSourceFile() === context) || null; } + + cloneWithAlias(alias: Expression): Reference { + const ref = new Reference(this.node, this.bestGuessOwningModule); + ref.identifiers = [...this.identifiers]; + ref._alias = alias; + return ref; + } + + cloneWithNoIdentifiers(): Reference { + const ref = new Reference(this.node, this.bestGuessOwningModule); + ref._alias = this._alias; + ref.identifiers = []; + return ref; + } } diff --git a/packages/compiler-cli/src/ngtsc/program.ts b/packages/compiler-cli/src/ngtsc/program.ts index f6ea81d4a7..22a667ef87 100644 --- a/packages/compiler-cli/src/ngtsc/program.ts +++ b/packages/compiler-cli/src/ngtsc/program.ts @@ -17,7 +17,7 @@ import {BaseDefDecoratorHandler} from './annotations/src/base_def'; import {CycleAnalyzer, ImportGraph} from './cycles'; import {ErrorCode, ngErrorCode} from './diagnostics'; import {FlatIndexGenerator, ReferenceGraph, checkForPrivateExports, findFlatIndexEntryPoint} from './entry_point'; -import {AbsoluteModuleStrategy, FileToModuleHost, FileToModuleStrategy, ImportRewriter, LocalIdentifierStrategy, LogicalProjectStrategy, ModuleResolver, NoopImportRewriter, R3SymbolsImportRewriter, Reference, ReferenceEmitter} from './imports'; +import {AbsoluteModuleStrategy, AliasGenerator, AliasStrategy, FileToModuleHost, FileToModuleStrategy, ImportRewriter, LocalIdentifierStrategy, LogicalProjectStrategy, ModuleResolver, NoopImportRewriter, R3SymbolsImportRewriter, Reference, ReferenceEmitter} from './imports'; import {PartialEvaluator} from './partial_evaluator'; import {AbsoluteFsPath, LogicalFileSystem} from './path'; import {TypeScriptReflectionHost} from './reflection'; @@ -27,6 +27,7 @@ import {LocalModuleScopeRegistry, MetadataDtsModuleScopeResolver} from './scope' import {FactoryGenerator, FactoryInfo, GeneratedShimsHostWrapper, ShimGenerator, SummaryGenerator, generatedFactoryTransform} from './shims'; import {ivySwitchTransform} from './switch'; import {IvyCompilation, declarationTransformFactory, ivyTransformFactory} from './transform'; +import {aliasTransformFactory} from './transform/src/alias'; import {TypeCheckContext, TypeCheckProgramHost} from './typecheck'; import {normalizeSeparators} from './util/src/path'; import {getRootDirs, isDtsPath} from './util/src/typescript'; @@ -272,10 +273,16 @@ export class NgtscProgram implements api.Program { }; const customTransforms = opts && opts.customTransformers; - const beforeTransforms = [ivyTransformFactory( - compilation, this.reflector, this.importRewriter, this.isCore, - this.closureCompilerEnabled)]; - const afterDeclarationsTransforms = [declarationTransformFactory(compilation)]; + const beforeTransforms = [ + ivyTransformFactory( + compilation, this.reflector, this.importRewriter, this.isCore, + this.closureCompilerEnabled), + aliasTransformFactory(compilation.exportStatements) as ts.TransformerFactory, + ]; + const afterDeclarationsTransforms = [ + declarationTransformFactory(compilation), + ]; + if (this.factoryToSourceInfo !== null) { beforeTransforms.push( @@ -314,6 +321,8 @@ export class NgtscProgram implements api.Program { private makeCompilation(): IvyCompilation { const checker = this.tsProgram.getTypeChecker(); + + let aliasGenerator: AliasGenerator|null = null; // Construct the ReferenceEmitter. if (this.fileToModuleHost === null || !this.options._useHostForImportGeneration) { // The CompilerHost doesn't have fileNameToModuleName, so build an NPM-centric reference @@ -333,14 +342,19 @@ export class NgtscProgram implements api.Program { this.refEmitter = new ReferenceEmitter([ // First, try to use local identifiers if available. new LocalIdentifierStrategy(), + // Then use aliased references (this is a workaround to StrictDeps checks). + new AliasStrategy(), // Then use fileNameToModuleName to emit imports. new FileToModuleStrategy(checker, this.fileToModuleHost), ]); + aliasGenerator = new AliasGenerator(this.fileToModuleHost); } const evaluator = new PartialEvaluator(this.reflector, checker); - const depScopeReader = new MetadataDtsModuleScopeResolver(checker, this.reflector); - const scopeRegistry = new LocalModuleScopeRegistry(depScopeReader); + const depScopeReader = + new MetadataDtsModuleScopeResolver(checker, this.reflector, aliasGenerator); + const scopeRegistry = + new LocalModuleScopeRegistry(depScopeReader, this.refEmitter, aliasGenerator); // If a flat module entrypoint was specified, then track references via a `ReferenceGraph` in diff --git a/packages/compiler-cli/src/ngtsc/scope/src/dependency.ts b/packages/compiler-cli/src/ngtsc/scope/src/dependency.ts index bf557d7975..db3c587adf 100644 --- a/packages/compiler-cli/src/ngtsc/scope/src/dependency.ts +++ b/packages/compiler-cli/src/ngtsc/scope/src/dependency.ts @@ -8,7 +8,7 @@ import * as ts from 'typescript'; -import {Reference} from '../../imports'; +import {AliasGenerator, Reference} from '../../imports'; import {ReflectionHost} from '../../reflection'; import {ExportScope, ScopeDirective, ScopePipe} from './api'; @@ -31,7 +31,9 @@ export class MetadataDtsModuleScopeResolver { */ private cache = new Map(); - constructor(private checker: ts.TypeChecker, private reflector: ReflectionHost) {} + constructor( + private checker: ts.TypeChecker, private reflector: ReflectionHost, + private aliasGenerator: AliasGenerator|null) {} /** * Resolve a `Reference`'d NgModule from a .d.ts file and produce a transitive `ExportScope` @@ -42,9 +44,10 @@ export class MetadataDtsModuleScopeResolver { */ resolve(ref: Reference): ExportScope|null { const clazz = ref.node; - if (!clazz.getSourceFile().isDeclarationFile) { + const sourceFile = clazz.getSourceFile(); + if (!sourceFile.isDeclarationFile) { throw new Error( - `Debug error: DtsModuleScopeResolver.read(${ref.debugName} from ${clazz.getSourceFile().fileName}), but not a .d.ts file`); + `Debug error: DtsModuleScopeResolver.read(${ref.debugName} from ${sourceFile.fileName}), but not a .d.ts file`); } if (this.cache.has(clazz)) { @@ -61,31 +64,64 @@ export class MetadataDtsModuleScopeResolver { return null; } + const declarations = new Set(); + for (const declRef of meta.declarations) { + declarations.add(declRef.node); + } + // Only the 'exports' field of the NgModule's metadata is important. Imports and declarations // don't affect the export scope. for (const exportRef of meta.exports) { // Attempt to process the export as a directive. const directive = this.readScopeDirectiveFromClassWithDef(exportRef); if (directive !== null) { - directives.push(directive); + if (!declarations.has(exportRef.node)) { + directives.push(this.maybeAlias(directive, sourceFile)); + } else { + directives.push(directive); + } continue; } // Attempt to process the export as a pipe. const pipe = this.readScopePipeFromClassWithDef(exportRef); if (pipe !== null) { - pipes.push(pipe); + if (!declarations.has(exportRef.node)) { + pipes.push(this.maybeAlias(pipe, sourceFile)); + } else { + pipes.push(pipe); + } continue; } // Attempt to process the export as a module. const exportScope = this.resolve(exportRef); if (exportScope !== null) { - // It is a module. Add exported directives and pipes to the current scope. - directives.push(...exportScope.exported.directives); - pipes.push(...exportScope.exported.pipes); - continue; + // It is a module. Add exported directives and pipes to the current scope. This might + // involve rewriting the `Reference`s to those types to have an alias expression if one is + // required. + if (this.aliasGenerator === null) { + // Fast path when aliases aren't required. + directives.push(...exportScope.exported.directives); + pipes.push(...exportScope.exported.pipes); + } else { + // It's necessary to rewrite the `Reference`s to add alias expressions. This way, imports + // generated to these directives and pipes will use a shallow import to `sourceFile` + // instead of a deep import directly to the directive or pipe class. + // + // One important check here is whether the directive/pipe is declared in the same + // source file as the re-exporting NgModule. This can happen if both a directive, its + // NgModule, and the re-exporting NgModule are all in the same file. In this case, + // no import alias is needed as it would go to the same file anyway. + for (const directive of exportScope.exported.directives) { + directives.push(this.maybeAlias(directive, sourceFile)); + } + for (const pipe of exportScope.exported.pipes) { + pipes.push(this.maybeAlias(pipe, sourceFile)); + } + } } + continue; // The export was not a directive, a pipe, or a module. This is an error. // TODO(alxhub): produce a ts.Diagnostic @@ -190,6 +226,22 @@ export class MetadataDtsModuleScopeResolver { const name = type.literal.text; return {ref, name}; } + + private maybeAlias( + dirOrPipe: T, maybeAliasFrom: ts.SourceFile): T { + if (this.aliasGenerator === null) { + return dirOrPipe; + } + const ref = dirOrPipe.ref; + if (ref.node.getSourceFile() !== maybeAliasFrom) { + return { + ...dirOrPipe, + ref: ref.cloneWithAlias(this.aliasGenerator.aliasTo(ref.node, maybeAliasFrom)), + }; + } else { + return dirOrPipe; + } + } } /** diff --git a/packages/compiler-cli/src/ngtsc/scope/src/local.ts b/packages/compiler-cli/src/ngtsc/scope/src/local.ts index c2c4fb9e23..13b496e405 100644 --- a/packages/compiler-cli/src/ngtsc/scope/src/local.ts +++ b/packages/compiler-cli/src/ngtsc/scope/src/local.ts @@ -6,9 +6,10 @@ * found in the LICENSE file at https://angular.io/license */ +import {ExternalExpr} from '@angular/compiler'; import * as ts from 'typescript'; -import {Reference} from '../../imports'; +import {AliasGenerator, Reexport, Reference, ReferenceEmitter} from '../../imports'; import {ExportScope, ScopeData, ScopeDirective, ScopePipe} from './api'; import {DtsModuleScopeResolver} from './dependency'; @@ -19,14 +20,10 @@ export interface LocalNgModuleData { exports: Reference[]; } -/** - * A scope produced for an NgModule declared locally (in the current program being compiled). - * - * The `LocalModuleScope` contains the compilation scope, the transitive set of directives and pipes - * visible to any component declared in this module. It also contains an `ExportScope`, the - * transitive set of directives and pipes - */ -export interface LocalModuleScope extends ExportScope { compilation: ScopeData; } +export interface LocalModuleScope extends ExportScope { + compilation: ScopeData; + reexports: Reexport[]|null; +} /** * A registry which collects information about NgModules, Directives, Components, and Pipes which @@ -91,7 +88,9 @@ export class LocalModuleScopeRegistry { */ private remoteScoping = new Set(); - constructor(private dependencyScopeReader: DtsModuleScopeResolver) {} + constructor( + private dependencyScopeReader: DtsModuleScopeResolver, private refEmitter: ReferenceEmitter, + private aliasGenerator: AliasGenerator|null) {} /** * Add an NgModule's data to the registry. @@ -151,6 +150,9 @@ export class LocalModuleScopeRegistry { const compilationDirectives = new Map(); const compilationPipes = new Map(); + const declared = new Set(); + const sourceFile = clazz.getSourceFile(); + // Directives and pipes exported to any importing NgModules. const exportDirectives = new Map(); const exportPipes = new Map(); @@ -181,6 +183,8 @@ export class LocalModuleScopeRegistry { // ngtools tests rely on analysis of broken components. continue; } + + declared.add(decl.node); } // 2) process imports. @@ -229,16 +233,55 @@ export class LocalModuleScopeRegistry { } } + const exported = { + directives: Array.from(exportDirectives.values()), + pipes: Array.from(exportPipes.values()), + }; + + let reexports: Reexport[]|null = null; + if (this.aliasGenerator !== null) { + reexports = []; + const addReexport = (ref: Reference) => { + if (!declared.has(ref.node) && ref.node.getSourceFile() !== sourceFile) { + const exportName = this.aliasGenerator !.aliasSymbolName(ref.node, sourceFile); + if (ref.alias && ref.alias instanceof ExternalExpr) { + reexports !.push({ + fromModule: ref.alias.value.moduleName !, + symbolName: ref.alias.value.name !, + asAlias: exportName, + }); + } else { + const expr = this.refEmitter.emit(ref.cloneWithNoIdentifiers(), sourceFile); + if (!(expr instanceof ExternalExpr) || expr.value.moduleName === null || + expr.value.name === null) { + throw new Error('Expected ExternalExpr'); + } + reexports !.push({ + fromModule: expr.value.moduleName, + symbolName: expr.value.name, + asAlias: exportName, + }); + } + } + }; + for (const {ref} of exported.directives) { + addReexport(ref); + } + for (const {ref} of exported.pipes) { + addReexport(ref); + } + } + + + // Finally, produce the `LocalModuleScope` with both the compilation and export scopes. const scope = { compilation: { directives: Array.from(compilationDirectives.values()), pipes: Array.from(compilationPipes.values()), }, - exported: { - directives: Array.from(exportDirectives.values()), - pipes: Array.from(exportPipes.values()), - }, + exported, + reexports, }; this.cache.set(clazz, scope); return scope; diff --git a/packages/compiler-cli/src/ngtsc/scope/test/BUILD.bazel b/packages/compiler-cli/src/ngtsc/scope/test/BUILD.bazel index 07e44cf254..ed9c2b3e6e 100644 --- a/packages/compiler-cli/src/ngtsc/scope/test/BUILD.bazel +++ b/packages/compiler-cli/src/ngtsc/scope/test/BUILD.bazel @@ -10,6 +10,7 @@ ts_library( ]), deps = [ "//packages:types", + "//packages/compiler", "//packages/compiler-cli/src/ngtsc/imports", "//packages/compiler-cli/src/ngtsc/reflection", "//packages/compiler-cli/src/ngtsc/scope", diff --git a/packages/compiler-cli/src/ngtsc/scope/test/dependency_spec.ts b/packages/compiler-cli/src/ngtsc/scope/test/dependency_spec.ts index f045170d92..5b722a2755 100644 --- a/packages/compiler-cli/src/ngtsc/scope/test/dependency_spec.ts +++ b/packages/compiler-cli/src/ngtsc/scope/test/dependency_spec.ts @@ -6,17 +6,24 @@ * found in the LICENSE file at https://angular.io/license */ +import {ExternalExpr, ExternalReference} from '@angular/compiler'; import * as ts from 'typescript'; -import {Reference} from '../../imports'; +import {AliasGenerator, FileToModuleHost, Reference} from '../../imports'; import {TypeScriptReflectionHost} from '../../reflection'; import {makeProgram} from '../../testing/in_memory_typescript'; - import {ExportScope} from '../src/api'; import {MetadataDtsModuleScopeResolver} from '../src/dependency'; const MODULE_FROM_NODE_MODULES_PATH = /.*node_modules\/(\w+)\/index\.d\.ts$/; +const testHost: FileToModuleHost = { + fileNameToModuleName: function(imported: string): string { + const res = MODULE_FROM_NODE_MODULES_PATH.exec(imported) !; + return 'root/' + res[1]; + } +}; + /** * Simple metadata types are added to the top of each testing file, for convenience. */ @@ -33,7 +40,8 @@ export declare type PipeMeta = never; * This returns both the `MetadataDtsModuleScopeResolver` and a `refs` object which can be * destructured to retrieve references to specific declared classes. */ -function makeTestEnv(modules: {[module: string]: string}): { +function makeTestEnv( + modules: {[module: string]: string}, aliasGenerator: AliasGenerator | null = null): { refs: {[name: string]: Reference}, resolver: MetadataDtsModuleScopeResolver, } { @@ -46,8 +54,8 @@ function makeTestEnv(modules: {[module: string]: string}): { }); const {program} = makeProgram(files); const checker = program.getTypeChecker(); - const resolver = - new MetadataDtsModuleScopeResolver(checker, new TypeScriptReflectionHost(checker)); + const resolver = new MetadataDtsModuleScopeResolver( + checker, new TypeScriptReflectionHost(checker), aliasGenerator); // Resolver for the refs object. const get = (target: {}, name: string): Reference => { @@ -135,10 +143,143 @@ describe('MetadataDtsModuleScopeResolver', () => { // Explicitly verify that the directive has the correct owning module. expect(scope.exported.directives[0].ref.ownedByModuleGuess).toBe('declaration'); }); + + it('should write correct aliases for deep dependencies', () => { + const {resolver, refs} = makeTestEnv( + { + 'deep': ` + export declare class DeepDir { + static ngDirectiveDef: DirectiveMeta; + } + + export declare class DeepModule { + static ngModuleDef: ModuleMeta; + } + `, + 'middle': ` + import * as deep from 'deep'; + + export declare class MiddleDir { + static ngDirectiveDef: DirectiveMeta; + } + + export declare class MiddleModule { + static ngModuleDef: ModuleMeta; + } + `, + 'shallow': ` + import * as middle from 'middle'; + + export declare class ShallowDir { + static ngDirectiveDef: DirectiveMeta; + } + + export declare class ShallowModule { + static ngModuleDef: ModuleMeta; + } + `, + }, + new AliasGenerator(testHost)); + const {ShallowModule} = refs; + const scope = resolver.resolve(ShallowModule) !; + const [DeepDir, MiddleDir, ShallowDir] = scopeToRefs(scope); + expect(getAlias(DeepDir)).toEqual({ + moduleName: 'root/shallow', + name: 'ɵng$root$deep$$DeepDir', + }); + expect(getAlias(MiddleDir)).toEqual({ + moduleName: 'root/shallow', + name: 'ɵng$root$middle$$MiddleDir', + }); + expect(getAlias(ShallowDir)).toBeNull(); + }); + + it('should write correct aliases for bare directives in exports', () => { + const {resolver, refs} = makeTestEnv( + { + 'deep': ` + export declare class DeepDir { + static ngDirectiveDef: DirectiveMeta; + } + + export declare class DeepModule { + static ngModuleDef: ModuleMeta; + } + `, + 'middle': ` + import * as deep from 'deep'; + + export declare class MiddleDir { + static ngDirectiveDef: DirectiveMeta; + } + + export declare class MiddleModule { + static ngModuleDef: ModuleMeta; + } + `, + 'shallow': ` + import * as middle from 'middle'; + + export declare class ShallowDir { + static ngDirectiveDef: DirectiveMeta; + } + + export declare class ShallowModule { + static ngModuleDef: ModuleMeta; + } + `, + }, + new AliasGenerator(testHost)); + const {ShallowModule} = refs; + const scope = resolver.resolve(ShallowModule) !; + const [DeepDir, MiddleDir, ShallowDir] = scopeToRefs(scope); + expect(getAlias(DeepDir)).toEqual({ + moduleName: 'root/shallow', + name: 'ɵng$root$deep$$DeepDir', + }); + expect(getAlias(MiddleDir)).toEqual({ + moduleName: 'root/shallow', + name: 'ɵng$root$middle$$MiddleDir', + }); + expect(getAlias(ShallowDir)).toBeNull(); + }); + + it('should not use an alias if a directive is declared in the same file as the re-exporting module', + () => { + const {resolver, refs} = makeTestEnv( + { + 'module': ` + export declare class DeepDir { + static ngDirectiveDef: DirectiveMeta; + } + + export declare class DeepModule { + static ngModuleDef: ModuleMeta; + } + + export declare class DeepExportModule { + static ngModuleDef: ModuleMeta; + } + `, + }, + new AliasGenerator(testHost)); + const {DeepExportModule} = refs; + const scope = resolver.resolve(DeepExportModule) !; + const [DeepDir] = scopeToRefs(scope); + expect(getAlias(DeepDir)).toBeNull(); + }); }); -function scopeToRefs(scope: ExportScope): Reference[] { +function scopeToRefs(scope: ExportScope): Reference[] { const directives = scope.exported.directives.map(dir => dir.ref); - const pipes = scope.exported.pipes.map(pipe => pipe.ref); + const pipes = scope.exported.pipes.map(pipe => pipe.ref as Reference); return [...directives, ...pipes].sort((a, b) => a.debugName !.localeCompare(b.debugName !)); } + +function getAlias(ref: Reference): ExternalReference|null { + if (ref.alias === null) { + return null; + } else { + return (ref.alias as ExternalExpr).value; + } +} diff --git a/packages/compiler-cli/src/ngtsc/scope/test/local_spec.ts b/packages/compiler-cli/src/ngtsc/scope/test/local_spec.ts index 94803ce348..6a38e632d3 100644 --- a/packages/compiler-cli/src/ngtsc/scope/test/local_spec.ts +++ b/packages/compiler-cli/src/ngtsc/scope/test/local_spec.ts @@ -8,7 +8,7 @@ import * as ts from 'typescript'; -import {Reference} from '../../imports'; +import {Reference, ReferenceEmitter} from '../../imports'; import {ScopeData, ScopeDirective, ScopePipe} from '../src/api'; import {DtsModuleScopeResolver} from '../src/dependency'; import {LocalModuleScopeRegistry} from '../src/local'; @@ -31,9 +31,12 @@ function registerFakeRefs(registry: LocalModuleScopeRegistry): } describe('LocalModuleScopeRegistry', () => { + const refEmitter = new ReferenceEmitter([]); let registry !: LocalModuleScopeRegistry; - beforeEach(() => { registry = new LocalModuleScopeRegistry(new MockDtsModuleScopeResolver()); }); + beforeEach(() => { + registry = new LocalModuleScopeRegistry(new MockDtsModuleScopeResolver(), refEmitter, null); + }); it('should produce an accurate LocalModuleScope for a basic NgModule', () => { const {Dir1, Dir2, Pipe1, Module} = registerFakeRefs(registry); diff --git a/packages/compiler-cli/src/ngtsc/transform/src/alias.ts b/packages/compiler-cli/src/ngtsc/transform/src/alias.ts new file mode 100644 index 0000000000..359d3bf44f --- /dev/null +++ b/packages/compiler-cli/src/ngtsc/transform/src/alias.ts @@ -0,0 +1,36 @@ +/** + * @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'; + +export function aliasTransformFactory(exportStatements: Map>): + ts.TransformerFactory { + return (context: ts.TransformationContext) => { + return (file: ts.SourceFile | ts.Bundle) => { + if (ts.isBundle(file) || !exportStatements.has(file.fileName)) { + return file; + } + + const statements = [...file.statements]; + exportStatements.get(file.fileName) !.forEach(([moduleName, symbolName], aliasName) => { + const stmt = ts.createExportDeclaration( + /* decorators */ undefined, + /* modifiers */ undefined, + /* exportClause */ ts.createNamedExports([ts.createExportSpecifier( + /* propertyName */ symbolName, + /* name */ aliasName)]), + /* moduleSpecifier */ ts.createStringLiteral(moduleName)); + statements.push(stmt); + }); + + file = ts.getMutableClone(file); + file.statements = ts.createNodeArray(statements); + return file; + }; + }; +} diff --git a/packages/compiler-cli/src/ngtsc/transform/src/api.ts b/packages/compiler-cli/src/ngtsc/transform/src/api.ts index ab86acdb18..1120661ae1 100644 --- a/packages/compiler-cli/src/ngtsc/transform/src/api.ts +++ b/packages/compiler-cli/src/ngtsc/transform/src/api.ts @@ -9,6 +9,7 @@ import {ConstantPool, Expression, Statement, Type} from '@angular/compiler'; import * as ts from 'typescript'; +import {Reexport} from '../../imports'; import {Decorator} from '../../reflection'; import {TypeCheckContext} from '../../typecheck'; @@ -82,7 +83,7 @@ export interface DecoratorHandler { * `DecoratorHandler` a chance to leverage information from the whole compilation unit to enhance * the `analysis` before the emit phase. */ - resolve?(node: ts.Declaration, analysis: A): void; + resolve?(node: ts.Declaration, analysis: A): ResolveResult; typeCheck?(ctx: TypeCheckContext, node: ts.Declaration, metadata: A): void; @@ -121,3 +122,5 @@ export interface CompileResult { statements: Statement[]; type: Type; } + +export interface ResolveResult { reexports?: Reexport[]; } diff --git a/packages/compiler-cli/src/ngtsc/transform/src/compilation.ts b/packages/compiler-cli/src/ngtsc/transform/src/compilation.ts index 7ce20e9b12..240fcc077e 100644 --- a/packages/compiler-cli/src/ngtsc/transform/src/compilation.ts +++ b/packages/compiler-cli/src/ngtsc/transform/src/compilation.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {ConstantPool} from '@angular/compiler'; +import {ConstantPool, ExternalExpr} from '@angular/compiler'; import * as ts from 'typescript'; import {ErrorCode, FatalDiagnosticError} from '../../diagnostics'; @@ -58,6 +58,8 @@ export class IvyCompilation { * Tracks the `DtsFileTransformer`s for each TS file that needs .d.ts transformations. */ private dtsMap = new Map(); + + private reexportMap = new Map>(); private _diagnostics: ts.Diagnostic[] = []; @@ -76,6 +78,8 @@ export class IvyCompilation { private sourceToFactorySymbols: Map>|null) {} + get exportStatements(): Map> { return this.reexportMap; } + analyzeSync(sf: ts.SourceFile): void { return this.analyze(sf, false); } analyzeAsync(sf: ts.SourceFile): Promise|undefined { return this.analyze(sf, true); } @@ -243,7 +247,17 @@ export class IvyCompilation { for (const match of ivyClass.matchedHandlers) { if (match.handler.resolve !== undefined && match.analyzed !== null && match.analyzed.analysis !== undefined) { - match.handler.resolve(node, match.analyzed.analysis); + const res = match.handler.resolve(node, match.analyzed.analysis); + if (res.reexports !== undefined) { + const fileName = node.getSourceFile().fileName; + if (!this.reexportMap.has(fileName)) { + this.reexportMap.set(fileName, new Map()); + } + const fileReexports = this.reexportMap.get(fileName) !; + for (const reexport of res.reexports) { + fileReexports.set(reexport.asAlias, [reexport.fromModule, reexport.symbolName]); + } + } } } }); diff --git a/packages/compiler-cli/src/transformers/compiler_host.ts b/packages/compiler-cli/src/transformers/compiler_host.ts index 284c96db55..3822745387 100644 --- a/packages/compiler-cli/src/transformers/compiler_host.ts +++ b/packages/compiler-cli/src/transformers/compiler_host.ts @@ -21,9 +21,20 @@ const NODE_MODULES_PACKAGE_NAME = /node_modules\/((\w|-|\.)+|(@(\w|-|\.)+\/(\w|- const EXT = /(\.ts|\.d\.ts|\.js|\.jsx|\.tsx)$/; const CSS_PREPROCESSOR_EXT = /(\.scss|\.less|\.styl)$/; +let augmentHostForTest: {[name: string]: Function}|null = null; + +export function setAugmentHostForTest(augmentation: {[name: string]: Function} | null): void { + augmentHostForTest = augmentation; +} + export function createCompilerHost( {options, tsHost = ts.createCompilerHost(options, true)}: {options: CompilerOptions, tsHost?: ts.CompilerHost}): CompilerHost { + if (augmentHostForTest !== null) { + for (const name of Object.keys(augmentHostForTest)) { + (tsHost as any)[name] = augmentHostForTest[name]; + } + } return tsHost; } diff --git a/packages/compiler-cli/test/ngtsc/env.ts b/packages/compiler-cli/test/ngtsc/env.ts index bb95a3cbb6..9b5d29b25a 100644 --- a/packages/compiler-cli/test/ngtsc/env.ts +++ b/packages/compiler-cli/test/ngtsc/env.ts @@ -7,6 +7,7 @@ */ import {CustomTransformers} from '@angular/compiler-cli'; +import {setAugmentHostForTest} from '@angular/compiler-cli/src/transformers/compiler_host'; import * as fs from 'fs'; import * as path from 'path'; import * as ts from 'typescript'; @@ -49,6 +50,7 @@ export class NgtscTestEnvironment { process.chdir(support.basePath); setupFakeCore(support); + setAugmentHostForTest(null); const env = new NgtscTestEnvironment(support, outDir); @@ -108,6 +110,15 @@ export class NgtscTestEnvironment { }; } this.write('tsconfig.json', JSON.stringify(tsconfig, null, 2)); + + if (extraOpts['_useHostForImportGeneration'] === true) { + const cwd = process.cwd(); + setAugmentHostForTest({ + fileNameToModuleName: (importedFilePath: string) => { + return 'root' + importedFilePath.substr(cwd.length).replace(/(\.d)?.ts$/, ''); + } + }); + } } /** diff --git a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts index b26110382a..18d7eb7c80 100644 --- a/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts +++ b/packages/compiler-cli/test/ngtsc/ngtsc_spec.ts @@ -532,10 +532,14 @@ describe('ngtsc behavioral tests', () => { }) export class FooModule {} `); - env.write('node_modules/foo/index.d.ts', ` - import * as i0 from '@angular/core'; + env.write('node_modules/foo/index.ts', ` + import {Component} from '@angular/core'; + + @Component({ + selector: 'foo', + template: '', + }) export class Foo { - static ngComponentDef: i0.ɵComponentDef; } `); @@ -950,10 +954,11 @@ describe('ngtsc behavioral tests', () => { `); env.write('node_modules/router/index.d.ts', ` - import {ModuleWithProviders} from '@angular/core'; + import {ModuleWithProviders, ɵNgModuleDefWithMeta} from '@angular/core'; declare class RouterModule { static forRoot(): ModuleWithProviders; + static ngModuleDef: ɵNgModuleDefWithMeta; } `); @@ -990,7 +995,10 @@ describe('ngtsc behavioral tests', () => { `); env.write('node_modules/router/internal.d.ts', ` - export declare class InternalRouterModule {} + import {ɵNgModuleDefWithMeta} from '@angular/core'; + export declare class InternalRouterModule { + static ngModuleDef: ɵNgModuleDefWithMeta; + } `); env.driveMain(); @@ -1018,12 +1026,13 @@ describe('ngtsc behavioral tests', () => { `); env.write('node_modules/router/index.d.ts', ` - import {ModuleWithProviders} from '@angular/core'; + import {ModuleWithProviders, ɵNgModuleDefWithMeta} from '@angular/core'; export interface MyType extends ModuleWithProviders {} declare class RouterModule { static forRoot(): (MyType)&{ngModule:RouterModule}; + static ngModuleDef: ɵNgModuleDefWithMeta; } `); @@ -2571,7 +2580,7 @@ describe('ngtsc behavioral tests', () => { export declare class RouterModule { static forRoot(arg1: any, arg2: any): ModuleWithProviders; static forChild(arg1: any): ModuleWithProviders; - static ngModuleDef: NgModuleDefWithMeta + static ngModuleDef: NgModuleDefWithMeta; } `); }); @@ -2853,6 +2862,7 @@ describe('ngtsc behavioral tests', () => { Test2Module, ], imports: [ + Test2Module, RouterModule.forRoot([ {path: '', loadChildren: './lazy-1/lazy-1#Lazy1Module'}, ]), @@ -3193,6 +3203,118 @@ export const Foo = Foo__PRE_R3__; expect(sourceTestInsideAngularCore).toContain(sourceTestOutsideAngularCore); }); }); + + describe('NgModule export aliasing', () => { + it('should use an alias to import a directive from a deep dependency', () => { + env.tsconfig({'_useHostForImportGeneration': true}); + + // 'alpha' declares the directive which will ultimately be imported. + env.write('alpha.d.ts', ` + import {ɵDirectiveDefWithMeta, ɵNgModuleDefWithMeta} from '@angular/core'; + + export declare class ExternalDir { + static ngDirectiveDef: ɵDirectiveDefWithMeta; + } + + export declare class AlphaModule { + static ngModuleDef: ɵNgModuleDefWithMeta; + } + `); + + // 'beta' re-exports AlphaModule from alpha. + env.write('beta.d.ts', ` + import {ɵNgModuleDefWithMeta} from '@angular/core'; + import {AlphaModule} from './alpha'; + + export declare class BetaModule { + static ngModuleDef: ɵNgModuleDefWithMeta; + } + `); + + // The application imports BetaModule from beta, gaining visibility of ExternalDir from alpha. + env.write('test.ts', ` + import {Component, NgModule} from '@angular/core'; + import {BetaModule} from './beta'; + + @Component({ + selector: 'cmp', + template: '
', + }) + export class Cmp {} + + @NgModule({ + declarations: [Cmp], + imports: [BetaModule], + }) + export class Module {} + `); + env.driveMain(); + const jsContents = env.getContents('test.js'); + + // Expect that ExternalDir from alpha is imported via the re-export from beta. + expect(jsContents).toContain('import * as i1 from "root/beta";'); + expect(jsContents).toContain('directives: [i1.ɵng$root$alpha$$ExternalDir]'); + }); + + it('should write alias ES2015 exports for NgModule exported directives', () => { + env.tsconfig({'_useHostForImportGeneration': true}); + env.write('external.d.ts', ` + import {ɵDirectiveDefWithMeta, ɵNgModuleDefWithMeta} from '@angular/core'; + import {LibModule} from './lib'; + + export declare class ExternalDir { + static ngDirectiveDef: ɵDirectiveDefWithMeta; + } + + export declare class ExternalModule { + static ngModuleDef: ɵNgModuleDefWithMeta; + } + `); + env.write('lib.d.ts', ` + import {ɵDirectiveDefWithMeta, ɵNgModuleDefWithMeta} from '@angular/core'; + + export declare class LibDir { + static ngDirectiveDef: ɵDirectiveDefWithMeta; + } + + export declare class LibModule { + static ngModuleDef: ɵNgModuleDefWithMeta; + } + `); + env.write('foo.ts', ` + import {Directive, NgModule} from '@angular/core'; + import {ExternalModule} from './external'; + + @Directive({selector: '[foo]'}) + export class FooDir {} + + @NgModule({ + declarations: [FooDir], + exports: [FooDir, ExternalModule] + }) + export class FooModule {} + `); + env.write('index.ts', ` + import {Component, NgModule} from '@angular/core'; + import {FooModule} from './foo'; + + @Component({ + selector: 'index', + template: '
', + }) + export class IndexCmp {} + + @NgModule({ + declarations: [IndexCmp], + exports: [FooModule], + }) + export class IndexModule {} + `); + env.driveMain(); + const jsContents = env.getContents('index.js'); + expect(jsContents).toContain('export { FooDir as ɵng$root$foo$$FooDir } from "root/foo";'); + }); + }); }); function expectTokenAtPosition(