iSharkFly-Docs
/
angular-docs-cn
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(compiler-cli): use correct module resolution context for absolute imports in .d.ts files (#42879) 

The compiler keeps track of how a declaration has been referenced
using absolute module imports and from which path the absolute module
should be resolved from. There was a bug in how the .d.ts metadata
extraction would incorrectly use the .d.ts file itself as resolution
context for symbols that had been imported using a relative module
specifier. This could result in module resolution failures.

For example, when extracting NgModule metadata from
`/node_modules/lib/index.d.ts` that looks like

```
import {LibDirective} from './dir';

@NgModule({
  declarations: [LibDirective],
  exports: [LibDirective],
})
export class LibModule {}
```

and `/app.module.ts` that contains

```
import {LibModule} from 'lib';

@NgModule({
  imports: [LibModule],
})
export class AppModule {}
```

then `AppModule` would have recorded a reference to `LibModule` using
the `'lib'` module specifier. When extracting the NgModule metadata from
the `/node_modules/lib/index.d.ts` file the relative import into `./dir`
should also be assumed to be importable from `'lib'` (according to APF
where symbols need to be exported from a single entry-point)
so the reference to `LibDirective` should have `'lib'` as absolute
module specifier, but it would incorrectly have
`/node_modules/lib/index.d.ts` as resolution context path. The latter is
incorrect as `'lib'` needs to be resolved from `/app.module.ts` and not
from within the library itself.

Fixes #42810

PR Close #42879
This commit is contained in:
JoostK 2021-07-16 23:58:50 +02:00 committed by Dylan Hunn
parent dbae00195e
commit ed9cfb674f
4 changed files with 111 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
packages/compiler-cli/src/ngtsc/metadata/src/dts.ts
View File

@ -30,7 +30,7 @@ export class DtsMetadataReader implements MetadataReader {
*/
getNgModuleMetadata(ref: Reference<ClassDeclaration>): 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,
};

18
packages/compiler-cli/src/ngtsc/metadata/src/util.ts
View File

@ -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<ClassDeclaration>[] {
checker: ts.TypeChecker, def: ts.TypeNode,
bestGuessOwningModule: OwningModule|null): Reference<ClassDeclaration>[] {
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);
}
});
}

90
packages/compiler-cli/src/ngtsc/metadata/test/dts_spec.ts
View File

@ -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<RelativeModule, [typeof i2.RelativeDir], never, [typeof i1.AbsoluteDir, typeof i2.RelativeDir]>;
}
`
},
{
name: absoluteFrom('/relative.d.ts'),
contents: `
import * as i0 from '@angular/core';
export declare class RelativeDir {
static ɵdir: i0.ɵɵDirectiveDeclaration<RelativeDir, '[dir]', never, never, never, never>;
}
`
},
{
name: absoluteFrom('/node_modules/absolute.d.ts'),
contents: `
import * as i0 from '@angular/core';
export declare class AbsoluteDir {
static ɵdir: i0.ɵɵDirectiveDeclaration<ExternalDir, '[dir]', never, never, never, never>;
}
`
}
],
{
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);
});
});
});

10
packages/compiler-cli/src/ngtsc/scope/test/dependency_spec.ts
View File

@ -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', () => {