Skip to content
Permalink
Browse files

fix(ngcc): do not emit ES2015 code in ES5 files

Previously, ngcc's `Renderer` would add some constants in the processed
files which were emitted as ES2015 code (e.g. `const` declarations).
This would result in invalid ES5 generated code that would break when
run on browsers that do not support the emitted format.

This commit fixes it by adding a `printStatement()` method to
`RenderingFormatter`, which can convert statements to JavaScript code in
a suitable format for the corresponding `RenderingFormatter`.
Additionally, the `translateExpression()` and `translateStatement()`
ngtsc helper methods are augmented to accept an extra hint to know
whether the code needs to be translated to ES5 format or not.

Fixes angular#32665
  • Loading branch information
gkalpak committed Nov 4, 2019
1 parent 1a4acfd commit d88ce3118866ea5c50ef21a31a8571fe5336f9de
@@ -91,7 +91,7 @@ assertSucceeded "Expected 'ngcc' to log 'Compiling'."
grep "const ɵMatTable_BaseFactory = ɵngcc0.ɵɵgetInheritedFactory(MatTable);" node_modules/@angular/material/esm2015/table.js
assertSucceeded "Expected 'ngcc' to generate a base factory for 'MatTable' in '@angular/material' (esm2015)."

grep "const ɵMatTable_BaseFactory = ɵngcc0.ɵɵgetInheritedFactory(MatTable);" node_modules/@angular/material/esm5/table.es5.js
grep "var ɵMatTable_BaseFactory = ɵngcc0.ɵɵgetInheritedFactory(MatTable);" node_modules/@angular/material/esm5/table.es5.js
assertSucceeded "Expected 'ngcc' to generate a base factory for 'MatTable' in '@angular/material' (esm5)."


@@ -5,8 +5,11 @@
* 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 {Statement} from '@angular/compiler';
import MagicString from 'magic-string';
import * as ts from 'typescript';
import {NOOP_DEFAULT_IMPORT_RECORDER} from '../../../src/ngtsc/imports';
import {ImportManager, translateStatement} from '../../../src/ngtsc/translator';
import {CompiledClass} from '../analysis/types';
import {getIifeBody} from '../host/esm5_host';
import {EsmRenderingFormatter} from './esm_rendering_formatter';
@@ -35,4 +38,22 @@ export class Esm5RenderingFormatter extends EsmRenderingFormatter {
const insertionPoint = returnStatement.getFullStart();
output.appendLeft(insertionPoint, '\n' + definitions);
}

/**
* Convert a `Statement` to JavaScript code in a format suitable for rendering by this formatter.
*
* @param stmt The `Statement` to print.
* @param sourceFile A `ts.SourceFile` that provides context for the statement. See
* `ts.Printer#printNode()` for more info.
* @param importManager The `ImportManager` to use for managing imports.
*
* @return The JavaScript code corresponding to `stmt` (in the appropriate format).
*/
printStatement(stmt: Statement, sourceFile: ts.SourceFile, importManager: ImportManager): string {
const node =
translateStatement(stmt, importManager, NOOP_DEFAULT_IMPORT_RECORDER, /* targetEs5 */ true);
const code = this.printer.printNode(ts.EmitHint.Unspecified, node, sourceFile);

return code;
}
}
@@ -5,23 +5,26 @@
* 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 {Statement} from '@angular/compiler';
import MagicString from 'magic-string';
import * as ts from 'typescript';
import {relative, dirname, AbsoluteFsPath, absoluteFromSourceFile} from '../../../src/ngtsc/file_system';
import {Import, ImportManager} from '../../../src/ngtsc/translator';
import {NOOP_DEFAULT_IMPORT_RECORDER, Reexport} from '../../../src/ngtsc/imports';
import {Import, ImportManager, translateStatement} from '../../../src/ngtsc/translator';
import {isDtsPath} from '../../../src/ngtsc/util/src/typescript';
import {CompiledClass} from '../analysis/types';
import {NgccReflectionHost, POST_R3_MARKER, PRE_R3_MARKER, SwitchableVariableDeclaration} from '../host/ngcc_host';
import {ModuleWithProvidersInfo} from '../analysis/module_with_providers_analyzer';
import {ExportInfo} from '../analysis/private_declarations_analyzer';
import {RenderingFormatter, RedundantDecoratorMap} from './rendering_formatter';
import {stripExtension} from './utils';
import {Reexport} from '../../../src/ngtsc/imports';

