fix(ivy): render alias exports for private declarations if possible (#28735)

Sometimes declarations are not exported publicly but are exported under
a private name. In this case, rather than adding a completely new
export to the entry point, we should create an export that aliases the
private name back to the original public name.

This is important when the typings files have been rolled-up using a tool
such as the [API Extractor](https://api-extractor.com/). In this case
the internal type of an aliased private export will be removed completely
from the typings file, so there is no "original" type to re-export.

For example:

If there are the following TS files:

**entry-point.ts**

```ts
export {Internal as External} from './internal';
```

**internal.ts**

```ts
export class Internal {
  foo(): void;
}
```

Then the API Extractor might roll up the .d.ts files into:

```ts
export declare class External {
  foo(): void;
}
```

In this case ngcc should add an export so the file looks like:

```ts
export declare class External {
  foo(): void;
}
export {External as Internal};
```

PR Close #28735
This commit is contained in:
Alan Agius 2019-02-14 18:59:46 +01:00 committed by Kara Erickson
parent 49dccf4bfc
commit a5b8420234
8 changed files with 278 additions and 112 deletions

View File

@ -20,6 +20,7 @@ ts_library(
"//packages/compiler-cli/src/ngtsc/scope",
"//packages/compiler-cli/src/ngtsc/transform",
"//packages/compiler-cli/src/ngtsc/translator",
"//packages/compiler-cli/src/ngtsc/util",
"@npm//@types/convert-source-map",
"@npm//@types/node",
"@npm//@types/shelljs",

View File

@ -16,6 +16,7 @@ export interface ExportInfo {
identifier: string;
from: string;
dtsFrom?: string|null;
alias?: string|null;
}
export type PrivateDeclarationsAnalyses = ExportInfo[];
@ -40,22 +41,65 @@ export class PrivateDeclarationsAnalyzer {
rootFiles: ts.SourceFile[],
declarations: Map<ts.Identifier, Declaration>): PrivateDeclarationsAnalyses {
const privateDeclarations: Map<ts.Identifier, Declaration> = new Map(declarations);
const exportAliasDeclarations: Map<ts.Identifier, string> = new Map();
rootFiles.forEach(f => {
const exports = this.host.getExportsOfModule(f);
if (exports) {
exports.forEach((declaration, exportedName) => {
if (hasNameIdentifier(declaration.node) && declaration.node.name.text === exportedName) {
if (hasNameIdentifier(declaration.node)) {
const privateDeclaration = privateDeclarations.get(declaration.node.name);
if (privateDeclaration) {
if (privateDeclaration.node !== declaration.node) {
throw new Error(`${declaration.node.name.text} is declared multiple times.`);
}
if (declaration.node.name.text === exportedName) {
// This declaration is public so we can remove it from the list
privateDeclarations.delete(declaration.node.name);
} else if (!this.host.getDtsDeclaration(declaration.node)) {
// The referenced declaration is exported publicly but via an alias.
// In some cases the original declaration is missing from the dts program, such as
// when rolling up (flattening) the dts files.
// This is because the original declaration gets renamed to the exported alias.
// There is a constraint on this which we cannot handle. Consider the following
// code:
//
// /src/entry_point.js:
// export {MyComponent as aliasedMyComponent} from './a';
// export {MyComponent} from './b';`
//
// /src/a.js:
// export class MyComponent {}
//
// /src/b.js:
// export class MyComponent {}
//
// //typings/entry_point.d.ts:
// export declare class aliasedMyComponent {}
// export declare class MyComponent {}
//
// In this case we would end up matching the `MyComponent` from `/src/a.js` to the
// `MyComponent` declared in `/typings/entry_point.d.ts` even though that
// declaration is actually for the `MyComponent` in `/src/b.js`.
exportAliasDeclarations.set(declaration.node.name, exportedName);
}
}
}
});
}
});
return Array.from(privateDeclarations.keys()).map(id => {
const from = id.getSourceFile().fileName;
const declaration = privateDeclarations.get(id) !;
const alias = exportAliasDeclarations.get(id) || null;
const dtsDeclaration = this.host.getDtsDeclaration(declaration.node);
const dtsFrom = dtsDeclaration && dtsDeclaration.getSourceFile().fileName;
return {identifier: id.text, from, dtsFrom};
return {identifier: id.text, from, dtsFrom, alias};
});
}
}

View File

@ -79,7 +79,7 @@ export function mainNgcc(args: string[]): number {
});
});
} catch (e) {
console.error(e.stack);
console.error(e.stack || e.message);
return 1;
}

