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
This commit is contained in:
Pete Bacon Darwin 2019-12-18 14:03:05 +00:00 committed by Kara Erickson
parent 918d8c9909
commit 9264f43511
8 changed files with 5 additions and 199 deletions

View File

@ -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<ts.Identifier, ConcreteDeclaration>): PrivateDeclarationsAnalyses {
const privateDeclarations: Map<ts.Identifier, ConcreteDeclaration> = new Map(declarations);
const exportAliasDeclarations: Map<ts.Identifier, string> = 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};
});
}
}

View File

@ -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 ` +

View File

@ -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);
});
}

View File

@ -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',
}]);
});
});
});

View File

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

View File

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

View File

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

View File

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