diff --git a/packages/compiler-cli/src/ngcc/BUILD.bazel b/packages/compiler-cli/src/ngcc/BUILD.bazel index 00658a5e0e..129cbb02b8 100644 --- a/packages/compiler-cli/src/ngcc/BUILD.bazel +++ b/packages/compiler-cli/src/ngcc/BUILD.bazel @@ -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", diff --git a/packages/compiler-cli/src/ngcc/src/analysis/private_declarations_analyzer.ts b/packages/compiler-cli/src/ngcc/src/analysis/private_declarations_analyzer.ts index bfa76cf0c6..465c069534 100644 --- a/packages/compiler-cli/src/ngcc/src/analysis/private_declarations_analyzer.ts +++ b/packages/compiler-cli/src/ngcc/src/analysis/private_declarations_analyzer.ts @@ -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): PrivateDeclarationsAnalyses { const privateDeclarations: Map = new Map(declarations); + const exportAliasDeclarations: Map = 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) { - privateDeclarations.delete(declaration.node.name); + 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}; }); } } diff --git a/packages/compiler-cli/src/ngcc/src/main.ts b/packages/compiler-cli/src/ngcc/src/main.ts index f8c613d18a..a339919b01 100644 --- a/packages/compiler-cli/src/ngcc/src/main.ts +++ b/packages/compiler-cli/src/ngcc/src/main.ts @@ -79,7 +79,7 @@ export function mainNgcc(args: string[]): number { }); }); } catch (e) { - console.error(e.stack); + console.error(e.stack || e.message); return 1; } diff --git a/packages/compiler-cli/src/ngcc/src/rendering/esm_renderer.ts b/packages/compiler-cli/src/ngcc/src/rendering/esm_renderer.ts index 580ce15b99..7f8360ad93 100644 --- a/packages/compiler-cli/src/ngcc/src/rendering/esm_renderer.ts +++ b/packages/compiler-cli/src/ngcc/src/rendering/esm_renderer.ts @@ -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); - const relativePath = './' + relative(dirname(entryPointBasePath), basePath); - const exportFrom = entryPointBasePath !== basePath ? ` from '${relativePath}'` : ''; - const exportStr = `\nexport {${e.identifier}}${exportFrom};`; + 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); + 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); }); } diff --git a/packages/compiler-cli/src/ngcc/src/rendering/renderer.ts b/packages/compiler-cli/src/ngcc/src/rendering/renderer.ts index 00fc737b3f..70bd3fc70b 100644 --- a/packages/compiler-cli/src/ngcc/src/rendering/renderer.ts +++ b/packages/compiler-cli/src/ngcc/src/rendering/renderer.ts @@ -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); } diff --git a/packages/compiler-cli/src/ngcc/test/analysis/private_declarations_analyzer_spec.ts b/packages/compiler-cli/src/ngcc/test/analysis/private_declarations_analyzer_spec.ts index ed74e29212..98e6879093 100644 --- a/packages/compiler-cli/src/ngcc/test/analysis/private_declarations_analyzer_spec.ts +++ b/packages/compiler-cli/src/ngcc/test/analysis/private_declarations_analyzer_spec.ts @@ -14,142 +14,228 @@ import {PrivateDeclarationsAnalyzer} from '../../src/analysis/private_declaratio import {Esm2015ReflectionHost} from '../../src/host/esm2015_host'; import {getDeclaration, makeTestBundleProgram, makeTestProgram} from '../helpers/utils'; -const TEST_PROGRAM = [ - { - name: '/src/entry_point.js', - isRoot: true, - contents: ` +describe('PrivateDeclarationsAnalyzer', () => { + describe('analyzeProgram()', () => { + + const TEST_PROGRAM = [ + { + name: '/src/entry_point.js', + isRoot: true, + contents: ` export {PublicComponent} from './a'; export {ModuleA} from './mod'; export {ModuleB} from './b'; ` - }, - { - name: '/src/a.js', - isRoot: false, - contents: ` + }, + { + name: '/src/a.js', + isRoot: false, + contents: ` import {Component} from '@angular/core'; export class PublicComponent {} PublicComponent.decorators = [ {type: Component, args: [{selectors: 'a', template: ''}]} ]; ` - }, - { - name: '/src/b.js', - isRoot: false, - contents: ` + }, + { + name: '/src/b.js', + 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]}]} ]; ` - }, - { - name: '/src/c.js', - isRoot: false, - contents: ` + }, + { + name: '/src/c.js', + 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: ''}]} ]; ` - }, - { - name: '/src/mod.js', - isRoot: false, - contents: ` + }, + { + name: '/src/mod.js', + isRoot: false, + contents: ` 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] }]} ]; ` - } -]; -const TEST_DTS_PROGRAM = [ - { - name: '/typings/entry_point.d.ts', - isRoot: true, - contents: ` + } + ]; + const TEST_DTS_PROGRAM = [ + { + name: '/typings/entry_point.d.ts', + isRoot: true, + contents: ` export {PublicComponent} from './a'; export {ModuleA} from './mod'; export {ModuleB} from './b'; ` - }, - { - name: '/typings/a.d.ts', - isRoot: false, - contents: ` + }, + { + name: '/typings/a.d.ts', + isRoot: false, + contents: ` export declare class PublicComponent {} ` - }, - { - name: '/typings/b.d.ts', - isRoot: false, - contents: ` + }, + { + name: '/typings/b.d.ts', + isRoot: false, + contents: ` export declare class ModuleB {} ` - }, - { - name: '/typings/c.d.ts', - isRoot: false, - contents: ` - export declare class InternalComponent {} + }, + { + name: '/typings/c.d.ts', + isRoot: false, + contents: ` + export declare class InternalComponent1 {} ` - }, - { - name: '/typings/mod.d.ts', - isRoot: false, - contents: ` + }, + { + name: '/typings/mod.d.ts', + isRoot: false, + 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 host = new Esm2015ReflectionHost(false, program.getTypeChecker(), dts); - const referencesRegistry = new NgccReferencesRegistry(host); - const analyzer = new PrivateDeclarationsAnalyzer(host, referencesRegistry); + const {program, referencesRegistry, analyzer} = setup(TEST_PROGRAM, TEST_DTS_PROGRAM); - // 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)); + 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: 'PrivateComponent', from: '/src/b.js', dtsFrom: null}, - {identifier: 'InternalComponent', from: '/src/c.js', dtsFrom: '/typings/c.d.ts'}, + {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}; +} + +/** + * 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)); +} diff --git a/packages/compiler-cli/src/ngcc/test/rendering/esm2015_renderer_spec.ts b/packages/compiler-cli/src/ngcc/test/rendering/esm2015_renderer_spec.ts index fd6419e8cc..b5154418d8 100644 --- a/packages/compiler-cli/src/ngcc/test/rendering/esm2015_renderer_spec.ts +++ b/packages/compiler-cli/src/ngcc/test/rendering/esm2015_renderer_spec.ts @@ -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', () => { diff --git a/packages/compiler-cli/src/ngcc/test/rendering/esm5_renderer_spec.ts b/packages/compiler-cli/src/ngcc/test/rendering/esm5_renderer_spec.ts index 23f21f3d94..62ecd76e8a 100644 --- a/packages/compiler-cli/src/ngcc/test/rendering/esm5_renderer_spec.ts +++ b/packages/compiler-cli/src/ngcc/test/rendering/esm5_renderer_spec.ts @@ -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', () => {