diff --git a/packages/compiler-cli/src/ngtsc/metadata/src/dts.ts b/packages/compiler-cli/src/ngtsc/metadata/src/dts.ts index 497d2e900f..2ea11b0c8e 100644 --- a/packages/compiler-cli/src/ngtsc/metadata/src/dts.ts +++ b/packages/compiler-cli/src/ngtsc/metadata/src/dts.ts @@ -30,7 +30,7 @@ export class DtsMetadataReader implements MetadataReader { */ getNgModuleMetadata(ref: Reference): NgModuleMeta|null { const clazz = ref.node; - const resolutionContext = clazz.getSourceFile().fileName; + // This operation is explicitly not memoized, as it depends on `ref.ownedByModuleGuess`. // TODO(alxhub): investigate caching of .d.ts module metadata. const ngModuleDef = this.reflector.getMembersOfClass(clazz).find( @@ -49,12 +49,10 @@ export class DtsMetadataReader implements MetadataReader { const [_, declarationMetadata, importMetadata, exportMetadata] = ngModuleDef.type.typeArguments; return { ref, - declarations: extractReferencesFromType( - this.checker, declarationMetadata, ref.ownedByModuleGuess, resolutionContext), - exports: extractReferencesFromType( - this.checker, exportMetadata, ref.ownedByModuleGuess, resolutionContext), - imports: extractReferencesFromType( - this.checker, importMetadata, ref.ownedByModuleGuess, resolutionContext), + declarations: + extractReferencesFromType(this.checker, declarationMetadata, ref.bestGuessOwningModule), + exports: extractReferencesFromType(this.checker, exportMetadata, ref.bestGuessOwningModule), + imports: extractReferencesFromType(this.checker, importMetadata, ref.bestGuessOwningModule), schemas: [], rawDeclarations: null, }; diff --git a/packages/compiler-cli/src/ngtsc/metadata/src/util.ts b/packages/compiler-cli/src/ngtsc/metadata/src/util.ts index a27a1b2654..597417440f 100644 --- a/packages/compiler-cli/src/ngtsc/metadata/src/util.ts +++ b/packages/compiler-cli/src/ngtsc/metadata/src/util.ts @@ -8,7 +8,7 @@ import * as ts from 'typescript'; -import {Reference} from '../../imports'; +import {OwningModule, Reference} from '../../imports'; import {ClassDeclaration, ClassMember, ClassMemberKind, isNamedClassDeclaration, ReflectionHost, reflectTypeEntityToDeclaration} from '../../reflection'; import {nodeDebugInfo} from '../../util/src/typescript'; @@ -16,8 +16,8 @@ import {DirectiveMeta, DirectiveTypeCheckMeta, MetadataReader, NgModuleMeta, Pip import {ClassPropertyMapping, ClassPropertyName} from './property_mapping'; export function extractReferencesFromType( - checker: ts.TypeChecker, def: ts.TypeNode, ngModuleImportedFrom: string|null, - resolutionContext: string): Reference[] { + checker: ts.TypeChecker, def: ts.TypeNode, + bestGuessOwningModule: OwningModule|null): Reference[] { if (!ts.isTupleTypeNode(def)) { return []; } @@ -31,11 +31,15 @@ export function extractReferencesFromType( if (!isNamedClassDeclaration(node)) { throw new Error(`Expected named ClassDeclaration: ${nodeDebugInfo(node)}`); } - const specifier = (from !== null && !from.startsWith('.') ? from : ngModuleImportedFrom); - if (specifier !== null) { - return new Reference(node, {specifier, resolutionContext}); + if (from !== null && !from.startsWith('.')) { + // The symbol was imported using an absolute module specifier so return a reference that + // uses that absolute module specifier as its best guess owning module. + return new Reference( + node, {specifier: from, resolutionContext: def.getSourceFile().fileName}); } else { - return new Reference(node); + // For local symbols or symbols that were imported using a relative module import it is + // assumed that the symbol is exported from the provided best guess owning module. + return new Reference(node, bestGuessOwningModule); } }); } diff --git a/packages/compiler-cli/src/ngtsc/metadata/test/dts_spec.ts b/packages/compiler-cli/src/ngtsc/metadata/test/dts_spec.ts index c34de79b16..f3acf1e479 100644 --- a/packages/compiler-cli/src/ngtsc/metadata/test/dts_spec.ts +++ b/packages/compiler-cli/src/ngtsc/metadata/test/dts_spec.ts @@ -10,7 +10,7 @@ import * as ts from 'typescript'; import {absoluteFrom, getFileSystem, getSourceFileOrError} from '../../file_system'; import {runInEachFileSystem} from '../../file_system/testing'; -import {Reference} from '../../imports'; +import {OwningModule, Reference} from '../../imports'; import {isNamedClassDeclaration, TypeScriptReflectionHost} from '../../reflection'; import {loadFakeCore, makeProgram} from '../../testing'; @@ -89,5 +89,93 @@ runInEachFileSystem(() => { const meta = dtsReader.getDirectiveMetadata(new Reference(clazz))!; expect(meta.isStructural).toBeTrue(); }); + + it('should retain an absolute owning module for relative imports', () => { + const externalPath = absoluteFrom('/external.d.ts'); + const {program} = makeProgram( + [ + { + name: externalPath, + contents: ` + import * as i0 from '@angular/core'; + import * as i1 from 'absolute'; + import * as i2 from './relative'; + + export declare class ExternalModule { + static ɵmod: i0.ɵɵNgModuleDeclaration; + } + ` + }, + { + name: absoluteFrom('/relative.d.ts'), + contents: ` + import * as i0 from '@angular/core'; + + export declare class RelativeDir { + static ɵdir: i0.ɵɵDirectiveDeclaration; + } + ` + }, + { + name: absoluteFrom('/node_modules/absolute.d.ts'), + contents: ` + import * as i0 from '@angular/core'; + + export declare class AbsoluteDir { + static ɵdir: i0.ɵɵDirectiveDeclaration; + } + ` + } + ], + { + skipLibCheck: true, + lib: ['es6', 'dom'], + }); + + const externalSf = getSourceFileOrError(program, externalPath); + const clazz = externalSf.statements[3]; + if (!isNamedClassDeclaration(clazz)) { + return fail('Expected class declaration'); + } + + const typeChecker = program.getTypeChecker(); + const dtsReader = + new DtsMetadataReader(typeChecker, new TypeScriptReflectionHost(typeChecker)); + + const withoutOwningModule = dtsReader.getNgModuleMetadata(new Reference(clazz))!; + expect(withoutOwningModule.exports.length).toBe(2); + + // `AbsoluteDir` was imported from an absolute module so the export Reference should have + // a corresponding best guess owning module. + expect(withoutOwningModule.exports[0].bestGuessOwningModule).toEqual({ + specifier: 'absolute', + resolutionContext: externalSf.fileName, + }); + + // `RelativeDir` was imported from a relative module specifier so the original reference's + // best guess owning module should have been retained, which was null. + expect(withoutOwningModule.exports[1].bestGuessOwningModule).toBeNull(); + + const owningModule: OwningModule = { + specifier: 'module', + resolutionContext: absoluteFrom('/context.ts'), + }; + const withOwningModule = dtsReader.getNgModuleMetadata(new Reference(clazz, owningModule))!; + expect(withOwningModule.exports.length).toBe(2); + + // Again, `AbsoluteDir` was imported from an absolute module so the export Reference should + // have a corresponding best guess owning module; the owning module of the incoming reference + // is irrelevant here. + expect(withOwningModule.exports[0].bestGuessOwningModule).toEqual({ + specifier: 'absolute', + resolutionContext: externalSf.fileName, + }); + + // As `RelativeDir` was imported from a relative module specifier, the export Reference should + // continue to have the owning module of the incoming reference as the relatively imported + // symbol is assumed to also be exported from the absolute module specifier as captured in the + // best guess owning module. + expect(withOwningModule.exports[1].bestGuessOwningModule).toEqual(owningModule); + }); }); }); 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 861348c039..8298910a4d 100644 --- a/packages/compiler-cli/src/ngtsc/scope/test/dependency_spec.ts +++ b/packages/compiler-cli/src/ngtsc/scope/test/dependency_spec.ts @@ -68,8 +68,7 @@ function makeTestEnv( const exportedSymbol = symbol.exports!.get(name as ts.__String); if (exportedSymbol !== undefined) { const decl = exportedSymbol.valueDeclaration as ts.ClassDeclaration; - const specifier = MODULE_FROM_NODE_MODULES_PATH.exec(sf.fileName)![1]; - return new Reference(decl, {specifier, resolutionContext: sf.fileName}); + return new Reference(decl); } } throw new Error('Class not found: ' + name); @@ -143,10 +142,13 @@ runInEachFileSystem(() => { }); const {Dir, ModuleB} = refs; const scope = resolver.resolve(ModuleB)!; - expect(scopeToRefs(scope)).toEqual([Dir]); + expect(scopeToRefs(scope).map(ref => ref.node)).toEqual([Dir.node]); // Explicitly verify that the directive has the correct owning module. - expect(scope.exported.directives[0].ref.ownedByModuleGuess).toBe('declaration'); + expect(scope.exported.directives[0].ref.bestGuessOwningModule).toEqual({ + specifier: 'declaration', + resolutionContext: ModuleB.node.getSourceFile().fileName, + }); }); it('should write correct aliases for deep dependencies', () => {