From 543c80d3627a4fed5f31d82a101143c6849ec3c0 Mon Sep 17 00:00:00 2001 From: George Kalpakas Date: Mon, 4 Nov 2019 15:45:06 +0200 Subject: [PATCH] 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 passing an extra hint to `translateExpression()` and `translateStatement()` helper methods to translate the code to ES5 format if necessary. Fixes #32665 --- integration/ngcc/test.sh | 2 +- .../ngcc/test/integration/ngcc_spec.ts | 30 +++++++++++++++++++ .../src/ngtsc/translator/src/translator.ts | 26 +++++++++++----- 3 files changed, 49 insertions(+), 9 deletions(-) diff --git a/integration/ngcc/test.sh b/integration/ngcc/test.sh index da360842fa1094..5280cc5affa992 100755 --- a/integration/ngcc/test.sh +++ b/integration/ngcc/test.sh @@ -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)." diff --git a/packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts b/packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts index 3cb1610825520d..5917de2784249d 100644 --- a/packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts +++ b/packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts @@ -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({ diff --git a/packages/compiler-cli/src/ngtsc/translator/src/translator.ts b/packages/compiler-cli/src/ngtsc/translator/src/translator.ts index 6b9b39314a3041..0d6b5679bc632c 100644 --- a/packages/compiler-cli/src/ngtsc/translator/src/translator.ts +++ b/packages/compiler-cli/src/ngtsc/translator/src/translator.ts @@ -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(); 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); }