feat(ivy): ngcc - render private declaration exports (#26906)

Ngcc will now render additional exports for classes that are referenced in
`NgModule` decorated classes, but which were not publicly exported
from an entry-point of the package.

This is important because when ngtsc compiles libraries processed by ngcc
it needs to be able to publcly access decorated classes that are referenced
by `NgModule` decorated classes in order to build templates that use these
classes.

Doing this re-exporting is not without its risks. There are chances that
the class is not exported correctly: there may already be similarly named
exports from the entry-point or the class may be being aliased. But there
is not much more we can do from the point of view of ngcc to workaround
such scenarios. Generally, packages should have been built so that this
approach works.

PR Close #26906
This commit is contained in:
Pete Bacon Darwin 2018-11-25 22:07:51 +00:00 committed by Igor Minar
parent bf3ac41e36
commit 912b0529c1
6 changed files with 165 additions and 44 deletions

View File

@ -11,6 +11,7 @@ import {mkdir, mv} from 'shelljs';
import {DecorationAnalyzer} from '../analysis/decoration_analyzer';
import {NgccReferencesRegistry} from '../analysis/ngcc_references_registry';
import {PrivateDeclarationsAnalyzer} from '../analysis/private_declarations_analyzer';
import {SwitchMarkerAnalyzer} from '../analysis/switch_marker_analyzer';
import {Esm2015ReflectionHost} from '../host/esm2015_host';
import {Esm5ReflectionHost} from '../host/esm5_host';
@ -62,12 +63,13 @@ export class Transformer {
const reflectionHost = this.getHost(isCore, bundle);
// Parse and analyze the files.
const {decorationAnalyses, switchMarkerAnalyses} =
const {decorationAnalyses, switchMarkerAnalyses, privateDeclarationsAnalyses} =
this.analyzeProgram(reflectionHost, isCore, bundle);
// Transform the source files and source maps.
const renderer = this.getRenderer(reflectionHost, isCore, bundle);
const renderedFiles = renderer.renderProgram(decorationAnalyses, switchMarkerAnalyses);
const renderedFiles = renderer.renderProgram(
decorationAnalyses, switchMarkerAnalyses, privateDeclarationsAnalyses);
// Write out all the transformed files.
renderedFiles.forEach(file => this.writeFile(file));
@ -109,9 +111,13 @@ export class Transformer {
const decorationAnalyzer = new DecorationAnalyzer(
typeChecker, reflectionHost, referencesRegistry, bundle.rootDirs, isCore);
const switchMarkerAnalyzer = new SwitchMarkerAnalyzer(reflectionHost);
const privateDeclarationsAnalyzer =
new PrivateDeclarationsAnalyzer(reflectionHost, referencesRegistry);
const decorationAnalyses = decorationAnalyzer.analyzeProgram(bundle.src.program);
const switchMarkerAnalyses = switchMarkerAnalyzer.analyzeProgram(bundle.src.program);
return {decorationAnalyses, switchMarkerAnalyses};
const privateDeclarationsAnalyses =
privateDeclarationsAnalyzer.analyzeProgram(bundle.src.program);
return {decorationAnalyses, switchMarkerAnalyses, privateDeclarationsAnalyses};
}
writeFile(file: FileInfo): void {

View File

@ -5,10 +5,12 @@
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/
import * as ts from 'typescript';
import {dirname, relative} from 'canonical-path';
import MagicString from 'magic-string';
import * as ts from 'typescript';
import {NgccReflectionHost, POST_R3_MARKER, PRE_R3_MARKER, SwitchableVariableDeclaration} from '../host/ngcc_host';
import {CompiledClass} from '../analysis/decoration_analyzer';
import {ExportInfo} from '../analysis/private_declarations_analyzer';
import {Renderer, stripExtension} from './renderer';
import {EntryPointBundle} from '../packages/entry_point_bundle';
@ -27,6 +29,19 @@ export class EsmRenderer extends Renderer {
imports.forEach(i => { output.appendLeft(0, `import * as ${i.as} from '${i.name}';\n`); });
}
addExports(output: MagicString, entryPointBasePath: string, exports: {
identifier: string,
from: string
}[]): 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};`;
output.append(exportStr);
});
}
addConstants(output: MagicString, constants: string, file: ts.SourceFile): void {
if (constants === '') {
return;

View File

@ -18,6 +18,7 @@ import {CompileResult} from '@angular/compiler-cli/src/ngtsc/transform';
import {translateStatement, translateType} from '../../../ngtsc/translator';
import {NgccImportManager} from './ngcc_import_manager';
import {CompiledClass, CompiledFile, DecorationAnalyses} from '../analysis/decoration_analyzer';
import {PrivateDeclarationsAnalyses, ExportInfo} from '../analysis/private_declarations_analyzer';
import {SwitchMarkerAnalyses, SwitchMarkerAnalysis} from '../analysis/switch_marker_analyzer';
import {IMPORT_PREFIX} from '../constants';
import {NgccReflectionHost, SwitchableVariableDeclaration} from '../host/ngcc_host';
@ -60,8 +61,9 @@ export abstract class Renderer {
protected bundle: EntryPointBundle, protected sourcePath: string,
protected targetPath: string) {}
renderProgram(decorationAnalyses: DecorationAnalyses, switchMarkerAnalyses: SwitchMarkerAnalyses):
FileInfo[] {
renderProgram(
decorationAnalyses: DecorationAnalyses, switchMarkerAnalyses: SwitchMarkerAnalyses,
privateDeclarationsAnalyses: PrivateDeclarationsAnalyses): FileInfo[] {
const renderedFiles: FileInfo[] = [];
// Transform the source files.
@ -70,7 +72,8 @@ export abstract class Renderer {
const switchMarkerAnalysis = switchMarkerAnalyses.get(sourceFile);
if (compiledFile || switchMarkerAnalysis || sourceFile === this.bundle.src.file) {
renderedFiles.push(...this.renderFile(sourceFile, compiledFile, switchMarkerAnalysis));
renderedFiles.push(...this.renderFile(
sourceFile, compiledFile, switchMarkerAnalysis, privateDeclarationsAnalyses));
}
});
@ -83,7 +86,9 @@ export abstract class Renderer {
if (!dtsFiles.has(this.bundle.dts.file)) {
dtsFiles.set(this.bundle.dts.file, []);
}
dtsFiles.forEach((classes, file) => renderedFiles.push(...this.renderDtsFile(file, classes)));
dtsFiles.forEach(
(classes, file) => renderedFiles.push(
...this.renderDtsFile(file, classes, privateDeclarationsAnalyses)));
}
return renderedFiles;
@ -96,7 +101,8 @@ export abstract class Renderer {
*/
renderFile(
sourceFile: ts.SourceFile, compiledFile: CompiledFile|undefined,
switchMarkerAnalysis: SwitchMarkerAnalysis|undefined): FileInfo[] {
switchMarkerAnalysis: SwitchMarkerAnalysis|undefined,
privateDeclarationsAnalyses: PrivateDeclarationsAnalyses): FileInfo[] {
const input = this.extractSourceMap(sourceFile);
const outputText = new MagicString(input.source);
@ -129,10 +135,18 @@ export abstract class Renderer {
this.removeDecorators(outputText, decoratorsToRemove);
}
// Add exports to the entry-point file
if (sourceFile === this.bundle.src.file) {
const entryPointBasePath = stripExtension(this.bundle.src.path);
this.addExports(outputText, entryPointBasePath, privateDeclarationsAnalyses);
}
return this.renderSourceAndMap(sourceFile, input, outputText);
}
renderDtsFile(dtsFile: ts.SourceFile, dtsClasses: DtsClassInfo[]): FileInfo[] {
renderDtsFile(
dtsFile: ts.SourceFile, dtsClasses: DtsClassInfo[],
privateDeclarationsAnalyses: PrivateDeclarationsAnalyses): FileInfo[] {
const input = this.extractSourceMap(dtsFile);
const outputText = new MagicString(input.source);
const importManager = new NgccImportManager(false, this.isCore, IMPORT_PREFIX);
@ -149,12 +163,30 @@ export abstract class Renderer {
this.addImports(
outputText, importManager.getAllImports(dtsFile.fileName, this.bundle.dts !.r3SymbolsFile));
if (dtsFile === this.bundle.dts !.file) {
const dtsExports = privateDeclarationsAnalyses.map(e => {
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 ` +
`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};
});
this.addExports(outputText, dtsFile.fileName, dtsExports);
}
return this.renderSourceAndMap(dtsFile, input, outputText);
}
protected abstract addConstants(output: MagicString, constants: string, file: ts.SourceFile):
void;
protected abstract addImports(output: MagicString, imports: {name: string, as: string}[]): void;
protected abstract addExports(output: MagicString, entryPointBasePath: string, exports: {
identifier: string,
from: string
}[]): void;
protected abstract addDefinitions(
output: MagicString, compiledClass: CompiledClass, definitions: string): void;
protected abstract removeDecorators(

View File

@ -6,15 +6,14 @@
* found in the LICENSE file at https://angular.io/license
*/
import {dirname} from 'canonical-path';
import * as ts from 'typescript';
import MagicString from 'magic-string';
import {makeTestEntryPointBundle} from '../helpers/utils';
import * as ts from 'typescript';
import {DecorationAnalyzer} from '../../src/analysis/decoration_analyzer';
import {NgccReferencesRegistry} from '../../src/analysis/ngcc_references_registry';
import {SwitchMarkerAnalyzer} from '../../src/analysis/switch_marker_analyzer';
import {Esm2015ReflectionHost} from '../../src/host/esm2015_host';
import {EsmRenderer} from '../../src/rendering/esm_renderer';
import {makeTestEntryPointBundle} from '../helpers/utils';
function setup(file: {name: string, contents: string}) {
const dir = dirname(file.name);
@ -122,6 +121,24 @@ import * as i1 from '@angular/common';
});
});
describe('addExports', () => {
it('should insert the given exports at the end of the source file', () => {
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'},
]);
expect(output.toString()).toContain(`
// Some other content
export {ComponentA1} from './a';
export {ComponentA2} from './a';
export {ComponentB} from './foo/b';
export {TopLevelComponent};`);
});
});
describe('addConstants', () => {
it('should insert the given constants after imports in the source file', () => {

View File

@ -8,13 +8,12 @@
import {dirname} from 'canonical-path';
import MagicString from 'magic-string';
import * as ts from 'typescript';
import {makeTestEntryPointBundle, getDeclaration} from '../helpers/utils';
import {DecorationAnalyzer} from '../../src/analysis/decoration_analyzer';
import {NgccReferencesRegistry} from '../../src/analysis/ngcc_references_registry';
import {SwitchMarkerAnalyzer} from '../../src/analysis/switch_marker_analyzer';
import {Esm5ReflectionHost} from '../../src/host/esm5_host';
import {Esm5Renderer} from '../../src/rendering/esm5_renderer';
import {makeTestEntryPointBundle, getDeclaration} from '../helpers/utils';
function setup(file: {name: string, contents: string}) {
const dir = dirname(file.name);
@ -159,6 +158,24 @@ import * as i1 from '@angular/common';
});
});
describe('addExports', () => {
it('should insert the given exports at the end of the source file', () => {
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'},
]);
expect(output.toString()).toContain(`
export {A, B, C, NoIife, BadIife};
export {ComponentA1} from './a';
export {ComponentA2} from './a';
export {ComponentB} from './foo/b';
export {TopLevelComponent};`);
});
});
describe('addConstants', () => {
it('should insert the given constants after imports in the source file', () => {

View File

@ -11,10 +11,10 @@ import * as ts from 'typescript';
import {fromObject, generateMapFileComment} from 'convert-source-map';
import {CompiledClass, DecorationAnalyzer} from '../../src/analysis/decoration_analyzer';
import {NgccReferencesRegistry} from '../../src/analysis/ngcc_references_registry';
import {PrivateDeclarationsAnalyzer} from '../../src/analysis/private_declarations_analyzer';
import {SwitchMarkerAnalyzer} from '../../src/analysis/switch_marker_analyzer';
import {Esm2015ReflectionHost} from '../../src/host/esm2015_host';
import {Renderer} from '../../src/rendering/renderer';
import {EntryPoint} from '../../src/packages/entry_point';
import {EntryPointBundle} from '../../src/packages/entry_point_bundle';
import {makeTestEntryPointBundle} from '../helpers/utils';
@ -25,6 +25,12 @@ class TestRenderer extends Renderer {
addImports(output: MagicString, imports: {name: string, as: string}[]) {
output.prepend('\n// ADD IMPORTS\n');
}
addExports(output: MagicString, baseEntryPointPath: string, exports: {
identifier: string,
from: string
}[]) {
output.prepend('\n// ADD EXPORTS\n');
}
addConstants(output: MagicString, constants: string, file: ts.SourceFile): void {
output.prepend('\n// ADD CONSTANTS\n');
}
@ -51,12 +57,14 @@ function createTestRenderer(
new DecorationAnalyzer(typeChecker, host, referencesRegistry, bundle.rootDirs, isCore)
.analyzeProgram(bundle.src.program);
const switchMarkerAnalyses = new SwitchMarkerAnalyzer(host).analyzeProgram(bundle.src.program);
const privateDeclarationsAnalyses =
new PrivateDeclarationsAnalyzer(host, referencesRegistry).analyzeProgram(bundle.src.program);
const renderer = new TestRenderer(host, isCore, bundle);
spyOn(renderer, 'addImports').and.callThrough();
spyOn(renderer, 'addDefinitions').and.callThrough();
spyOn(renderer, 'removeDecorators').and.callThrough();
return {renderer, decorationAnalyses, switchMarkerAnalyses};
return {renderer, decorationAnalyses, switchMarkerAnalyses, privateDeclarationsAnalyses};
}
@ -83,7 +91,7 @@ describe('Renderer', () => {
});
const RENDERED_CONTENTS =
`\n// REMOVE DECORATORS\n\n// ADD IMPORTS\n\n// ADD CONSTANTS\n\n// ADD DEFINITIONS\n` +
`\n// ADD EXPORTS\n\n// REMOVE DECORATORS\n\n// ADD IMPORTS\n\n// ADD CONSTANTS\n\n// ADD DEFINITIONS\n` +
INPUT_PROGRAM.contents;
const OUTPUT_PROGRAM_MAP = fromObject({
@ -92,14 +100,14 @@ describe('Renderer', () => {
'sources': ['/src/file.js'],
'sourcesContent': [INPUT_PROGRAM.contents],
'names': [],
'mappings': ';;;;;;;;AAAA;;;;;;;;;'
'mappings': ';;;;;;;;;;AAAA;;;;;;;;;'
});
const MERGED_OUTPUT_PROGRAM_MAP = fromObject({
'version': 3,
'sources': ['/src/file.ts'],
'names': [],
'mappings': ';;;;;;;;AAAA',
'mappings': ';;;;;;;;;;AAAA',
'file': '/dist/file.js',
'sourcesContent': [INPUT_PROGRAM.contents]
});
@ -107,9 +115,10 @@ describe('Renderer', () => {
describe('renderProgram()', () => {
it('should render the modified contents; and a new map file, if the original provided no map file.',
() => {
const {renderer, decorationAnalyses, switchMarkerAnalyses} =
const {renderer, decorationAnalyses, switchMarkerAnalyses, privateDeclarationsAnalyses} =
createTestRenderer('test-package', [INPUT_PROGRAM]);
const result = renderer.renderProgram(decorationAnalyses, switchMarkerAnalyses);
const result = renderer.renderProgram(
decorationAnalyses, switchMarkerAnalyses, privateDeclarationsAnalyses);
expect(result[0].path).toEqual('/dist/file.js');
expect(result[0].contents)
.toEqual(RENDERED_CONTENTS + '\n' + generateMapFileComment('/dist/file.js.map'));
@ -120,9 +129,10 @@ describe('Renderer', () => {
describe('calling abstract methods', () => {
it('should call addImports with the source code and info about the core Angular library.',
() => {
const {decorationAnalyses, renderer, switchMarkerAnalyses} =
const {renderer, decorationAnalyses, switchMarkerAnalyses, privateDeclarationsAnalyses} =
createTestRenderer('test-package', [INPUT_PROGRAM]);
renderer.renderProgram(decorationAnalyses, switchMarkerAnalyses);
const result = renderer.renderProgram(
decorationAnalyses, switchMarkerAnalyses, privateDeclarationsAnalyses);
const addImportsSpy = renderer.addImports as jasmine.Spy;
expect(addImportsSpy.calls.first().args[0].toString()).toEqual(RENDERED_CONTENTS);
expect(addImportsSpy.calls.first().args[1]).toEqual([
@ -132,9 +142,10 @@ describe('Renderer', () => {
it('should call addDefinitions with the source code, the analyzed class and the rendered definitions.',
() => {
const {decorationAnalyses, renderer, switchMarkerAnalyses} =
const {renderer, decorationAnalyses, switchMarkerAnalyses, privateDeclarationsAnalyses} =
createTestRenderer('test-package', [INPUT_PROGRAM]);
renderer.renderProgram(decorationAnalyses, switchMarkerAnalyses);
const result = renderer.renderProgram(
decorationAnalyses, switchMarkerAnalyses, privateDeclarationsAnalyses);
const addDefinitionsSpy = renderer.addDefinitions as jasmine.Spy;
expect(addDefinitionsSpy.calls.first().args[0].toString()).toEqual(RENDERED_CONTENTS);
expect(addDefinitionsSpy.calls.first().args[1]).toEqual(jasmine.objectContaining({
@ -151,9 +162,10 @@ A.ngDirectiveDef = ɵngcc0.ɵdefineDirective({ type: A, selectors: [["", "a", ""
it('should call removeDecorators with the source code, a map of class decorators that have been analyzed',
() => {
const {decorationAnalyses, renderer, switchMarkerAnalyses} =
const {renderer, decorationAnalyses, switchMarkerAnalyses, privateDeclarationsAnalyses} =
createTestRenderer('test-package', [INPUT_PROGRAM]);
renderer.renderProgram(decorationAnalyses, switchMarkerAnalyses);
const result = renderer.renderProgram(
decorationAnalyses, switchMarkerAnalyses, privateDeclarationsAnalyses);
const removeDecoratorsSpy = renderer.removeDecorators as jasmine.Spy;
expect(removeDecoratorsSpy.calls.first().args[0].toString()).toEqual(RENDERED_CONTENTS);
@ -175,12 +187,14 @@ A.ngDirectiveDef = ɵngcc0.ɵdefineDirective({ type: A, selectors: [["", "a", ""
describe('source map merging', () => {
it('should merge any inline source map from the original file and write the output as an inline source map',
() => {
const {decorationAnalyses, renderer, switchMarkerAnalyses} = createTestRenderer(
'test-package', [{
...INPUT_PROGRAM,
contents: INPUT_PROGRAM.contents + '\n' + INPUT_PROGRAM_MAP.toComment()
}]);
const result = renderer.renderProgram(decorationAnalyses, switchMarkerAnalyses);
const {decorationAnalyses, renderer, switchMarkerAnalyses, privateDeclarationsAnalyses} =
createTestRenderer(
'test-package', [{
...INPUT_PROGRAM,
contents: INPUT_PROGRAM.contents + '\n' + INPUT_PROGRAM_MAP.toComment()
}]);
const result = renderer.renderProgram(
decorationAnalyses, switchMarkerAnalyses, privateDeclarationsAnalyses);
expect(result[0].path).toEqual('/dist/file.js');
expect(result[0].contents)
.toEqual(RENDERED_CONTENTS + '\n' + MERGED_OUTPUT_PROGRAM_MAP.toComment());
@ -191,12 +205,14 @@ A.ngDirectiveDef = ɵngcc0.ɵdefineDirective({ type: A, selectors: [["", "a", ""
() => {
// Mock out reading the map file from disk
spyOn(fs, 'readFileSync').and.returnValue(INPUT_PROGRAM_MAP.toJSON());
const {decorationAnalyses, renderer, switchMarkerAnalyses} = createTestRenderer(
'test-package', [{
...INPUT_PROGRAM,
contents: INPUT_PROGRAM.contents + '\n//# sourceMappingURL=file.js.map'
}]);
const result = renderer.renderProgram(decorationAnalyses, switchMarkerAnalyses);
const {decorationAnalyses, renderer, switchMarkerAnalyses, privateDeclarationsAnalyses} =
createTestRenderer(
'test-package', [{
...INPUT_PROGRAM,
contents: INPUT_PROGRAM.contents + '\n//# sourceMappingURL=file.js.map'
}]);
const result = renderer.renderProgram(
decorationAnalyses, switchMarkerAnalyses, privateDeclarationsAnalyses);
expect(result[0].path).toEqual('/dist/file.js');
expect(result[0].contents)
.toEqual(RENDERED_CONTENTS + '\n' + generateMapFileComment('/dist/file.js.map'));
@ -250,9 +266,10 @@ A.ngDirectiveDef = ɵngcc0.ɵdefineDirective({ type: A, selectors: [["", "a", ""
describe('rendering typings', () => {
it('should render extract types into typings files', () => {
const {renderer, decorationAnalyses, switchMarkerAnalyses} =
const {renderer, decorationAnalyses, switchMarkerAnalyses, privateDeclarationsAnalyses} =
createTestRenderer('test-package', [INPUT_PROGRAM], INPUT_DTS_PROGRAM);
const result = renderer.renderProgram(decorationAnalyses, switchMarkerAnalyses);
const result = renderer.renderProgram(
decorationAnalyses, switchMarkerAnalyses, privateDeclarationsAnalyses);
const typingsFile = result.find(f => f.path === '/typings/file.d.ts') !;
expect(typingsFile.contents)
@ -261,13 +278,30 @@ A.ngDirectiveDef = ɵngcc0.ɵdefineDirective({ type: A, selectors: [["", "a", ""
});
it('should render imports into typings files', () => {
const {renderer, decorationAnalyses, switchMarkerAnalyses} =
const {renderer, decorationAnalyses, switchMarkerAnalyses, privateDeclarationsAnalyses} =
createTestRenderer('test-package', [INPUT_PROGRAM], INPUT_DTS_PROGRAM);
const result = renderer.renderProgram(decorationAnalyses, switchMarkerAnalyses);
const result = renderer.renderProgram(
decorationAnalyses, switchMarkerAnalyses, privateDeclarationsAnalyses);
const typingsFile = result.find(f => f.path === '/typings/file.d.ts') !;
expect(typingsFile.contents).toContain(`// ADD IMPORTS\nexport declare class A`);
});
it('should render exports into typings files', () => {
const {renderer, decorationAnalyses, switchMarkerAnalyses, privateDeclarationsAnalyses} =
createTestRenderer('test-package', [INPUT_PROGRAM], INPUT_DTS_PROGRAM);
// Add a mock export to trigger export rendering
privateDeclarationsAnalyses.push(
{identifier: 'ComponentB', from: '/src/file.js', dtsFrom: '/typings/b.d.ts'});
const result = renderer.renderProgram(
decorationAnalyses, switchMarkerAnalyses, privateDeclarationsAnalyses);
const typingsFile = result.find(f => f.path === '/typings/file.d.ts') !;
expect(typingsFile.contents)
.toContain(`// ADD EXPORTS\n\n// ADD IMPORTS\nexport declare class A`);
});
});
});
});