From 9264f43511a14e72058951dc1ee7597a5b554388 Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Wed, 18 Dec 2019 14:03:05 +0000 Subject: [PATCH] refactor(ngcc): remove private declaration aliases (#34254) Now that the source to typings matching is able to handle aliasing of exports, there is no need to handle aliases in private declarations analysis. These were originally added to cope when the typings files had to use the name that the original source files used when exporting. PR Close #34254 --- .../analysis/private_declarations_analyzer.ts | 18 +----- .../ngcc/src/rendering/dts_renderer.ts | 2 +- .../src/rendering/esm_rendering_formatter.ts | 4 +- .../private_declarations_analyzer_spec.ts | 60 ------------------- .../commonjs_rendering_formatter_spec.ts | 30 ---------- .../esm5_rendering_formatter_spec.ts | 30 ---------- .../rendering/esm_rendering_formatter_spec.ts | 30 ---------- .../rendering/umd_rendering_formatter_spec.ts | 30 ---------- 8 files changed, 5 insertions(+), 199 deletions(-) diff --git a/packages/compiler-cli/ngcc/src/analysis/private_declarations_analyzer.ts b/packages/compiler-cli/ngcc/src/analysis/private_declarations_analyzer.ts index e8498c8e8d..d005345201 100644 --- a/packages/compiler-cli/ngcc/src/analysis/private_declarations_analyzer.ts +++ b/packages/compiler-cli/ngcc/src/analysis/private_declarations_analyzer.ts @@ -17,7 +17,6 @@ export interface ExportInfo { identifier: string; from: AbsoluteFsPath; dtsFrom?: AbsoluteFsPath|null; - alias?: string|null; } export type PrivateDeclarationsAnalyses = ExportInfo[]; @@ -42,7 +41,6 @@ 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); @@ -54,17 +52,8 @@ export class PrivateDeclarationsAnalyzer { 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 { - // 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. - exportAliasDeclarations.set(declaration.node.name, exportedName); - } + // This declaration is public so we can remove it from the list + privateDeclarations.delete(declaration.node.name); } } }); @@ -74,11 +63,10 @@ export class PrivateDeclarationsAnalyzer { return Array.from(privateDeclarations.keys()).map(id => { const from = absoluteFromSourceFile(id.getSourceFile()); const declaration = privateDeclarations.get(id) !; - const alias = exportAliasDeclarations.has(id) ? exportAliasDeclarations.get(id) ! : null; const dtsDeclaration = this.host.getDtsDeclaration(declaration.node); const dtsFrom = dtsDeclaration && absoluteFromSourceFile(dtsDeclaration.getSourceFile()); - return {identifier: id.text, from, dtsFrom, alias}; + return {identifier: id.text, from, dtsFrom}; }); } } diff --git a/packages/compiler-cli/ngcc/src/rendering/dts_renderer.ts b/packages/compiler-cli/ngcc/src/rendering/dts_renderer.ts index e754d3be38..9063070c55 100644 --- a/packages/compiler-cli/ngcc/src/rendering/dts_renderer.ts +++ b/packages/compiler-cli/ngcc/src/rendering/dts_renderer.ts @@ -156,7 +156,7 @@ export class DtsRenderer { // Capture the private declarations that need to be re-exported if (privateDeclarationsAnalyses.length) { privateDeclarationsAnalyses.forEach(e => { - if (!e.dtsFrom && !e.alias) { + if (!e.dtsFrom) { 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 ` + diff --git a/packages/compiler-cli/ngcc/src/rendering/esm_rendering_formatter.ts b/packages/compiler-cli/ngcc/src/rendering/esm_rendering_formatter.ts index ef8e79c2ac..54e46f63fb 100644 --- a/packages/compiler-cli/ngcc/src/rendering/esm_rendering_formatter.ts +++ b/packages/compiler-cli/ngcc/src/rendering/esm_rendering_formatter.ts @@ -55,9 +55,7 @@ export class EsmRenderingFormatter implements RenderingFormatter { 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};`; + const exportStr = `\nexport {${e.identifier}}${exportFrom};`; output.append(exportStr); }); } diff --git a/packages/compiler-cli/ngcc/test/analysis/private_declarations_analyzer_spec.ts b/packages/compiler-cli/ngcc/test/analysis/private_declarations_analyzer_spec.ts index ffebe17e49..f31e9c003c 100644 --- a/packages/compiler-cli/ngcc/test/analysis/private_declarations_analyzer_spec.ts +++ b/packages/compiler-cli/ngcc/test/analysis/private_declarations_analyzer_spec.ts @@ -161,74 +161,14 @@ runInEachFileSystem(() => { identifier: 'PrivateComponent1', from: _('/node_modules/test-package/src/b.js'), dtsFrom: null, - alias: null }, { identifier: 'InternalComponent1', from: _('/node_modules/test-package/src/c.js'), dtsFrom: _('/node_modules/test-package/typings/c.d.ts'), - alias: null }, ]); }); - - it('should find all non-public declarations that were aliased', () => { - const _ = absoluteFrom; - const ALIASED_EXPORTS_PROGRAM = [ - { - name: _('/node_modules/test-package/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: _('/node_modules/test-package/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: _('/node_modules/test-package/typings/entry_point.d.ts'), - isRoot: true, - contents: ` - export declare class aliasedComponentOne {} - export declare class ComponentTwo {} - export {ComponentTwo as aliasedComponentTwo} - ` - }, - ]; - const {program, referencesRegistry, analyzer} = - setup(ALIASED_EXPORTS_PROGRAM, ALIASED_EXPORTS_DTS_PROGRAM); - - addToReferencesRegistry( - program, referencesRegistry, _('/node_modules/test-package/src/a.js'), 'ComponentOne'); - addToReferencesRegistry( - program, referencesRegistry, _('/node_modules/test-package/src/a.js'), 'ComponentTwo'); - - const analyses = analyzer.analyzeProgram(program); - expect(analyses).toEqual([{ - identifier: 'ComponentOne', - from: _('/node_modules/test-package/src/a.js'), - dtsFrom: _('/node_modules/test-package/typings/entry_point.d.ts'), - alias: 'aliasedComponentOne', - }]); - }); }); }); diff --git a/packages/compiler-cli/ngcc/test/rendering/commonjs_rendering_formatter_spec.ts b/packages/compiler-cli/ngcc/test/rendering/commonjs_rendering_formatter_spec.ts index c77e32ba1c..30a1390a6c 100644 --- a/packages/compiler-cli/ngcc/test/rendering/commonjs_rendering_formatter_spec.ts +++ b/packages/compiler-cli/ngcc/test/rendering/commonjs_rendering_formatter_spec.ts @@ -226,36 +226,6 @@ exports.ComponentA2 = i0.ComponentA2; exports.ComponentB = i1.ComponentB; exports.TopLevelComponent = TopLevelComponent;`); }); - - it('should not insert alias exports in js output', () => { - const {importManager, renderer, sourceFile} = setup(PROGRAM); - const output = new MagicString(PROGRAM.contents); - renderer.addExports( - output, _(PROGRAM.name.replace(/\.js$/, '')), - [ - { - from: _('/node_modules/test-package/some/a.js'), - alias: 'eComponentA1', - identifier: 'ComponentA1' - }, - { - from: _('/node_modules/test-package/some/a.js'), - alias: 'eComponentA2', - identifier: 'ComponentA2' - }, - { - from: _('/node_modules/test-package/some/foo/b.js'), - alias: 'eComponentB', - identifier: 'ComponentB' - }, - {from: PROGRAM.name, alias: 'eTopLevelComponent', identifier: 'TopLevelComponent'}, - ], - importManager, sourceFile); - 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/ngcc/test/rendering/esm5_rendering_formatter_spec.ts b/packages/compiler-cli/ngcc/test/rendering/esm5_rendering_formatter_spec.ts index 57616b0a34..79701f3378 100644 --- a/packages/compiler-cli/ngcc/test/rendering/esm5_rendering_formatter_spec.ts +++ b/packages/compiler-cli/ngcc/test/rendering/esm5_rendering_formatter_spec.ts @@ -229,36 +229,6 @@ export {ComponentA2} from './a'; export {ComponentB} from './foo/b'; export {TopLevelComponent};`); }); - - it('should not insert alias exports in js output', () => { - const {importManager, renderer, sourceFile} = setup(PROGRAM); - const output = new MagicString(PROGRAM.contents); - renderer.addExports( - output, _(PROGRAM.name.replace(/\.js$/, '')), - [ - { - from: _('/node_modules/test-package/some/a.js'), - alias: 'eComponentA1', - identifier: 'ComponentA1' - }, - { - from: _('/node_modules/test-package/some/a.js'), - alias: 'eComponentA2', - identifier: 'ComponentA2' - }, - { - from: _('/node_modules/test-package/some/foo/b.js'), - alias: 'eComponentB', - identifier: 'ComponentB' - }, - {from: PROGRAM.name, alias: 'eTopLevelComponent', identifier: 'TopLevelComponent'}, - ], - importManager, sourceFile); - 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/ngcc/test/rendering/esm_rendering_formatter_spec.ts b/packages/compiler-cli/ngcc/test/rendering/esm_rendering_formatter_spec.ts index 7ac27aca13..2336cd742e 100644 --- a/packages/compiler-cli/ngcc/test/rendering/esm_rendering_formatter_spec.ts +++ b/packages/compiler-cli/ngcc/test/rendering/esm_rendering_formatter_spec.ts @@ -146,36 +146,6 @@ export {ComponentA2} from './a'; export {ComponentB} from './foo/b'; export {TopLevelComponent};`); }); - - it('should not insert alias exports in js output', () => { - const {importManager, renderer, sourceFile} = setup([PROGRAM]); - const output = new MagicString(PROGRAM.contents); - renderer.addExports( - output, _(PROGRAM.name.replace(/\.js$/, '')), - [ - { - from: _('/node_modules/test-package/some/a.js'), - alias: 'eComponentA1', - identifier: 'ComponentA1' - }, - { - from: _('/node_modules/test-package/some/a.js'), - alias: 'eComponentA2', - identifier: 'ComponentA2' - }, - { - from: _('/node_modules/test-package/some/foo/b.js'), - alias: 'eComponentB', - identifier: 'ComponentB' - }, - {from: PROGRAM.name, alias: 'eTopLevelComponent', identifier: 'TopLevelComponent'}, - ], - importManager, sourceFile); - 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/ngcc/test/rendering/umd_rendering_formatter_spec.ts b/packages/compiler-cli/ngcc/test/rendering/umd_rendering_formatter_spec.ts index 10264beaaa..0a0a8bc665 100644 --- a/packages/compiler-cli/ngcc/test/rendering/umd_rendering_formatter_spec.ts +++ b/packages/compiler-cli/ngcc/test/rendering/umd_rendering_formatter_spec.ts @@ -359,36 +359,6 @@ exports.TopLevelComponent = TopLevelComponent; expect(generateNamedImportSpy).toHaveBeenCalledWith('./a', 'ComponentA2'); expect(generateNamedImportSpy).toHaveBeenCalledWith('./foo/b', 'ComponentB'); }); - - it('should not insert alias exports in js output', () => { - const {importManager, renderer, sourceFile} = setup(PROGRAM); - const output = new MagicString(PROGRAM.contents); - renderer.addExports( - output, PROGRAM.name.replace(/\.js$/, ''), - [ - { - from: _('/node_modules/test-package/some/a.js'), - alias: 'eComponentA1', - identifier: 'ComponentA1' - }, - { - from: _('/node_modules/test-package/some/a.js'), - alias: 'eComponentA2', - identifier: 'ComponentA2' - }, - { - from: _('/node_modules/test-package/some/foo/b.js'), - alias: 'eComponentB', - identifier: 'ComponentB' - }, - {from: PROGRAM.name, alias: 'eTopLevelComponent', identifier: 'TopLevelComponent'}, - ], - importManager, sourceFile); - const outputString = output.toString(); - expect(outputString).not.toContain(`eComponentA1`); - expect(outputString).not.toContain(`eComponentB`); - expect(outputString).not.toContain(`eTopLevelComponent`); - }); }); describe('addConstants', () => {