From 87bca2a5c61d3ca0cb6c92bdb7985bfe8ef747aa Mon Sep 17 00:00:00 2001 From: JoostK Date: Mon, 15 Feb 2021 22:28:19 +0100 Subject: [PATCH] perf(compiler-cli): avoid module resolution in cycle analysis (#40948) The compiler performs cycle analysis for the used directives and pipes of a component's template to avoid introducing a cyclic import into the generated output. The used directives and pipes are represented by their output expression which would typically be an `ExternalExpr`; those are responsible for the generation of an `import` statement. Cycle analysis needs to determine the `ts.SourceFile` that would end up being imported by these `ExternalExpr`s, as the `ts.SourceFile` is then checked against the program's `ImportGraph` to determine if the import is allowed, i.e. does not introduce a cycle. To accomplish this, the `ExternalExpr` was dissected and ran through module resolution to obtain the imported `ts.SourceFile`. This module resolution step is relatively expensive, as it typically needs to hit the filesystem. Even in the presence of a module resolution cache would these module resolution requests generally see cache misses, as the generated import originates from a file for which the cache has not previously seen the imported module specifier. This commit removes the need for the module resolution by wrapping the generated `Expression` in an `EmittedReference` struct. This allows the reference emitter mechanism that is responsible for generating the `Expression` to also communicate from which `ts.SourceFile` the generated `Expression` would be imported, precluding the need for module resolution down the road. PR Close #40948 --- .../src/ngtsc/annotations/src/component.ts | 62 ++++++--- .../src/ngtsc/annotations/src/ng_module.ts | 10 +- .../src/ngtsc/annotations/src/util.ts | 13 +- .../compiler-cli/src/ngtsc/imports/index.ts | 2 +- .../src/ngtsc/imports/src/alias.ts | 11 +- .../src/ngtsc/imports/src/emitter.ts | 118 +++++++++++++----- .../src/ngtsc/imports/test/emitter_spec.ts | 20 +-- .../ngtsc/modulewithproviders/src/scanner.ts | 2 +- .../compiler-cli/src/ngtsc/scope/src/local.ts | 3 +- .../src/ngtsc/typecheck/src/environment.ts | 4 +- 10 files changed, 162 insertions(+), 83 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/component.ts b/packages/compiler-cli/src/ngtsc/annotations/src/component.ts index 0f31c76a52..ed83da4796 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/component.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/component.ts @@ -12,7 +12,7 @@ import * as ts from 'typescript'; import {Cycle, CycleAnalyzer, CycleHandlingStrategy} from '../../cycles'; import {ErrorCode, FatalDiagnosticError, makeDiagnostic, makeRelatedInformation} from '../../diagnostics'; import {absoluteFrom, relative} from '../../file_system'; -import {DefaultImportRecorder, ModuleResolver, Reference, ReferenceEmitter} from '../../imports'; +import {DefaultImportRecorder, ImportedFile, ModuleResolver, Reference, ReferenceEmitter} from '../../imports'; import {DependencyTracker} from '../../incremental/api'; import {extractSemanticTypeParameters, isArrayEqual, isReferenceEqual, SemanticDepGraphUpdater, SemanticReference, SemanticSymbol} from '../../incremental/semantic_graph'; import {IndexingContext} from '../../indexer'; @@ -628,11 +628,14 @@ export class ComponentDecoratorHandler implements const bound = binder.bind({template: metadata.template.nodes}); // The BoundTarget knows which directives and pipes matched the template. - type UsedDirective = R3UsedDirectiveMetadata&{ref: Reference}; + type UsedDirective = + R3UsedDirectiveMetadata&{ref: Reference, importedFile: ImportedFile}; const usedDirectives: UsedDirective[] = bound.getUsedDirectives().map(directive => { + const type = this.refEmitter.emit(directive.ref, context); return { ref: directive.ref, - type: this.refEmitter.emit(directive.ref, context), + type: type.expression, + importedFile: type.importedFile, selector: directive.selector, inputs: directive.inputs.propertyNames, outputs: directive.outputs.propertyNames, @@ -640,17 +643,25 @@ export class ComponentDecoratorHandler implements isComponent: directive.isComponent, }; }); - type UsedPipe = {ref: Reference, pipeName: string, expression: Expression}; + + type UsedPipe = { + ref: Reference, + pipeName: string, + expression: Expression, + importedFile: ImportedFile, + }; const usedPipes: UsedPipe[] = []; for (const pipeName of bound.getUsedPipes()) { if (!pipes.has(pipeName)) { continue; } const pipe = pipes.get(pipeName)!; + const type = this.refEmitter.emit(pipe, context); usedPipes.push({ ref: pipe, pipeName, - expression: this.refEmitter.emit(pipe, context), + expression: type.expression, + importedFile: type.importedFile, }); } if (this.semanticDepGraphUpdater !== null) { @@ -665,14 +676,16 @@ export class ComponentDecoratorHandler implements // import which needs to be generated would create a cycle. const cyclesFromDirectives = new Map(); for (const usedDirective of usedDirectives) { - const cycle = this._checkForCyclicImport(usedDirective.ref, usedDirective.type, context); + const cycle = + this._checkForCyclicImport(usedDirective.importedFile, usedDirective.type, context); if (cycle !== null) { cyclesFromDirectives.set(usedDirective, cycle); } } const cyclesFromPipes = new Map(); for (const usedPipe of usedPipes) { - const cycle = this._checkForCyclicImport(usedPipe.ref, usedPipe.expression, context); + const cycle = + this._checkForCyclicImport(usedPipe.importedFile, usedPipe.expression, context); if (cycle !== null) { cyclesFromPipes.set(usedPipe, cycle); } @@ -682,11 +695,11 @@ export class ComponentDecoratorHandler implements if (!cycleDetected) { // No cycle was detected. Record the imports that need to be created in the cycle detector // so that future cyclic import checks consider their production. - for (const {type} of usedDirectives) { - this._recordSyntheticImport(type, context); + for (const {type, importedFile} of usedDirectives) { + this._recordSyntheticImport(importedFile, type, context); } - for (const {expression} of usedPipes) { - this._recordSyntheticImport(expression, context); + for (const {expression, importedFile} of usedPipes) { + this._recordSyntheticImport(importedFile, expression, context); } // Check whether the directive/pipe arrays in ɵcmp need to be wrapped in closures. @@ -1189,7 +1202,17 @@ export class ComponentDecoratorHandler implements } } - private _expressionToImportedFile(expr: Expression, origin: ts.SourceFile): ts.SourceFile|null { + private _resolveImportedFile(importedFile: ImportedFile, expr: Expression, origin: ts.SourceFile): + ts.SourceFile|null { + // If `importedFile` is not 'unknown' then it accurately reflects the source file that is + // being imported. + if (importedFile !== 'unknown') { + return importedFile; + } + + // Otherwise `expr` has to be inspected to determine the file that is being imported. If `expr` + // is not an `ExternalExpr` then it does not correspond with an import, so return null in that + // case. if (!(expr instanceof ExternalExpr)) { return null; } @@ -1204,18 +1227,19 @@ export class ComponentDecoratorHandler implements * * @returns a `Cycle` object if a cycle would be created, otherwise `null`. */ - private _checkForCyclicImport(ref: Reference, expr: Expression, origin: ts.SourceFile): Cycle - |null { - const importedFile = this._expressionToImportedFile(expr, origin); - if (importedFile === null) { + private _checkForCyclicImport( + importedFile: ImportedFile, expr: Expression, origin: ts.SourceFile): Cycle|null { + const imported = this._resolveImportedFile(importedFile, expr, origin); + if (imported === null) { return null; } // Check whether the import is legal. - return this.cycleAnalyzer.wouldCreateCycle(origin, importedFile); + return this.cycleAnalyzer.wouldCreateCycle(origin, imported); } - private _recordSyntheticImport(expr: Expression, origin: ts.SourceFile): void { - const imported = this._expressionToImportedFile(expr, origin); + private _recordSyntheticImport( + importedFile: ImportedFile, expr: Expression, origin: ts.SourceFile): void { + const imported = this._resolveImportedFile(importedFile, expr, origin); if (imported === null) { return; } 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 f27984945f..76e3cc5411 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/ng_module.ts @@ -418,7 +418,7 @@ export class NgModuleDecoratorHandler implements const context = getSourceFile(node); for (const exportRef of analysis.exports) { if (isNgModule(exportRef.node, scope.compilation)) { - data.injectorImports.push(this.refEmitter.emit(exportRef, context)); + data.injectorImports.push(this.refEmitter.emit(exportRef, context).expression); } } @@ -466,12 +466,12 @@ export class NgModuleDecoratorHandler implements for (const decl of analysis.declarations) { const remoteScope = this.scopeRegistry.getRemoteScope(decl.node); if (remoteScope !== null) { - const directives = - remoteScope.directives.map(directive => this.refEmitter.emit(directive, context)); - const pipes = remoteScope.pipes.map(pipe => this.refEmitter.emit(pipe, context)); + const directives = remoteScope.directives.map( + directive => this.refEmitter.emit(directive, context).expression); + const pipes = remoteScope.pipes.map(pipe => this.refEmitter.emit(pipe, context).expression); const directiveArray = new LiteralArrayExpr(directives); const pipesArray = new LiteralArrayExpr(pipes); - const declExpr = this.refEmitter.emit(decl, context)!; + const declExpr = this.refEmitter.emit(decl, context).expression; const setComponentScope = new ExternalExpr(R3Identifiers.setComponentScope); const callExpr = new InvokeFunctionExpr(setComponentScope, [declExpr, directiveArray, pipesArray]); diff --git a/packages/compiler-cli/src/ngtsc/annotations/src/util.ts b/packages/compiler-cli/src/ngtsc/annotations/src/util.ts index 073fa02897..47e787a609 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/src/util.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/src/util.ts @@ -267,13 +267,12 @@ function createUnsuitableInjectionTokenError( export function toR3Reference( valueRef: Reference, typeRef: Reference, valueContext: ts.SourceFile, typeContext: ts.SourceFile, refEmitter: ReferenceEmitter): R3Reference { - const value = refEmitter.emit(valueRef, valueContext); - const type = refEmitter.emit( - typeRef, typeContext, ImportFlags.ForceNewImport | ImportFlags.AllowTypeImports); - if (value === null || type === null) { - throw new Error(`Could not refer to ${ts.SyntaxKind[valueRef.node.kind]}`); - } - return {value, type}; + return { + value: refEmitter.emit(valueRef, valueContext).expression, + type: refEmitter + .emit(typeRef, typeContext, ImportFlags.ForceNewImport | ImportFlags.AllowTypeImports) + .expression, + }; } export function isAngularCore(decorator: Decorator): decorator is Decorator&{import: Import} { diff --git a/packages/compiler-cli/src/ngtsc/imports/index.ts b/packages/compiler-cli/src/ngtsc/imports/index.ts index 6ee65f03af..8843ad1824 100644 --- a/packages/compiler-cli/src/ngtsc/imports/index.ts +++ b/packages/compiler-cli/src/ngtsc/imports/index.ts @@ -9,7 +9,7 @@ export {AliasingHost, AliasStrategy, PrivateExportAliasingHost, UnifiedModulesAliasingHost} from './src/alias'; export {ImportRewriter, NoopImportRewriter, R3SymbolsImportRewriter, validateAndRewriteCoreSymbol} from './src/core'; export {DefaultImportRecorder, DefaultImportTracker, NOOP_DEFAULT_IMPORT_RECORDER} from './src/default'; -export {AbsoluteModuleStrategy, ImportFlags, LocalIdentifierStrategy, LogicalProjectStrategy, ReferenceEmitStrategy, ReferenceEmitter, RelativePathStrategy, UnifiedModulesStrategy} from './src/emitter'; +export {AbsoluteModuleStrategy, EmittedReference, ImportedFile, ImportFlags, LocalIdentifierStrategy, LogicalProjectStrategy, ReferenceEmitStrategy, ReferenceEmitter, RelativePathStrategy, UnifiedModulesStrategy} from './src/emitter'; export {Reexport} from './src/reexport'; export {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 index 1eec3c2a84..7af8ffebf0 100644 --- a/packages/compiler-cli/src/ngtsc/imports/src/alias.ts +++ b/packages/compiler-cli/src/ngtsc/imports/src/alias.ts @@ -10,13 +10,12 @@ import {Expression, ExternalExpr} from '@angular/compiler'; import * as ts from 'typescript'; import {UnifiedModulesHost} from '../../core/api'; -import {ClassDeclaration, isNamedClassDeclaration, ReflectionHost} from '../../reflection'; +import {ClassDeclaration, ReflectionHost} from '../../reflection'; -import {ImportFlags, ReferenceEmitStrategy} from './emitter'; +import {EmittedReference, ImportFlags, ReferenceEmitStrategy} from './emitter'; import {Reference} from './references'; - // Escape anything that isn't alphanumeric, '/' or '_'. const CHARS_TO_ESCAPE = /[^a-zA-Z0-9/_]/g; @@ -213,11 +212,11 @@ export class PrivateExportAliasingHost implements AliasingHost { * directive or pipe, if it exists. */ export class AliasStrategy implements ReferenceEmitStrategy { - emit(ref: Reference, context: ts.SourceFile, importMode: ImportFlags): Expression|null { - if (importMode & ImportFlags.NoAliasing) { + emit(ref: Reference, context: ts.SourceFile, importMode: ImportFlags): EmittedReference|null { + if (importMode & ImportFlags.NoAliasing || ref.alias === null) { return null; } - return ref.alias; + return {expression: ref.alias, importedFile: 'unknown'}; } } diff --git a/packages/compiler-cli/src/ngtsc/imports/src/emitter.ts b/packages/compiler-cli/src/ngtsc/imports/src/emitter.ts index aa77bb1884..f21dca0ce1 100644 --- a/packages/compiler-cli/src/ngtsc/imports/src/emitter.ts +++ b/packages/compiler-cli/src/ngtsc/imports/src/emitter.ts @@ -11,7 +11,7 @@ import * as ts from 'typescript'; import {UnifiedModulesHost} from '../../core/api'; import {absoluteFromSourceFile, dirname, LogicalFileSystem, LogicalProjectPath, relative, toRelativeImport} from '../../file_system'; import {stripExtension} from '../../file_system/src/util'; -import {DeclarationNode, isConcreteDeclaration, ReflectionHost} from '../../reflection'; +import {DeclarationNode, ReflectionHost} from '../../reflection'; import {getSourceFile, isDeclaration, isTypeDeclaration, nodeNameForError} from '../../util/src/typescript'; import {findExportedNameOfNode} from './find_export'; @@ -51,6 +51,36 @@ export enum ImportFlags { AllowTypeImports = 0x04, } +/** + * An emitter strategy has the ability to indicate which `ts.SourceFile` is being imported by the + * expression that it has generated. This information is useful for consumers of the emitted + * reference that would otherwise have to perform a relatively expensive module resolution step, + * e.g. for cyclic import analysis. In cases the emitter is unable to definitively determine the + * imported source file or a computation would be required to actually determine the imported + * source file, then `'unknown'` should be returned. If the generated expression does not represent + * an import then `null` should be used. + */ +export type ImportedFile = ts.SourceFile|'unknown'|null; + +/** + * Represents the emitted expression of a `Reference` that is valid in the source file it was + * emitted from. + */ +export interface EmittedReference { + /** + * The expression that refers to `Reference`. + */ + expression: Expression; + + /** + * The `ts.SourceFile` that is imported by `expression`. This is not necessarily the source file + * of the `Reference`'s declaration node, as the reference may have been rewritten through an + * alias export. It could also be `null` if `expression` is a local identifier, or `'unknown'` if + * the exact source file that is being imported is not known to the emitter. + */ + importedFile: ImportedFile; +} + /** * A particular strategy for generating an expression which refers to a `Reference`. * @@ -71,9 +101,10 @@ export interface ReferenceEmitStrategy { * @param ref the `Reference` for which to generate an expression * @param context the source file in which the `Expression` must be valid * @param importFlags a flag which controls whether imports should be generated or not - * @returns an `Expression` which refers to the `Reference`, or `null` if none can be generated + * @returns an `EmittedReference` which refers to the `Reference`, or `null` if none can be + * generated */ - emit(ref: Reference, context: ts.SourceFile, importFlags: ImportFlags): Expression|null; + emit(ref: Reference, context: ts.SourceFile, importFlags: ImportFlags): EmittedReference|null; } /** @@ -85,8 +116,8 @@ export interface ReferenceEmitStrategy { export class ReferenceEmitter { constructor(private strategies: ReferenceEmitStrategy[]) {} - emit(ref: Reference, context: ts.SourceFile, importFlags: ImportFlags = ImportFlags.None): - Expression { + emit(ref: Reference, context: ts.SourceFile, importFlags: ImportFlags = ImportFlags.None): + EmittedReference { for (const strategy of this.strategies) { const emitted = strategy.emit(ref, context, importFlags); if (emitted !== null) { @@ -103,7 +134,7 @@ export class ReferenceEmitter { * such identifiers are available. */ export class LocalIdentifierStrategy implements ReferenceEmitStrategy { - emit(ref: Reference, context: ts.SourceFile, importFlags: ImportFlags): Expression|null { + emit(ref: Reference, context: ts.SourceFile, importFlags: ImportFlags): EmittedReference|null { const refSf = getSourceFile(ref.node); // If the emitter has specified ForceNewImport, then LocalIdentifierStrategy should not use a @@ -118,20 +149,41 @@ export class LocalIdentifierStrategy implements ReferenceEmitStrategy { // such a case, the reference's `identities` property would be `[foo]`, which would result in an // invalid emission of a free-standing `foo` identifier, rather than `exports.foo`. if (!isDeclaration(ref.node) && refSf === context) { - return new WrappedNodeExpr(ref.node); + return { + expression: new WrappedNodeExpr(ref.node), + importedFile: null, + }; } // A Reference can have multiple identities in different files, so it may already have an // Identifier in the requested context file. const identifier = ref.getIdentityIn(context); if (identifier !== null) { - return new WrappedNodeExpr(identifier); + return { + expression: new WrappedNodeExpr(identifier), + importedFile: null, + }; } else { return null; } } } +/** + * Represents the exported declarations from a module source file. + */ +interface ModuleExports { + /** + * The source file of the module. + */ + module: ts.SourceFile; + + /** + * The map of declarations to their exported name. + */ + exportMap: Map; +} + /** * A `ReferenceEmitStrategy` which will refer to declarations that come from `node_modules` using * an absolute import. @@ -146,13 +198,13 @@ export class AbsoluteModuleStrategy implements ReferenceEmitStrategy { * A cache of the exports of specific modules, because resolving a module to its exports is a * costly operation. */ - private moduleExportsCache = new Map|null>(); + private moduleExportsCache = new Map(); constructor( protected program: ts.Program, protected checker: ts.TypeChecker, protected moduleResolver: ModuleResolver, private reflectionHost: ReflectionHost) {} - emit(ref: Reference, context: ts.SourceFile, importFlags: ImportFlags): Expression|null { + emit(ref: Reference, context: ts.SourceFile, importFlags: ImportFlags): EmittedReference|null { if (ref.bestGuessOwningModule === null) { // There is no module name available for this Reference, meaning it was arrived at via a // relative path. @@ -168,38 +220,30 @@ export class AbsoluteModuleStrategy implements ReferenceEmitStrategy { // Try to find the exported name of the declaration, if one is available. const {specifier, resolutionContext} = ref.bestGuessOwningModule; - const symbolName = this.resolveImportName(specifier, ref.node, resolutionContext); - if (symbolName === null) { + const exports = this.getExportsOfModule(specifier, resolutionContext); + if (exports === null || !exports.exportMap.has(ref.node)) { // TODO(alxhub): make this error a ts.Diagnostic pointing at whatever caused this import to be // triggered. throw new Error(`Symbol ${ref.debugName} declared in ${ getSourceFile(ref.node).fileName} is not exported from ${specifier} (import into ${ context.fileName})`); } + const symbolName = exports.exportMap.get(ref.node)!; - return new ExternalExpr(new ExternalReference(specifier, symbolName)); + return { + expression: new ExternalExpr(new ExternalReference(specifier, symbolName)), + importedFile: exports.module, + }; } - private resolveImportName(moduleName: string, target: DeclarationNode, fromFile: string): string - |null { - const exports = this.getExportsOfModule(moduleName, fromFile); - if (exports !== null && exports.has(target)) { - return exports.get(target)!; - } else { - return null; - } - } - - private getExportsOfModule(moduleName: string, fromFile: string): - Map|null { + private getExportsOfModule(moduleName: string, fromFile: string): ModuleExports|null { if (!this.moduleExportsCache.has(moduleName)) { this.moduleExportsCache.set(moduleName, this.enumerateExportsOfModule(moduleName, fromFile)); } return this.moduleExportsCache.get(moduleName)!; } - protected enumerateExportsOfModule(specifier: string, fromFile: string): - Map|null { + protected enumerateExportsOfModule(specifier: string, fromFile: string): ModuleExports|null { // First, resolve the module specifier to its entry point, and get the ts.Symbol for it. const entryPointFile = this.moduleResolver.resolveModule(specifier, fromFile); if (entryPointFile === null) { @@ -214,7 +258,7 @@ export class AbsoluteModuleStrategy implements ReferenceEmitStrategy { exports.forEach((declaration, name) => { exportMap.set(declaration.node, name); }); - return exportMap; + return {module: entryPointFile, exportMap}; } } @@ -229,7 +273,7 @@ export class AbsoluteModuleStrategy implements ReferenceEmitStrategy { export class LogicalProjectStrategy implements ReferenceEmitStrategy { constructor(private reflector: ReflectionHost, private logicalFs: LogicalFileSystem) {} - emit(ref: Reference, context: ts.SourceFile): Expression|null { + emit(ref: Reference, context: ts.SourceFile): EmittedReference|null { const destSf = getSourceFile(ref.node); // Compute the relative path from the importing file to the file being imported. This is done @@ -260,7 +304,10 @@ export class LogicalProjectStrategy implements ReferenceEmitStrategy { // With both files expressed as LogicalProjectPaths, getting the module specifier as a relative // path is now straightforward. const moduleName = LogicalProjectPath.relativePathBetween(originPath, destPath); - return new ExternalExpr({moduleName, name}); + return { + expression: new ExternalExpr({moduleName, name}), + importedFile: destSf, + }; } } @@ -273,14 +320,14 @@ export class LogicalProjectStrategy implements ReferenceEmitStrategy { export class RelativePathStrategy implements ReferenceEmitStrategy { constructor(private reflector: ReflectionHost) {} - emit(ref: Reference, context: ts.SourceFile): Expression|null { + emit(ref: Reference, context: ts.SourceFile): EmittedReference|null { const destSf = getSourceFile(ref.node); const relativePath = relative(dirname(absoluteFromSourceFile(context)), absoluteFromSourceFile(destSf)); const moduleName = toRelativeImport(stripExtension(relativePath)); const name = findExportedNameOfNode(ref.node, destSf, this.reflector); - return new ExternalExpr({moduleName, name}); + return {expression: new ExternalExpr({moduleName, name}), importedFile: destSf}; } } @@ -291,7 +338,7 @@ export class RelativePathStrategy implements ReferenceEmitStrategy { export class UnifiedModulesStrategy implements ReferenceEmitStrategy { constructor(private reflector: ReflectionHost, private unifiedModulesHost: UnifiedModulesHost) {} - emit(ref: Reference, context: ts.SourceFile): Expression|null { + emit(ref: Reference, context: ts.SourceFile): EmittedReference|null { const destSf = getSourceFile(ref.node); const name = findExportedNameOfNode(ref.node, destSf, this.reflector); if (name === null) { @@ -301,6 +348,9 @@ export class UnifiedModulesStrategy implements ReferenceEmitStrategy { const moduleName = this.unifiedModulesHost.fileNameToModuleName(destSf.fileName, context.fileName); - return new ExternalExpr({moduleName, name}); + return { + expression: new ExternalExpr({moduleName, name}), + importedFile: destSf, + }; } } diff --git a/packages/compiler-cli/src/ngtsc/imports/test/emitter_spec.ts b/packages/compiler-cli/src/ngtsc/imports/test/emitter_spec.ts index 79fbf5c986..c556a1cf72 100644 --- a/packages/compiler-cli/src/ngtsc/imports/test/emitter_spec.ts +++ b/packages/compiler-cli/src/ngtsc/imports/test/emitter_spec.ts @@ -72,11 +72,14 @@ runInEachFileSystem(() => { resolutionContext: context.fileName, }); const emitted = strategy.emit(reference, context, ImportFlags.None); - if (!(emitted instanceof ExternalExpr)) { + if (emitted === null) { + return fail('Reference should be emitted'); + } + if (!(emitted.expression instanceof ExternalExpr)) { return fail('Reference should be emitted as ExternalExpr'); } - expect(emitted.value.name).toEqual('Bar'); - expect(emitted.value.moduleName).toEqual('external'); + expect(emitted.expression.value.name).toEqual('Bar'); + expect(emitted.expression.value.moduleName).toEqual('external'); }); it('should throw when generating an import to a type-only declaration when not allowed', () => { @@ -121,11 +124,14 @@ runInEachFileSystem(() => { const reference = new Reference(decl, {specifier: 'external', resolutionContext: context.fileName}); const emitted = strategy.emit(reference, context, ImportFlags.AllowTypeImports); - if (!(emitted instanceof ExternalExpr)) { + if (emitted === null) { + return fail('Reference should be emitted'); + } + if (!(emitted.expression instanceof ExternalExpr)) { return fail('Reference should be emitted as ExternalExpr'); } - expect(emitted.value.name).toEqual('Foo'); - expect(emitted.value.moduleName).toEqual('external'); + expect(emitted.expression.value.name).toEqual('Foo'); + expect(emitted.expression.value.moduleName).toEqual('external'); }); }); @@ -165,7 +171,7 @@ runInEachFileSystem(() => { expect(ref).not.toBeNull(); // Expect the prefixed name from the TestHost. - expect((ref! as ExternalExpr).value.name).toEqual('testFoo'); + expect((ref!.expression as ExternalExpr).value.name).toEqual('testFoo'); }); }); }); diff --git a/packages/compiler-cli/src/ngtsc/modulewithproviders/src/scanner.ts b/packages/compiler-cli/src/ngtsc/modulewithproviders/src/scanner.ts index f5635550e3..573dd20a2d 100644 --- a/packages/compiler-cli/src/ngtsc/modulewithproviders/src/scanner.ts +++ b/packages/compiler-cli/src/ngtsc/modulewithproviders/src/scanner.ts @@ -101,7 +101,7 @@ export class ModuleWithProvidersScanner { const ngModuleExpr = this.emitter.emit(ngModule, decl.getSourceFile(), ImportFlags.ForceNewImport); - const ngModuleType = new ExpressionType(ngModuleExpr); + const ngModuleType = new ExpressionType(ngModuleExpr.expression); const mwpNgType = new ExpressionType( new ExternalExpr(Identifiers.ModuleWithProviders), [/* modifiers */], [ngModuleType]); diff --git a/packages/compiler-cli/src/ngtsc/scope/src/local.ts b/packages/compiler-cli/src/ngtsc/scope/src/local.ts index 5badea3f24..5e509324d1 100644 --- a/packages/compiler-cli/src/ngtsc/scope/src/local.ts +++ b/packages/compiler-cli/src/ngtsc/scope/src/local.ts @@ -511,7 +511,8 @@ export class LocalModuleScopeRegistry implements MetadataRegistry, ComponentScop asAlias: exportName, }); } else { - const expr = this.refEmitter.emit(exportRef.cloneWithNoIdentifiers(), sourceFile); + const expr = + this.refEmitter.emit(exportRef.cloneWithNoIdentifiers(), sourceFile).expression; if (!(expr instanceof ExternalExpr) || expr.value.moduleName === null || expr.value.name === null) { throw new Error('Expected ExternalExpr'); diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/environment.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/environment.ts index 73d589ecc7..4f5dddb68c 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/environment.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/environment.ts @@ -123,7 +123,7 @@ export class Environment { const ngExpr = this.refEmitter.emit(ref, this.contextFile, ImportFlags.NoAliasing); // Use `translateExpression` to convert the `Expression` into a `ts.Expression`. - return translateExpression(ngExpr, this.importManager); + return translateExpression(ngExpr.expression, this.importManager); } /** @@ -137,7 +137,7 @@ export class Environment { // Create an `ExpressionType` from the `Expression` and translate it via `translateType`. // TODO(alxhub): support references to types with generic arguments in a clean way. - return translateType(new ExpressionType(ngExpr), this.importManager); + return translateType(new ExpressionType(ngExpr.expression), this.importManager); } private emitTypeParameters(declaration: ClassDeclaration):