Skip to content

Commit

Permalink
fix(ngcc): do not emit ES2015 code in ES5 files
Browse files Browse the repository at this point in the history
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 13, 2019
1 parent 0f44e01 commit 1993cce
Show file tree
Hide file tree
Showing 17 changed files with 222 additions and 55 deletions.
2 changes: 1 addition & 1 deletion integration/ngcc/test.sh
Expand Up @@ -95,7 +95,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)."


Expand Down
Expand Up @@ -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';
Expand Down Expand Up @@ -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, ts.ScriptTarget.ES5);
const code = this.printer.printNode(ts.EmitHint.Unspecified, node, sourceFile);

return code;
}
}
Expand Up @@ -5,24 +5,27 @@
* 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';
import {isAssignment} from '../host/esm2015_host';

/**
* 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) {}

/**
Expand Down Expand Up @@ -225,6 +228,24 @@ 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, ts.ScriptTarget.ES2015);
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) &&
Expand Down
35 changes: 13 additions & 22 deletions packages/compiler-cli/ngcc/src/rendering/renderer.ts
Expand Up @@ -8,19 +8,18 @@
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';
import {IMPORT_PREFIX} from '../constants';
import {FileSystem} from '../../../src/ngtsc/file_system';
import {EntryPointBundle} from '../packages/entry_point_bundle';
import {NgccReflectionHost} from '../host/ngcc_host';
import {Logger} from '../logging/logger';
import {FileToWrite, getImportRewriter, stripExtension} from './utils';
import {EntryPointBundle} from '../packages/entry_point_bundle';
import {RenderingFormatter, RedundantDecoratorMap} from './rendering_formatter';
import {extractSourceMap, renderSourceAndMap} from './source_maps';
import {NgccReflectionHost} from '../host/ngcc_host';
import {FileToWrite, getImportRewriter, stripExtension} from './utils';

/**
* A base-class for rendering an `AnalyzedFile`.
Expand Down Expand Up @@ -98,7 +97,8 @@ export class Renderer {

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

Expand Down Expand Up @@ -185,25 +185,20 @@ export class Renderer {

private renderStatements(
sourceFile: ts.SourceFile, statements: Statement[], imports: ImportManager): string {
const printer = createPrinter();
const translate = (stmt: Statement) =>
translateStatement(stmt, imports, NOOP_DEFAULT_IMPORT_RECORDER);
const print = (stmt: Statement) =>
printer.printNode(ts.EmitHint.Unspecified, translate(stmt), sourceFile);
return statements.map(print).join('\n');
const printStatement = (stmt: Statement) =>
this.srcFormatter.printStatement(stmt, sourceFile, imports);
return statements.map(printStatement).join('\n');
}
}

/**
* 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))
.join('\n');
formatter: RenderingFormatter, sourceFile: ts.SourceFile, constantPool: ConstantPool,
imports: ImportManager): string {
const printStatement = (stmt: Statement) => formatter.printStatement(stmt, sourceFile, imports);
return constantPool.statements.map(printStatement).join('\n');
}

/**
Expand All @@ -216,7 +211,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});
}
Expand Up @@ -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';
Expand Down Expand Up @@ -45,4 +46,5 @@ export interface RenderingFormatter {
addModuleWithProvidersParams(
outputText: MagicString, moduleWithProviders: ModuleWithProvidersInfo[],
importManager: ImportManager): void;
printStatement(stmt: Statement, sourceFile: ts.SourceFile, importManager: ImportManager): string;
}
1 change: 1 addition & 0 deletions packages/compiler-cli/ngcc/test/BUILD.bazel
Expand Up @@ -10,6 +10,7 @@ ts_library(
exclude = ["integration/**/*.ts"],
),
deps = [
"//packages/compiler",
"//packages/compiler-cli/ngcc",
"//packages/compiler-cli/ngcc/test/helpers",
"//packages/compiler-cli/src/ngtsc/diagnostics",
Expand Down
32 changes: 32 additions & 0 deletions packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts
Expand Up @@ -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({
Expand Down
Expand Up @@ -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 {DeclareVarStmt, LiteralExpr, StmtModifier} from '@angular/compiler';
import MagicString from 'magic-string';
import * as ts from 'typescript';
import {NoopImportRewriter} from '../../../src/ngtsc/imports';
Expand Down Expand Up @@ -518,5 +519,19 @@ SOME DEFINITION TEXT
expect(output.toString()).toContain(`function C() {\n }\n return C;`);
});
});

