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 passing an extra hint to `translateExpression()`
and `translateStatement()` helper methods to translate the code to ES5
format if necessary.

Fixes angular#32665
  • Loading branch information
gkalpak committed Nov 4, 2019
1 parent 94eb709 commit e53463b
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 9 deletions.
2 changes: 1 addition & 1 deletion integration/ngcc/test.sh
Expand Up @@ -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)."


Expand Down
30 changes: 30 additions & 0 deletions packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts
Expand Up @@ -147,6 +147,36 @@ 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
26 changes: 18 additions & 8 deletions packages/compiler-cli/src/ngtsc/translator/src/translator.ts
Expand Up @@ -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 {
Expand All @@ -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(
Expand Down Expand Up @@ -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);
}

Expand Down

0 comments on commit e53463b

Please sign in to comment.