Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Closed
wants to merge 1 commit into from
Closed

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/compiler-cli/src/ngcc/BUILD.bazel
View file
Open in desktop
Expand Up @@ -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",
Expand Down
50 changes: 47 additions & 3 deletions packages/compiler-cli/src/ngcc/src/analysis/private_declarations_analyzer.ts
View file
Open in desktop
Expand Up @@ -16,6 +16,7 @@ export interface ExportInfo {
identifier: string;
from: string;
dtsFrom?: string|null;
alias?: string|null;
}
export type PrivateDeclarationsAnalyses = ExportInfo[];

Expand All @@ -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) {
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};
});
}
}
2 changes: 1 addition & 1 deletion packages/compiler-cli/src/ngcc/src/main.ts
View file
Open in desktop
Expand Up @@ -79,7 +79,7 @@ export function mainNgcc(args: string[]): number {
});
});
} catch (e) {
console.error(e.stack);
console.error(e.stack || e.message);
return 1;
}

Expand Down
24 changes: 16 additions & 8 deletions packages/compiler-cli/src/ngcc/src/rendering/esm_renderer.ts
View file
Open in desktop
Expand Up @@ -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(
Expand All @@ -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);
});
}
Expand Down
13 changes: 5 additions & 8 deletions packages/compiler-cli/src/ngcc/src/rendering/renderer.ts
View file
Open in desktop
Expand Up @@ -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(
Expand Down Expand Up @@ -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);
}

Expand Down