describe('printStatement', () => {
it('should transpile code to ES5', () => {
const {renderer, sourceFile, importManager} = setup(PROGRAM);

const stmt1 = new DeclareVarStmt('foo', new LiteralExpr(42), null, [StmtModifier.Static]);
const stmt2 = new DeclareVarStmt('bar', new LiteralExpr(true));
const stmt3 = new DeclareVarStmt('baz', new LiteralExpr('qux'), undefined, []);

expect(renderer.printStatement(stmt1, sourceFile, importManager)).toBe('var foo = 42;');
expect(renderer.printStatement(stmt2, sourceFile, importManager)).toBe('var bar = true;');
expect(renderer.printStatement(stmt3, sourceFile, importManager)).toBe('var baz = "qux";');
});
});
});
});
Expand Up @@ -53,6 +53,7 @@ class TestRenderingFormatter implements RenderingFormatter {
importManager: ImportManager): void {
output.prepend('\n// ADD MODUlE WITH PROVIDERS PARAMS\n');
}
printStatement(): string { return 'IGNORED'; }
}

function createTestRenderer(
Expand Down Expand Up @@ -87,6 +88,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);

Expand Down
Expand Up @@ -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 {DeclareVarStmt, LiteralExpr, StmtModifier} from '@angular/compiler';
import MagicString from 'magic-string';
import * as ts from 'typescript';
import {NoopImportRewriter} from '../../../src/ngtsc/imports';
Expand Down Expand Up @@ -545,5 +546,19 @@ SOME DEFINITION TEXT
});
});
});

describe('printStatement', () => {
it('should transpile code to ES5', () => {
const {renderer, sourceFile, importManager} = setup(PROGRAM);

const stmt1 = new DeclareVarStmt('foo', new LiteralExpr(42), null, [StmtModifier.Static]);
const stmt2 = new DeclareVarStmt('bar', new LiteralExpr(true));
const stmt3 = new DeclareVarStmt('baz', new LiteralExpr('qux'), undefined, []);

expect(renderer.printStatement(stmt1, sourceFile, importManager)).toBe('var foo = 42;');
expect(renderer.printStatement(stmt2, sourceFile, importManager)).toBe('var bar = true;');
expect(renderer.printStatement(stmt3, sourceFile, importManager)).toBe('var baz = "qux";');
});
});
});
});
Expand Up @@ -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 {DeclareVarStmt, LiteralExpr, StmtModifier} from '@angular/compiler';
import MagicString from 'magic-string';
import * as ts from 'typescript';
import {NoopImportRewriter} from '../../../src/ngtsc/imports';
Expand Down Expand Up @@ -623,5 +624,19 @@ export { D };
static withProviders2(): (ModuleWithProviders)&{ngModule:ExternalModule};`);
});
});

describe('printStatement', () => {
it('should transpile code to ES2015', () => {
const {renderer, sourceFile, importManager} = setup([PROGRAM]);

const stmt1 = new DeclareVarStmt('foo', new LiteralExpr(42), null, [StmtModifier.Final]);
const stmt2 = new DeclareVarStmt('bar', new LiteralExpr(true));
const stmt3 = new DeclareVarStmt('baz', new LiteralExpr('qux'), undefined, []);

expect(renderer.printStatement(stmt1, sourceFile, importManager)).toBe('const foo = 42;');
expect(renderer.printStatement(stmt2, sourceFile, importManager)).toBe('var bar = true;');
expect(renderer.printStatement(stmt3, sourceFile, importManager)).toBe('var baz = "qux";');
});
});
});
});

0 comments on commit 1993cce

Please sign in to comment.