/**
* A RenderingFormatter that works with ECMAScript Module import and export statements.
*/
export class EsmRenderingFormatter implements RenderingFormatter {
protected printer = ts.createPrinter({newLine: ts.NewLineKind.LineFeed});

constructor(protected host: NgccReflectionHost, protected isCore: boolean) {}

/**
@@ -199,6 +202,23 @@ export class EsmRenderingFormatter implements RenderingFormatter {
});
}

/**
* Convert a `Statement` to JavaScript code in a format suitable for rendering by this formatter.
*
* @param stmt The `Statement` to print.
* @param sourceFile A `ts.SourceFile` that provides context for the statement. See
* `ts.Printer#printNode()` for more info.
* @param importManager The `ImportManager` to use for managing imports.
*
* @return The JavaScript code corresponding to `stmt` (in the appropriate format).
*/
printStatement(stmt: Statement, sourceFile: ts.SourceFile, importManager: ImportManager): string {
const node = translateStatement(stmt, importManager, NOOP_DEFAULT_IMPORT_RECORDER);
const code = this.printer.printNode(ts.EmitHint.Unspecified, node, sourceFile);

return code;
}

protected findEndOfImports(sf: ts.SourceFile): number {
for (const stmt of sf.statements) {
if (!ts.isImportDeclaration(stmt) && !ts.isImportEqualsDeclaration(stmt) &&
@@ -8,8 +8,7 @@
import {ConstantPool, Expression, Statement, WrappedNodeExpr, WritePropExpr} from '@angular/compiler';
import MagicString from 'magic-string';
import * as ts from 'typescript';
import {NOOP_DEFAULT_IMPORT_RECORDER} from '../../../src/ngtsc/imports';
import {translateStatement, ImportManager} from '../../../src/ngtsc/translator';
import {ImportManager} from '../../../src/ngtsc/translator';
import {CompiledClass, CompiledFile, DecorationAnalyses} from '../analysis/types';
import {PrivateDeclarationsAnalyses} from '../analysis/private_declarations_analyzer';
import {SwitchMarkerAnalyses, SwitchMarkerAnalysis} from '../analysis/switch_marker_analyzer';
@@ -81,7 +80,8 @@ export class Renderer {
this.srcFormatter.removeDecorators(outputText, decoratorsToRemove);

compiledFile.compiledClasses.forEach(clazz => {
const renderedDefinition = renderDefinitions(compiledFile.sourceFile, clazz, importManager);
const renderedDefinition =
renderDefinitions(this.srcFormatter, compiledFile.sourceFile, clazz, importManager);
this.srcFormatter.addDefinitions(outputText, clazz, renderedDefinition);

if (!isEntryPoint && clazz.reexports.length > 0) {
@@ -92,7 +92,8 @@ export class Renderer {

this.srcFormatter.addConstants(
outputText,
renderConstantPool(compiledFile.sourceFile, compiledFile.constantPool, importManager),
renderConstantPool(
this.srcFormatter, compiledFile.sourceFile, compiledFile.constantPool, importManager),
compiledFile.sourceFile);
}

@@ -149,11 +150,9 @@ export class Renderer {
* Render the constant pool as source code for the given class.
*/
export function renderConstantPool(
sourceFile: ts.SourceFile, constantPool: ConstantPool, imports: ImportManager): string {
const printer = createPrinter();
return constantPool.statements
.map(stmt => translateStatement(stmt, imports, NOOP_DEFAULT_IMPORT_RECORDER))
.map(stmt => printer.printNode(ts.EmitHint.Unspecified, stmt, sourceFile))
formatter: RenderingFormatter, sourceFile: ts.SourceFile, constantPool: ConstantPool,
imports: ImportManager): string {
return constantPool.statements.map(stmt => formatter.printStatement(stmt, sourceFile, imports))
.join('\n');
}

@@ -166,19 +165,16 @@ export function renderConstantPool(
* @param imports An object that tracks the imports that are needed by the rendered definitions.
*/
export function renderDefinitions(
sourceFile: ts.SourceFile, compiledClass: CompiledClass, imports: ImportManager): string {
const printer = createPrinter();
formatter: RenderingFormatter, sourceFile: ts.SourceFile, compiledClass: CompiledClass,
imports: ImportManager): string {
const name = compiledClass.declaration.name;
const translate = (stmt: Statement) =>
translateStatement(stmt, imports, NOOP_DEFAULT_IMPORT_RECORDER);
const print = (stmt: Statement) =>
printer.printNode(ts.EmitHint.Unspecified, translate(stmt), sourceFile);
const printStatement = (stmt: Statement) => formatter.printStatement(stmt, sourceFile, imports);
const statements: Statement[] =
compiledClass.compilation.map(c => createAssignmentStatement(name, c.name, c.initializer));
for (const c of compiledClass.compilation) {
statements.push(...c.statements);
}
return statements.map(print).join('\n');
return statements.map(printStatement).join('\n');
}

/**
@@ -191,7 +187,3 @@ function createAssignmentStatement(
const receiver = new WrappedNodeExpr(receiverName);
return new WritePropExpr(receiver, propName, initializer).toStmt();
}

function createPrinter(): ts.Printer {
return ts.createPrinter({newLine: ts.NewLineKind.LineFeed});
}
@@ -5,6 +5,7 @@
* 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 {Statement} from '@angular/compiler';
import MagicString from 'magic-string';
import * as ts from 'typescript';
import {Reexport} from '../../../src/ngtsc/imports';
@@ -43,4 +44,5 @@ export interface RenderingFormatter {
addModuleWithProvidersParams(
outputText: MagicString, moduleWithProviders: ModuleWithProvidersInfo[],
importManager: ImportManager): void;
printStatement(stmt: Statement, sourceFile: ts.SourceFile, importManager: ImportManager): string;
}
@@ -49,6 +49,7 @@ ts_library(
["integration/**/*.ts"],
),
deps = [
"//packages/compiler",
"//packages/compiler-cli/ngcc",
"//packages/compiler-cli/ngcc/test/helpers",
"//packages/compiler-cli/src/ngtsc/file_system",
@@ -147,6 +147,38 @@ runInEachFileSystem(() => {
'{ bar: [{ type: Input }] });');
});

it('should not add `const` in ES5 generated code', () => {
genNodeModules({
'test-package': {
'/index.ts': `
import {Directive, Input, NgModule} from '@angular/core';
@Directive({
selector: '[foo]',
host: {bar: ''},
})
export class FooDirective {
}
@NgModule({
declarations: [FooDirective],
})
export class FooModule {}
`,
},
});

mainNgcc({
basePath: '/node_modules',
targetEntryPointPath: 'test-package',
propertiesToConsider: ['main'],
});

const jsContents = fs.readFile(_(`/node_modules/test-package/index.js`));
expect(jsContents).not.toMatch(/\bconst \w+\s*=/);
expect(jsContents).toMatch(/\bvar _c0 =/);
});

describe('in async mode', () => {
it('should run ngcc without errors for fesm2015', async() => {
const promise = mainNgcc({
@@ -50,6 +50,7 @@ class TestRenderingFormatter implements RenderingFormatter {
importManager: ImportManager): void {
output.prepend('\n// ADD MODUlE WITH PROVIDERS PARAMS\n');
}
printStatement(): string { return 'IGNORED'; }
}

function createTestRenderer(
@@ -83,6 +84,7 @@ function createTestRenderer(
spyOn(testFormatter, 'removeDecorators').and.callThrough();
spyOn(testFormatter, 'rewriteSwitchableDeclarations').and.callThrough();
spyOn(testFormatter, 'addModuleWithProvidersParams').and.callThrough();
spyOn(testFormatter, 'printStatement').and.callThrough();

const renderer = new DtsRenderer(testFormatter, fs, logger, host, bundle);

@@ -5,14 +5,15 @@
* 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 {Statement} from '@angular/compiler';
import MagicString from 'magic-string';
import * as ts from 'typescript';
import {fromObject, generateMapFileComment, SourceMapConverter} from 'convert-source-map';
import {absoluteFrom, getFileSystem} from '../../../src/ngtsc/file_system';
import {TestFile, runInEachFileSystem} from '../../../src/ngtsc/file_system/testing';
import {Reexport} from '../../../src/ngtsc/imports';
import {NOOP_DEFAULT_IMPORT_RECORDER, Reexport} from '../../../src/ngtsc/imports';
import {loadTestFiles} from '../../../test/helpers';
import {Import, ImportManager} from '../../../src/ngtsc/translator';
import {Import, ImportManager, translateStatement} from '../../../src/ngtsc/translator';
import {DecorationAnalyzer} from '../../src/analysis/decoration_analyzer';
import {CompiledClass} from '../../src/analysis/types';
import {NgccReferencesRegistry} from '../../src/analysis/ngcc_references_registry';
@@ -26,6 +27,8 @@ import {RenderingFormatter, RedundantDecoratorMap} from '../../src/rendering/ren
import {makeTestEntryPointBundle, getRootFiles} from '../helpers/utils';

class TestRenderingFormatter implements RenderingFormatter {
private printer = ts.createPrinter({newLine: ts.NewLineKind.LineFeed});

addImports(output: MagicString, imports: Import[], sf: ts.SourceFile) {
output.prepend('\n// ADD IMPORTS\n');
}
@@ -52,6 +55,12 @@ class TestRenderingFormatter implements RenderingFormatter {
importManager: ImportManager): void {
output.prepend('\n// ADD MODUlE WITH PROVIDERS PARAMS\n');
}
printStatement(stmt: Statement, sourceFile: ts.SourceFile, importManager: ImportManager): string {
const node = translateStatement(stmt, importManager, NOOP_DEFAULT_IMPORT_RECORDER);
const code = this.printer.printNode(ts.EmitHint.Unspecified, node, sourceFile);

return `// TRANSPILED\n${code}`;
}
}

function createTestRenderer(
@@ -85,6 +94,7 @@ function createTestRenderer(
spyOn(testFormatter, 'removeDecorators').and.callThrough();
spyOn(testFormatter, 'rewriteSwitchableDeclarations').and.callThrough();
spyOn(testFormatter, 'addModuleWithProvidersParams').and.callThrough();
spyOn(testFormatter, 'printStatement').and.callThrough();

const renderer = new Renderer(testFormatter, fs, logger, bundle);

@@ -191,13 +201,15 @@ runInEachFileSystem(() => {
renderer.renderProgram(
decorationAnalyses, switchMarkerAnalyses, privateDeclarationsAnalyses);
const addDefinitionsSpy = testFormatter.addDefinitions as jasmine.Spy;
expect(addDefinitionsSpy.calls.first().args[2])
.toEqual(`A.ɵfac = function A_Factory(t) { return new (t || A)(); };
expect(addDefinitionsSpy.calls.first().args[2]).toEqual(`// TRANSPILED
A.ɵfac = function A_Factory(t) { return new (t || A)(); };
// TRANSPILED
A.ɵcmp = ɵngcc0.ɵɵdefineComponent({ type: A, selectors: [["a"]], decls: 1, vars: 1, template: function A_Template(rf, ctx) { if (rf & 1) {
ɵngcc0.ɵɵtext(0);
} if (rf & 2) {
ɵngcc0.ɵɵtextInterpolate(ctx.person.name);
} }, encapsulation: 2 });
// TRANSPILED
/*@__PURE__*/ ɵngcc0.ɵsetClassMetadata(A, [{
type: Component,
args: [{ selector: 'a', template: '{{ person!.name }}' }]
@@ -232,9 +244,11 @@ A.ɵcmp = ɵngcc0.ɵɵdefineComponent({ type: A, selectors: [["a"]], decls: 1, v
decorators: [jasmine.objectContaining({name: 'Directive'})]
}));

expect(addDefinitionsSpy.calls.first().args[2])
.toEqual(`A.ɵfac = function A_Factory(t) { return new (t || A)(); };
expect(addDefinitionsSpy.calls.first().args[2]).toEqual(`// TRANSPILED
A.ɵfac = function A_Factory(t) { return new (t || A)(); };
// TRANSPILED
A.ɵdir = ɵngcc0.ɵɵdefineDirective({ type: A, selectors: [["", "a", ""]] });
// TRANSPILED
/*@__PURE__*/ ɵngcc0.ɵsetClassMetadata(A, [{
type: Directive,
args: [{ selector: '[a]' }]
@@ -307,9 +321,9 @@ A.ɵdir = ɵngcc0.ɵɵdefineDirective({ type: A, selectors: [["", "a", ""]] });
decorationAnalyses, switchMarkerAnalyses, privateDeclarationsAnalyses);

const addDefinitionsSpy = testFormatter.addDefinitions as jasmine.Spy;
expect(addDefinitionsSpy.calls.first().args[2])
.toEqual(
`UndecoratedBase.ɵfac = function UndecoratedBase_Factory(t) { return new (t || UndecoratedBase)(); };
expect(addDefinitionsSpy.calls.first().args[2]).toEqual(`// TRANSPILED
UndecoratedBase.ɵfac = function UndecoratedBase_Factory(t) { return new (t || UndecoratedBase)(); };
// TRANSPILED
UndecoratedBase.ɵdir = ɵngcc0.ɵɵdefineDirective({ type: UndecoratedBase, viewQuery: function UndecoratedBase_Query(rf, ctx) { if (rf & 1) {
ɵngcc0.ɵɵstaticViewQuery(_c0, true);
} if (rf & 2) {
@@ -100,17 +100,19 @@ export class ImportManager {
}

export function translateExpression(
expression: Expression, imports: ImportManager,
defaultImportRecorder: DefaultImportRecorder): ts.Expression {
expression: Expression, imports: ImportManager, defaultImportRecorder: DefaultImportRecorder,
targetEs5 = false): ts.Expression {
return expression.visitExpression(
new ExpressionTranslatorVisitor(imports, defaultImportRecorder), new Context(false));
new ExpressionTranslatorVisitor(imports, defaultImportRecorder, targetEs5),
new Context(false));
}

export function translateStatement(
statement: Statement, imports: ImportManager,
defaultImportRecorder: DefaultImportRecorder): ts.Statement {
statement: Statement, imports: ImportManager, defaultImportRecorder: DefaultImportRecorder,
targetEs5 = false): ts.Statement {
return statement.visitStatement(
new ExpressionTranslatorVisitor(imports, defaultImportRecorder), new Context(true));
new ExpressionTranslatorVisitor(imports, defaultImportRecorder, targetEs5),
new Context(true));
}

export function translateType(type: Type, imports: ImportManager): ts.TypeNode {
@@ -120,10 +122,13 @@ export function translateType(type: Type, imports: ImportManager): ts.TypeNode {
class ExpressionTranslatorVisitor implements ExpressionVisitor, StatementVisitor {
private externalSourceFiles = new Map<string, ts.SourceMapSource>();
constructor(
private imports: ImportManager, private defaultImportRecorder: DefaultImportRecorder) {}
private imports: ImportManager, private defaultImportRecorder: DefaultImportRecorder,
private targetEs5: boolean) {}

visitDeclareVarStmt(stmt: DeclareVarStmt, context: Context): ts.VariableStatement {
const nodeFlags = stmt.hasModifier(StmtModifier.Final) ? ts.NodeFlags.Const : ts.NodeFlags.None;
const nodeFlags = (!this.targetEs5 && stmt.hasModifier(StmtModifier.Final)) ?
ts.NodeFlags.Const :
ts.NodeFlags.None;
return ts.createVariableStatement(
undefined, ts.createVariableDeclarationList(
[ts.createVariableDeclaration(
@@ -252,6 +257,11 @@ class ExpressionTranslatorVisitor implements ExpressionVisitor, StatementVisitor
}

visitLocalizedString(ast: LocalizedString, context: Context): ts.Expression {
if (this.targetEs5) {
// This should never happen. (Right?)
// TODO(gkalpak): Create a proper error.
throw new Error('Not supported.');
}
return visitLocalizedString(ast, context, this);
}

0 comments on commit d88ce31

Please sign in to comment.
You can’t perform that action at this time.