cargdev/angular
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat(ivy): enable re-export of the compilation scope of NgModules privately (#33177)

Browse Source 
This commit refactors the aliasing system to support multiple different
AliasingHost implementations, which control specific aliasing behavior
in ngtsc (see the README.md).

A new host is introduced, the `PrivateExportAliasingHost`. This solves a
longstanding problem in ngtsc regarding support for "monorepo" style private
libraries. These are libraries which are compiled separately from the main
application, and depended upon through TypeScript path mappings. Such
libraries are frequently not in the Angular Package Format and do not have
entrypoints, but rather make use of deep import style module specifiers.
This can cause issues with ngtsc's ability to import a directive given the
module specifier of its NgModule.

For example, if the application uses a directive `Foo` from such a library
`foo`, the user might write:

```typescript
import {FooModule} from 'foo/module';
```

In this case, foo/module.d.ts is path-mapped into the program. Ordinarily
the compiler would see this as an absolute module specifier, and assume that
the `Foo` directive can be imported from the same specifier. For such non-
APF libraries, this assumption fails. Really `Foo` should be imported from
the file which declares it, but there are two problems with this:

1. The compiler would have to reverse the path mapping in order to determine
   a path-mapped path to the file (maybe foo/dir.d.ts).
2. There is no guarantee that the file containing the directive is path-
   mapped in the program at all.

The compiler would effectively have to "guess" 'foo/dir' as a module
specifier, which may or may not be accurate depending on how the library and
path mapping are set up.

It's strongly desirable that the compiler not break its current invariant
that the module specifier given by the user for the NgModule is always the
module specifier from which directives/pipes are imported. Thus, for any
given NgModule from a particular module specifier, it must always be
possible to import any directives/pipes from the same specifier, no matter
how it's packaged.

To make this possible, when compiling a file containing an NgModule, ngtsc
will automatically add re-exports for any directives/pipes not yet exported
by the user, with a name of the form: ɵngExportɵModuleNameɵDirectiveName

This has several effects:

1. It guarantees anyone depending on the NgModule will be able to import its
   directives/pipes from the same specifier.
2. It maintains a stable name for the exported symbol that is safe to depend
   on from code on NPM. Effectively, this private exported name will be a
   part of the package's .d.ts API, and cannot be changed in a non-breaking
   fashion.

Fixes #29361
FW-1610 #resolve

PR Close #33177
This commit is contained in:
Alex Rickabaugh
2019-10-14 12:03:29 -07:00
committed by Matias Niemelä
parent a86a179f45
commit c4733c15c0
13 changed files with 894 additions and 96 deletions
Download Patch File Download Diff File Expand all files Collapse all files

48
packages/compiler-cli/src/ngtsc/scope/src/dependency.ts
View File

@ -8,7 +8,7 @@
import * as ts from 'typescript';
import {AliasGenerator, Reference} from '../../imports';
import {AliasingHost, Reference} from '../../imports';
import {DirectiveMeta, MetadataReader, PipeMeta} from '../../metadata';
import {ClassDeclaration} from '../../reflection';
@ -34,7 +34,7 @@ export class MetadataDtsModuleScopeResolver implements DtsModuleScopeResolver {
/**
* @param dtsMetaReader a `MetadataReader` which can read metadata from `.d.ts` files.
*/
constructor(private dtsMetaReader: MetadataReader, private aliasGenerator: AliasGenerator|null) {}
constructor(private dtsMetaReader: MetadataReader, private aliasingHost: AliasingHost|null) {}
/**
* Resolve a `Reference`'d NgModule from a .d.ts file and produce a transitive `ExportScope`
@ -76,22 +76,16 @@ export class MetadataDtsModuleScopeResolver implements DtsModuleScopeResolver {
// Attempt to process the export as a directive.
const directive = this.dtsMetaReader.getDirectiveMetadata(exportRef);
if (directive !== null) {
if (!declarations.has(exportRef.node)) {
directives.push(this.maybeAlias(directive, sourceFile));
} else {
directives.push(directive);
}
const isReExport = !declarations.has(exportRef.node);
directives.push(this.maybeAlias(directive, sourceFile, isReExport));
continue;
}
// Attempt to process the export as a pipe.
const pipe = this.dtsMetaReader.getPipeMetadata(exportRef);
if (pipe !== null) {
if (!declarations.has(exportRef.node)) {
pipes.push(this.maybeAlias(pipe, sourceFile));
} else {
pipes.push(pipe);
}
const isReExport = !declarations.has(exportRef.node);
pipes.push(this.maybeAlias(pipe, sourceFile, isReExport));
continue;
}
@ -101,7 +95,7 @@ export class MetadataDtsModuleScopeResolver implements DtsModuleScopeResolver {
// 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) {
if (this.aliasingHost === null) {
// Fast path when aliases aren't required.
directives.push(...exportScope.exported.directives);
pipes.push(...exportScope.exported.pipes);
@ -115,10 +109,10 @@ export class MetadataDtsModuleScopeResolver implements DtsModuleScopeResolver {
// 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));
directives.push(this.maybeAlias(directive, sourceFile, /* isReExport */ true));
}
for (const pipe of exportScope.exported.pipes) {
pipes.push(this.maybeAlias(pipe, sourceFile));
pipes.push(this.maybeAlias(pipe, sourceFile, /* isReExport */ true));
}
}
}
@ -134,19 +128,21 @@ export class MetadataDtsModuleScopeResolver implements DtsModuleScopeResolver {
};
}
private maybeAlias<T extends DirectiveMeta|PipeMeta>(dirOrPipe: T, maybeAliasFrom: ts.SourceFile):
T {
if (this.aliasGenerator === null) {
return dirOrPipe;
}
private maybeAlias<T extends DirectiveMeta|PipeMeta>(
dirOrPipe: T, maybeAliasFrom: ts.SourceFile, isReExport: boolean): T {
const ref = dirOrPipe.ref;
if (ref.node.getSourceFile() !== maybeAliasFrom) {
return {
...dirOrPipe,
ref: ref.cloneWithAlias(this.aliasGenerator.aliasTo(ref.node, maybeAliasFrom)),
};
} else {
if (this.aliasingHost === null || ref.node.getSourceFile() === maybeAliasFrom) {
return dirOrPipe;
}
const alias = this.aliasingHost.getAliasIn(ref.node, maybeAliasFrom, isReExport);
if (alias === null) {
return dirOrPipe;
}
return {
...dirOrPipe,
ref: ref.cloneWithAlias(alias),
};
}
}

121
packages/compiler-cli/src/ngtsc/scope/src/local.ts
View File

@ -10,7 +10,7 @@ import {ExternalExpr, SchemaMetadata} from '@angular/compiler';
import * as ts from 'typescript';
import {ErrorCode, makeDiagnostic} from '../../diagnostics';
import {AliasGenerator, Reexport, Reference, ReferenceEmitter} from '../../imports';
import {AliasingHost, Reexport, Reference, ReferenceEmitter} from '../../imports';
import {DirectiveMeta, MetadataReader, MetadataRegistry, NgModuleMeta, PipeMeta} from '../../metadata';
import {ClassDeclaration} from '../../reflection';
import {identifierOfNode, nodeNameForError} from '../../util/src/typescript';
@ -104,7 +104,7 @@ export class LocalModuleScopeRegistry implements MetadataRegistry, ComponentScop
constructor(
private localReader: MetadataReader, private dependencyScopeReader: DtsModuleScopeResolver,
private refEmitter: ReferenceEmitter, private aliasGenerator: AliasGenerator|null,
private refEmitter: ReferenceEmitter, private aliasingHost: AliasingHost|null,
private componentScopeRegistry: ComponentScopeRegistry = new NoopComponentScopeRegistry()) {}
/**
@ -217,7 +217,6 @@ export class LocalModuleScopeRegistry implements MetadataRegistry, ComponentScop
const compilationPipes = new Map<ts.Declaration, PipeMeta>();
const declared = new Set<ts.Declaration>();
const sourceFile = ref.node.getSourceFile();
// Directives and pipes exported to any importing NgModules.
const exportDirectives = new Map<ts.Declaration, DirectiveMeta>();
@ -321,39 +320,7 @@ export class LocalModuleScopeRegistry implements MetadataRegistry, ComponentScop
pipes: Array.from(exportPipes.values()),
};
let reexports: Reexport[]|null = null;
if (this.aliasGenerator !== null) {
reexports = [];
const addReexport = (ref: Reference<ClassDeclaration>) => {
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);
}
}
const reexports = this.getReexports(ngModule, ref, declared, exported, diagnostics);
// Check if this scope had any errors during production.
if (diagnostics.length > 0) {
@ -429,6 +396,66 @@ export class LocalModuleScopeRegistry implements MetadataRegistry, ComponentScop
}
}
private getReexports(
ngModule: NgModuleMeta, ref: Reference<ClassDeclaration>, declared: Set<ts.Declaration>,
exported: {directives: DirectiveMeta[], pipes: PipeMeta[]},
diagnostics: ts.Diagnostic[]): Reexport[]|null {
let reexports: Reexport[]|null = null;
const sourceFile = ref.node.getSourceFile();
if (this.aliasingHost === null) {
return null;
}
reexports = [];
// Track re-exports by symbol name, to produce diagnostics if two alias re-exports would share
// the same name.
const reexportMap = new Map<string, Reference<ClassDeclaration>>();
// Alias ngModuleRef added for readability below.
const ngModuleRef = ref;
const addReexport = (exportRef: Reference<ClassDeclaration>) => {
if (exportRef.node.getSourceFile() === sourceFile) {
return;
}
const isReExport = !declared.has(exportRef.node);
const exportName = this.aliasingHost !.maybeAliasSymbolAs(
exportRef, sourceFile, ngModule.ref.node.name.text, isReExport);
if (exportName === null) {
return;
}
if (!reexportMap.has(exportName)) {
if (exportRef.alias && exportRef.alias instanceof ExternalExpr) {
reexports !.push({
fromModule: exportRef.alias.value.moduleName !,
symbolName: exportRef.alias.value.name !,
asAlias: exportName,
});
} else {
const expr = this.refEmitter.emit(exportRef.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,
});
}
reexportMap.set(exportName, exportRef);
} else {
// Another re-export already used this name. Produce a diagnostic.
const prevRef = reexportMap.get(exportName) !;
diagnostics.push(reexportCollision(ngModuleRef.node, prevRef, exportRef));
}
};
for (const {ref} of exported.directives) {
addReexport(ref);
}
for (const {ref} of exported.pipes) {
addReexport(ref);
}
return reexports;
}
private assertCollecting(): void {
if (this.sealed) {
throw new Error(`Assertion: LocalModuleScopeRegistry is not COLLECTING`);
@ -472,3 +499,25 @@ function invalidReexport(clazz: ts.Declaration, decl: Reference<ts.Declaration>)
ErrorCode.NGMODULE_INVALID_REEXPORT, identifierOfNode(decl.node) || decl.node,
`Present in the NgModule.exports of ${nodeNameForError(clazz)} but neither declared nor imported`);
}
/**
* Produce a `ts.Diagnostic` for a collision in re-export names between two directives/pipes.
*/
function reexportCollision(
module: ClassDeclaration, refA: Reference<ClassDeclaration>,
refB: Reference<ClassDeclaration>): ts.Diagnostic {
const childMessageText =
`This directive/pipe is part of the exports of '${module.name.text}' and shares the same name as another exported directive/pipe.`;
return makeDiagnostic(
ErrorCode.NGMODULE_REEXPORT_NAME_COLLISION, module.name, `
There was a name collision between two classes named '${refA.node.name.text}', which are both part of the exports of '${module.name.text}'.
Angular generates re-exports of an NgModule's exported directives/pipes from the module's source file in certain cases, using the declared name of the class. If two classes of the same name are exported, this automatic naming does not work.
To fix this problem please re-export one or both classes directly from this file.
`.trim(),
[
{node: refA.node.name, messageText: childMessageText},
{node: refB.node.name, messageText: childMessageText},
]);
}

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

@ -7,9 +7,10 @@
*/
import {ExternalExpr, ExternalReference} from '@angular/compiler';
import * as ts from 'typescript';
import {absoluteFrom} from '../../file_system';
import {runInEachFileSystem} from '../../file_system/testing';
import {AliasGenerator, FileToModuleHost, Reference} from '../../imports';
import {AliasingHost, FileToModuleAliasingHost, FileToModuleHost, Reference} from '../../imports';
import {DtsMetadataReader} from '../../metadata';
import {ClassDeclaration, TypeScriptReflectionHost} from '../../reflection';
import {makeProgram} from '../../testing';
@ -42,7 +43,7 @@ export declare type PipeMeta<A, B> = never;
* destructured to retrieve references to specific declared classes.
*/
function makeTestEnv(
modules: {[module: string]: string}, aliasGenerator: AliasGenerator | null = null): {
modules: {[module: string]: string}, aliasGenerator: AliasingHost | null = null): {
refs: {[name: string]: Reference<ClassDeclaration>},
resolver: MetadataDtsModuleScopeResolver,
} {
@ -182,7 +183,7 @@ runInEachFileSystem(() => {
}
`,
},
new AliasGenerator(testHost));
new FileToModuleAliasingHost(testHost));
const {ShallowModule} = refs;
const scope = resolver.resolve(ShallowModule) !;
const [DeepDir, MiddleDir, ShallowDir] = scopeToRefs(scope);
@ -232,7 +233,7 @@ runInEachFileSystem(() => {
}
`,
},
new AliasGenerator(testHost));
new FileToModuleAliasingHost(testHost));
const {ShallowModule} = refs;
const scope = resolver.resolve(ShallowModule) !;
const [DeepDir, MiddleDir, ShallowDir] = scopeToRefs(scope);
@ -265,7 +266,7 @@ runInEachFileSystem(() => {
}
`,
},
new AliasGenerator(testHost));
new FileToModuleAliasingHost(testHost));
const {DeepExportModule} = refs;
const scope = resolver.resolve(DeepExportModule) !;
const [DeepDir] = scopeToRefs(scope);