View File

@ -12,6 +12,8 @@ import {NgccReflectionHost, POST_R3_MARKER, PRE_R3_MARKER, SwitchableVariableDec
import {CompiledClass} from '../analysis/decoration_analyzer';
import {RedundantDecoratorMap, Renderer, stripExtension} from './renderer';
import {EntryPointBundle} from '../packages/entry_point_bundle';
import {ExportInfo} from '../analysis/private_declarations_analyzer';
import {isDtsPath} from '../../../ngtsc/util/src/typescript';
export class EsmRenderer extends Renderer {
constructor(
@ -36,15 +38,21 @@ export class EsmRenderer extends Renderer {
});
}
addExports(output: MagicString, entryPointBasePath: string, exports: {
identifier: string,
from: string
}[]): void {
addExports(output: MagicString, entryPointBasePath: string, exports: ExportInfo[]): void {
exports.forEach(e => {
const basePath = stripExtension(e.from);
let exportFrom = '';
const isDtsFile = isDtsPath(entryPointBasePath);
const from = isDtsFile ? e.dtsFrom : e.from;
if (from) {
const basePath = stripExtension(from);
const relativePath = './' + relative(dirname(entryPointBasePath), basePath);
const exportFrom = entryPointBasePath !== basePath ? ` from '${relativePath}'` : '';
const exportStr = `\nexport {${e.identifier}}${exportFrom};`;
exportFrom = entryPointBasePath !== basePath ? ` from '${relativePath}'` : '';
}
// aliases should only be added in dts files as these are lost when rolling up dts file.
const exportStatement = e.alias && isDtsFile ? `${e.alias} as ${e.identifier}` : e.identifier;
const exportStr = `\nexport {${exportStatement}}${exportFrom};`;
output.append(exportStr);
});
}

View File

@ -248,10 +248,8 @@ export abstract class Renderer {
protected abstract addImports(
output: MagicString,
imports: {specifier: string, qualifier: string, isDefault: boolean}[]): void;
protected abstract addExports(output: MagicString, entryPointBasePath: string, exports: {
identifier: string,
from: string
}[]): void;
protected abstract addExports(
output: MagicString, entryPointBasePath: string, exports: ExportInfo[]): void;
protected abstract addDefinitions(
output: MagicString, compiledClass: CompiledClass, definitions: string): void;
protected abstract removeDecorators(
@ -391,19 +389,18 @@ export abstract class Renderer {
// Capture the private declarations that need to be re-exported
if (privateDeclarationsAnalyses.length) {
const dtsExports = privateDeclarationsAnalyses.map(e => {
if (!e.dtsFrom) {
privateDeclarationsAnalyses.forEach(e => {
if (!e.dtsFrom && !e.alias) {
throw new Error(
`There is no typings path for ${e.identifier} in ${e.from}.\n` +
`We need to add an export for this class to a .d.ts typings file because ` +
`Angular compiler needs to be able to reference this class in compiled code, such as templates.\n` +
`The simplest fix for this is to ensure that this class is exported from the package's entry-point.`);
}
return {identifier: e.identifier, from: e.dtsFrom};
});
const dtsEntryPoint = this.bundle.dts !.file;
const renderInfo = dtsMap.get(dtsEntryPoint) || new DtsRenderInfo();
renderInfo.privateExports = dtsExports;
renderInfo.privateExports = privateDeclarationsAnalyses;
dtsMap.set(dtsEntryPoint, renderInfo);
}

View File

@ -14,6 +14,9 @@ import {PrivateDeclarationsAnalyzer} from '../../src/analysis/private_declaratio
import {Esm2015ReflectionHost} from '../../src/host/esm2015_host';
import {getDeclaration, makeTestBundleProgram, makeTestProgram} from '../helpers/utils';
describe('PrivateDeclarationsAnalyzer', () => {
describe('analyzeProgram()', () => {
const TEST_PROGRAM = [
{
name: '/src/entry_point.js',
@ -40,13 +43,17 @@ const TEST_PROGRAM = [
isRoot: false,
contents: `
import {Component, NgModule} from '@angular/core';
class PrivateComponent {}
PrivateComponent.decorators = [
class PrivateComponent1 {}
PrivateComponent1.decorators = [
{type: Component, args: [{selectors: 'b', template: ''}]}
];
class PrivateComponent2 {}
PrivateComponent2.decorators = [
{type: Component, args: [{selectors: 'c', template: ''}]}
];
export class ModuleB {}
ModuleB.decorators = [
{type: NgModule, args: [{declarations: [PrivateComponent]}]}
{type: NgModule, args: [{declarations: [PrivateComponent1]}]}
];
`
},
@ -55,9 +62,13 @@ const TEST_PROGRAM = [
isRoot: false,
contents: `
import {Component} from '@angular/core';
export class InternalComponent {}
InternalComponent.decorators = [
{type: Component, args: [{selectors: 'c', template: ''}]}
export class InternalComponent1 {}
InternalComponent1.decorators = [
{type: Component, args: [{selectors: 'd', template: ''}]}
];
export class InternalComponent2 {}
InternalComponent2.decorators = [
{type: Component, args: [{selectors: 'e', template: ''}]}
];
`
},
@ -68,11 +79,11 @@ const TEST_PROGRAM = [
import {Component, NgModule} from '@angular/core';
import {PublicComponent} from './a';
import {ModuleB} from './b';
import {InternalComponent} from './c';
import {InternalComponent1} from './c';
export class ModuleA {}
ModuleA.decorators = [
{type: NgModule, args: [{
declarations: [PublicComponent, InternalComponent],
declarations: [PublicComponent, InternalComponent1],
imports: [ModuleB]
}]}
];
@ -107,7 +118,7 @@ const TEST_DTS_PROGRAM = [
name: '/typings/c.d.ts',
isRoot: false,
contents: `
export declare class InternalComponent {}
export declare class InternalComponent1 {}
`
},
{
@ -116,40 +127,115 @@ const TEST_DTS_PROGRAM = [
contents: `
import {PublicComponent} from './a';
import {ModuleB} from './b';
import {InternalComponent} from './c';
import {InternalComponent1} from './c';
export declare class ModuleA {}
`
},
];
describe('PrivateDeclarationsAnalyzer', () => {
describe('analyzeProgram()', () => {
it('should find all NgModule declarations that were not publicly exported from the entry-point',
() => {
const program = makeTestProgram(...TEST_PROGRAM);
const dts = makeTestBundleProgram(TEST_DTS_PROGRAM);
const {program, referencesRegistry, analyzer} = setup(TEST_PROGRAM, TEST_DTS_PROGRAM);
addToReferencesRegistry(program, referencesRegistry, '/src/a.js', 'PublicComponent');
addToReferencesRegistry(program, referencesRegistry, '/src/b.js', 'PrivateComponent1');
addToReferencesRegistry(program, referencesRegistry, '/src/c.js', 'InternalComponent1');
const analyses = analyzer.analyzeProgram(program);
// Note that `PrivateComponent2` and `InternalComponent2` are not found because they are
// not added to the ReferencesRegistry (i.e. they were not declared in an NgModule).
expect(analyses.length).toEqual(2);
expect(analyses).toEqual([
{identifier: 'PrivateComponent1', from: '/src/b.js', dtsFrom: null, alias: null},
{
identifier: 'InternalComponent1',
from: '/src/c.js',
dtsFrom: '/typings/c.d.ts',
alias: null
},
]);
});
const ALIASED_EXPORTS_PROGRAM = [
{
name: '/src/entry_point.js',
isRoot: true,
contents: `
// This component is only exported as an alias.
export {ComponentOne as aliasedComponentOne} from './a';
// This component is exported both as itself and an alias.
export {ComponentTwo as aliasedComponentTwo, ComponentTwo} from './a';
`
},
{
name: '/src/a.js',
isRoot: false,
contents: `
import {Component} from '@angular/core';
export class ComponentOne {}
ComponentOne.decorators = [
{type: Component, args: [{selectors: 'a', template: ''}]}
];
export class ComponentTwo {}
Component.decorators = [
{type: Component, args: [{selectors: 'a', template: ''}]}
];
`
}
];
const ALIASED_EXPORTS_DTS_PROGRAM = [
{
name: '/typings/entry_point.d.ts',
isRoot: true,
contents: `
export declare class aliasedComponentOne {}
export declare class ComponentTwo {}
export {ComponentTwo as aliasedComponentTwo}
`
},
];
it('should find all non-public declarations that were aliased', () => {
const {program, referencesRegistry, analyzer} =
setup(ALIASED_EXPORTS_PROGRAM, ALIASED_EXPORTS_DTS_PROGRAM);
addToReferencesRegistry(program, referencesRegistry, '/src/a.js', 'ComponentOne');
addToReferencesRegistry(program, referencesRegistry, '/src/a.js', 'ComponentTwo');
const analyses = analyzer.analyzeProgram(program);
expect(analyses).toEqual([{
identifier: 'ComponentOne',
from: '/src/a.js',
dtsFrom: null,
alias: 'aliasedComponentOne',
}]);
});
});
});
type Files = {
name: string,
contents: string, isRoot?: boolean | undefined
}[];
function setup(jsProgram: Files, dtsProgram: Files) {
const program = makeTestProgram(...jsProgram);
const dts = makeTestBundleProgram(dtsProgram);
const host = new Esm2015ReflectionHost(false, program.getTypeChecker(), dts);
const referencesRegistry = new NgccReferencesRegistry(host);
const analyzer = new PrivateDeclarationsAnalyzer(host, referencesRegistry);
return {program, referencesRegistry, analyzer};
}
// Set up the registry with references - this would normally be done by the
// decoration handlers in the `DecorationAnalyzer`.
const publicComponentDeclaration =
getDeclaration(program, '/src/a.js', 'PublicComponent', ts.isClassDeclaration);
referencesRegistry.add(null !, new Reference(publicComponentDeclaration));
const privateComponentDeclaration =
getDeclaration(program, '/src/b.js', 'PrivateComponent', ts.isClassDeclaration);
referencesRegistry.add(null !, new Reference(privateComponentDeclaration));
const internalComponentDeclaration =
getDeclaration(program, '/src/c.js', 'InternalComponent', ts.isClassDeclaration);
referencesRegistry.add(null !, new Reference(internalComponentDeclaration));
const analyses = analyzer.analyzeProgram(program);
expect(analyses.length).toEqual(2);
expect(analyses).toEqual([
{identifier: 'PrivateComponent', from: '/src/b.js', dtsFrom: null},
{identifier: 'InternalComponent', from: '/src/c.js', dtsFrom: '/typings/c.d.ts'},
]);
});
});
});
/**
* Add up the named component to the references registry.
*
* This would normally be done by the decoration handlers in the `DecorationAnalyzer`.
*/
function addToReferencesRegistry(
program: ts.Program, registry: NgccReferencesRegistry, fileName: string,
componentName: string) {
const declaration = getDeclaration(program, fileName, componentName, ts.isClassDeclaration);
registry.add(null !, new Reference(declaration));
}

View File

@ -140,10 +140,10 @@ import * as i1 from '@angular/common';
const {renderer} = setup(PROGRAM);
const output = new MagicString(PROGRAM.contents);
renderer.addExports(output, PROGRAM.name.replace(/\.js$/, ''), [
{from: '/some/a.js', identifier: 'ComponentA1'},
{from: '/some/a.js', identifier: 'ComponentA2'},
{from: '/some/foo/b.js', identifier: 'ComponentB'},
{from: PROGRAM.name, identifier: 'TopLevelComponent'},
{from: '/some/a.js', dtsFrom: '/some/a.d.ts', identifier: 'ComponentA1'},
{from: '/some/a.js', dtsFrom: '/some/a.d.ts', identifier: 'ComponentA2'},
{from: '/some/foo/b.js', dtsFrom: '/some/foo/b.d.ts', identifier: 'ComponentB'},
{from: PROGRAM.name, dtsFrom: PROGRAM.name, identifier: 'TopLevelComponent'},
]);
expect(output.toString()).toContain(`
// Some other content
@ -152,6 +152,21 @@ export {ComponentA2} from './a';
export {ComponentB} from './foo/b';
export {TopLevelComponent};`);
});
it('should not insert alias exports in js output', () => {
const {renderer} = setup(PROGRAM);
const output = new MagicString(PROGRAM.contents);
renderer.addExports(output, PROGRAM.name.replace(/\.js$/, ''), [
{from: '/some/a.js', alias: 'eComponentA1', identifier: 'ComponentA1'},
{from: '/some/a.js', alias: 'eComponentA2', identifier: 'ComponentA2'},
{from: '/some/foo/b.js', alias: 'eComponentB', identifier: 'ComponentB'},
{from: PROGRAM.name, alias: 'eTopLevelComponent', identifier: 'TopLevelComponent'},
]);
const outputString = output.toString();
expect(outputString).not.toContain(`{eComponentA1 as ComponentA1}`);
expect(outputString).not.toContain(`{eComponentB as ComponentB}`);
expect(outputString).not.toContain(`{eTopLevelComponent as TopLevelComponent}`);
});
});
describe('addConstants', () => {

View File

@ -177,10 +177,10 @@ import * as i1 from '@angular/common';
const {renderer} = setup(PROGRAM);
const output = new MagicString(PROGRAM.contents);
renderer.addExports(output, PROGRAM.name.replace(/\.js$/, ''), [
{from: '/some/a.js', identifier: 'ComponentA1'},
{from: '/some/a.js', identifier: 'ComponentA2'},
{from: '/some/foo/b.js', identifier: 'ComponentB'},
{from: PROGRAM.name, identifier: 'TopLevelComponent'},
{from: '/some/a.js', dtsFrom: '/some/a.d.ts', identifier: 'ComponentA1'},
{from: '/some/a.js', dtsFrom: '/some/a.d.ts', identifier: 'ComponentA2'},
{from: '/some/foo/b.js', dtsFrom: '/some/foo/b.d.ts', identifier: 'ComponentB'},
{from: PROGRAM.name, dtsFrom: PROGRAM.name, identifier: 'TopLevelComponent'},
]);
expect(output.toString()).toContain(`
export {A, B, C, NoIife, BadIife};
@ -189,6 +189,21 @@ export {ComponentA2} from './a';
export {ComponentB} from './foo/b';
export {TopLevelComponent};`);
});
it('should not insert alias exports in js output', () => {
const {renderer} = setup(PROGRAM);
const output = new MagicString(PROGRAM.contents);
renderer.addExports(output, PROGRAM.name.replace(/\.js$/, ''), [
{from: '/some/a.js', alias: 'eComponentA1', identifier: 'ComponentA1'},
{from: '/some/a.js', alias: 'eComponentA2', identifier: 'ComponentA2'},
{from: '/some/foo/b.js', alias: 'eComponentB', identifier: 'ComponentB'},
{from: PROGRAM.name, alias: 'eTopLevelComponent', identifier: 'TopLevelComponent'},
]);
const outputString = output.toString();
expect(outputString).not.toContain(`{eComponentA1 as ComponentA1}`);
expect(outputString).not.toContain(`{eComponentB as ComponentB}`);
expect(outputString).not.toContain(`{eTopLevelComponent as TopLevelComponent}`);
});
});
describe('addConstants', () => {