Skip to content

Commit

Permalink
fix(compiler-cli): generate let statements in ES2015+ mode (#38775)
Browse files Browse the repository at this point in the history 
When the target of the compiler is ES2015 or newer then we should
be generating `let` and `const` variable declarations rather than `var`.

PR Close #38775
  • Loading branch information
@petebacondarwin @mhevery
petebacondarwin authored and mhevery committed Sep 21, 2020
1 parent 6158dc1 commit 123bff7
Show file tree
Hide file tree
Showing 7 changed files with 48 additions and 44 deletions.
10 changes: 5 additions & 5 deletions packages/compiler-cli/ngcc/test/rendering/esm_rendering_formatter_spec.ts
View file
Expand Up @@ -69,7 +69,7 @@ B.decorators = [
{ type: OtherB },
{ type: Directive, args: [{ selector: '[b]' }] }
];
var C_1;
let C_1;
let C = C_1 = class C {};
C.decorators = [
{ type: Directive, args: [{ selector: '[c]' }] },
Expand Down Expand Up @@ -111,7 +111,7 @@ B.decorators = [
];
return B;
})();
var C_1;
let C_1;
let C = C_1 = /** @class */ (() => {
class C {}
C.decorators = [
Expand Down Expand Up @@ -432,7 +432,7 @@ A.decorators = [
name: _('/node_modules/test-package/some/file.js'),
contents: `
import * as tslib_1 from "tslib";
var D_1;
let D_1;
/* A copyright notice */
import { Directive } from '@angular/core';
const OtherA = () => (node) => { };
Expand Down Expand Up @@ -681,9 +681,9 @@ export { D };
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(stmt2, sourceFile, importManager)).toBe('let bar = true;');
expect(renderer.printStatement(stmt3, sourceFile, importManager))
.toBe('var baz = "qux";');
.toBe('let baz = "qux";');
});
});
});
Expand Down
9 changes: 6 additions & 3 deletions packages/compiler-cli/ngcc/test/rendering/renderer_spec.ts
View file
Expand Up @@ -32,6 +32,8 @@ import {getRootFiles, makeTestEntryPointBundle} from '../helpers/utils';
class TestRenderingFormatter implements RenderingFormatter {
private printer = ts.createPrinter({newLine: ts.NewLineKind.LineFeed});

constructor(private isEs5: boolean) {}

addImports(output: MagicString, imports: Import[], sf: ts.SourceFile) {
output.prepend('\n// ADD IMPORTS\n');
}
Expand Down Expand Up @@ -63,7 +65,8 @@ class TestRenderingFormatter implements RenderingFormatter {
}
printStatement(stmt: Statement, sourceFile: ts.SourceFile, importManager: ImportManager): string {
const node = translateStatement(
stmt, importManager, NOOP_DEFAULT_IMPORT_RECORDER, ts.ScriptTarget.ES2015);
stmt, importManager, NOOP_DEFAULT_IMPORT_RECORDER,
this.isEs5 ? ts.ScriptTarget.ES5 : ts.ScriptTarget.ES2015);
const code = this.printer.printNode(ts.EmitHint.Unspecified, node, sourceFile);

return `// TRANSPILED\n${code}`;
Expand Down Expand Up @@ -94,7 +97,7 @@ function createTestRenderer(
.analyzeProgram(bundle.src.program);
const privateDeclarationsAnalyses =
new PrivateDeclarationsAnalyzer(host, referencesRegistry).analyzeProgram(bundle.src.program);
const testFormatter = new TestRenderingFormatter();
const testFormatter = new TestRenderingFormatter(isEs5);
spyOn(testFormatter, 'addExports').and.callThrough();
spyOn(testFormatter, 'addImports').and.callThrough();
spyOn(testFormatter, 'addDefinitions').and.callThrough();
Expand Down Expand Up @@ -569,7 +572,7 @@ UndecoratedBase.ɵfac = function UndecoratedBase_Factory(t) { return new (t || U
UndecoratedBase.ɵdir = ɵngcc0.ɵɵdefineDirective({ type: UndecoratedBase, viewQuery: function UndecoratedBase_Query(rf, ctx) { if (rf & 1) {
ɵngcc0.ɵɵstaticViewQuery(_c0, true);
} if (rf & 2) {
var _t;
let _t;
ɵngcc0.ɵɵqueryRefresh(_t = ɵngcc0.ɵɵloadQuery()) && (ctx.test = _t.first);
} } });`);
});
Expand Down
7 changes: 4 additions & 3 deletions packages/compiler-cli/src/ngtsc/translator/src/translator.ts
View file
Expand Up @@ -134,15 +134,16 @@ class ExpressionTranslatorVisitor implements ExpressionVisitor, StatementVisitor
private scriptTarget: Exclude<ts.ScriptTarget, ts.ScriptTarget.JSON>) {}

visitDeclareVarStmt(stmt: DeclareVarStmt, context: Context): ts.VariableStatement {
const isConst =
this.scriptTarget >= ts.ScriptTarget.ES2015 && stmt.hasModifier(StmtModifier.Final);
const varType = this.scriptTarget < ts.ScriptTarget.ES2015 ?
ts.NodeFlags.None :
stmt.hasModifier(StmtModifier.Final) ? ts.NodeFlags.Const : ts.NodeFlags.Let;
const varDeclaration = ts.createVariableDeclaration(
/* name */ stmt.name,
/* type */ undefined,
/* initializer */ stmt.value?.visitExpression(this, context.withExpressionMode));
const declarationList = ts.createVariableDeclarationList(
/* declarations */[varDeclaration],
/* flags */ isConst ? ts.NodeFlags.Const : ts.NodeFlags.None);
/* flags */ varType);
const varStatement = ts.createVariableStatement(undefined, declarationList);
return attachComments(varStatement, stmt.leadingComments);
}
Expand Down
16 changes: 8 additions & 8 deletions packages/compiler-cli/test/compliance/r3_compiler_compliance_spec.ts
View file
Expand Up @@ -1638,7 +1638,7 @@ describe('compiler compliance', () => {
$r3$.ɵɵviewQuery(SomeDirective, true);
}
if (rf & 2) {
var $tmp$;
let $tmp$;
$r3$.ɵɵqueryRefresh($tmp$ = $r3$.ɵɵloadQuery()) && (ctx.someDir = $tmp$.first);
$r3$.ɵɵqueryRefresh($tmp$ = $r3$.ɵɵloadQuery()) && (ctx.someDirs = $tmp$);
}
Expand Down Expand Up @@ -1697,7 +1697,7 @@ describe('compiler compliance', () => {
$r3$.ɵɵviewQuery($e1_attrs$, true);
}
if (rf & 2) {
var $tmp$;
let $tmp$;
$r3$.ɵɵqueryRefresh($tmp$ = $r3$.ɵɵloadQuery()) && (ctx.myRef = $tmp$.first);
$r3$.ɵɵqueryRefresh($tmp$ = $r3$.ɵɵloadQuery()) && (ctx.myRefs = $tmp$);
}
Expand Down Expand Up @@ -1748,7 +1748,7 @@ describe('compiler compliance', () => {
$r3$.ɵɵviewQuery($refs$, true);
}
if (rf & 2) {
var $tmp$;
let $tmp$;
$r3$.ɵɵqueryRefresh($tmp$ = $r3$.ɵɵloadQuery()) && (ctx.someDir = $tmp$.first);
$r3$.ɵɵqueryRefresh($tmp$ = $r3$.ɵɵloadQuery()) && (ctx.foo = $tmp$.first);
}
Expand Down Expand Up @@ -1814,7 +1814,7 @@ describe('compiler compliance', () => {
$r3$.ɵɵviewQuery(SomeDirective, true, TemplateRef);
}
if (rf & 2) {
var $tmp$;
let $tmp$;
$r3$.ɵɵqueryRefresh($tmp$ = $r3$.ɵɵloadQuery()) && (ctx.myRef = $tmp$.first);
$r3$.ɵɵqueryRefresh($tmp$ = $r3$.ɵɵloadQuery()) && (ctx.someDir = $tmp$.first);
$r3$.ɵɵqueryRefresh($tmp$ = $r3$.ɵɵloadQuery()) && (ctx.myRefs = $tmp$);
Expand Down Expand Up @@ -1875,7 +1875,7 @@ describe('compiler compliance', () => {
$r3$.ɵɵcontentQuery(dirIndex, SomeDirective, false);
}
if (rf & 2) {
var $tmp$;
let $tmp$;
$r3$.ɵɵqueryRefresh($tmp$ = $r3$.ɵɵloadQuery()) && (ctx.someDir = $tmp$.first);
$r3$.ɵɵqueryRefresh($tmp$ = $r3$.ɵɵloadQuery()) && (ctx.someDirList = $tmp$);
}
Expand Down Expand Up @@ -1935,7 +1935,7 @@ describe('compiler compliance', () => {
$r3$.ɵɵcontentQuery(dirIndex, $e1_attrs$, false);
}
if (rf & 2) {
var $tmp$;
let $tmp$;
$r3$.ɵɵqueryRefresh($tmp$ = $r3$.ɵɵloadQuery()) && (ctx.myRef = $tmp$.first);
$r3$.ɵɵqueryRefresh($tmp$ = $r3$.ɵɵloadQuery()) && (ctx.myRefs = $tmp$);
}
Expand Down Expand Up @@ -1994,7 +1994,7 @@ describe('compiler compliance', () => {
$r3$.ɵɵcontentQuery(dirIndex, $ref0$, true);
}
if (rf & 2) {
var $tmp$;
let $tmp$;
$r3$.ɵɵqueryRefresh($tmp$ = $r3$.ɵɵloadQuery()) && (ctx.someDir = $tmp$.first);
$r3$.ɵɵqueryRefresh($tmp$ = $r3$.ɵɵloadQuery()) && (ctx.foo = $tmp$.first);
}
Expand Down Expand Up @@ -2061,7 +2061,7 @@ describe('compiler compliance', () => {
$r3$.ɵɵcontentQuery(dirIndex, SomeDirective, false, TemplateRef);
}
if (rf & 2) {
var $tmp$;
let $tmp$;
$r3$.ɵɵqueryRefresh($tmp$ = $r3$.ɵɵloadQuery()) && (ctx.myRef = $tmp$.first);
$r3$.ɵɵqueryRefresh($tmp$ = $r3$.ɵɵloadQuery()) && (ctx.someDir = $tmp$.first);
$r3$.ɵɵqueryRefresh($tmp$ = $r3$.ɵɵloadQuery()) && (ctx.myRefs = $tmp$);
Expand Down
4 changes: 2 additions & 2 deletions packages/compiler-cli/test/compliance/r3_view_compiler_binding_spec.ts
View file
Expand Up @@ -209,7 +209,7 @@ describe('compiler compliance: bindings', () => {
template: function MyComponent_Template(rf, ctx) {
…
if (rf & 2) {
var $tmp0$ = null;
let $tmp0$ = null;
$r3$.ɵɵproperty("title", ctx.myTitle)("id", ($tmp0$ = $r3$.ɵɵpipeBind1(1, 3, ($tmp0$ = ctx.auth()) == null ? null : $tmp0$.identity())) == null ? null : $tmp0$.id)("tabindex", 1);
}
}
Expand Down Expand Up @@ -750,7 +750,7 @@ describe('compiler compliance: bindings', () => {
hostVars: 1,
hostBindings: function HostBindingDir_HostBindings(rf, ctx) {
if (rf & 2) {
var $tmp0$ = null;
let $tmp0$ = null;
$r3$.ɵɵhostProperty("id", ($tmp0$ = ctx.getData()) == null ? null : $tmp0$.id);
}
}
Expand Down
4 changes: 2 additions & 2 deletions packages/compiler-cli/test/compliance/r3_view_compiler_di_spec.ts
View file
Expand Up @@ -212,7 +212,7 @@ describe('compiler compliance: dependency injection', () => {
MyService.ɵprov = $r3$.ɵɵdefineInjectable({
token: MyService,
factory: function MyService_Factory(t) {
var r = null;
let r = null;
if (t) {
r = new t();
} else {
Expand Down Expand Up @@ -285,7 +285,7 @@ describe('compiler compliance: dependency injection', () => {
MyService.ɵprov = $r3$.ɵɵdefineInjectable({
token: MyService,
factory: function MyService_Factory(t) {
var r = null;
let r = null;
if (t) {
r = new t();
} else {
Expand Down
42 changes: 21 additions & 21 deletions packages/compiler-cli/test/compliance/r3_view_compiler_i18n_spec.ts
View file
Expand Up @@ -251,7 +251,7 @@ const i18nMsg = (message: string, placeholders: Placeholder[] = [], meta?: Meta)
const closurePlaceholders = i18nPlaceholdersToString(placeholders);
const locMessageWithPlaceholders = i18nMsgInsertLocalizePlaceholders(message, placeholders);
return String.raw`
var ${varName};
let ${varName};
if (typeof ngI18nClosureMode !== "undefined" && ngI18nClosureMode) {
${i18nMsgClosureMeta(meta)}
const $MSG_EXTERNAL_${msgIndex}$ = goog.getMsg("${message}"${closurePlaceholders});
Expand Down Expand Up @@ -304,7 +304,7 @@ describe('i18n support in the template compiler', () => {

// Keeping this block as a raw string, since it checks escaping of special chars.
const i18n_6 = String.raw`
var $i18n_23$;
let $i18n_23$;
if (typeof ngI18nClosureMode !== "undefined" && ngI18nClosureMode) {
/**
* @desc [BACKUP_$` +
Expand All @@ -321,7 +321,7 @@ describe('i18n support in the template compiler', () => {

// Keeping this block as a raw string, since it checks escaping of special chars.
const i18n_7 = String.raw`
var $i18n_7$;
let $i18n_7$;
if (typeof ngI18nClosureMode !== "undefined" && ngI18nClosureMode) {
/**
* @desc Some text \' [BACKUP_MESSAGE_ID: xxx]
Expand Down Expand Up @@ -780,7 +780,7 @@ describe('i18n support in the template compiler', () => {
$r3$.ɵɵelementEnd();
}
if (rf & 2) {
var $tmp_0_0$ = null;
let $tmp_0_0$ = null;
$r3$.ɵɵi18nExp(($tmp_0_0$ = ctx.valueA.getRawValue()) == null ? null : $tmp_0_0$.getTitle());
$r3$.ɵɵi18nApply(1);
}
Expand Down Expand Up @@ -942,7 +942,7 @@ describe('i18n support in the template compiler', () => {
verify(input, output);
});

it('should sanitize ids and generate proper var names', () => {
it('should sanitize ids and generate proper variable names', () => {
const input = `
<div i18n="@@ID.WITH.INVALID.CHARS.2" i18n-title="@@ID.WITH.INVALID.CHARS" title="Element title">
Some content
Expand All @@ -951,7 +951,7 @@ describe('i18n support in the template compiler', () => {

// Keeping raw content (avoiding `i18nMsg`) to illustrate message id sanitization.
const output = String.raw`
var $I18N_0$;
let $I18N_0$;
if (typeof ngI18nClosureMode !== "undefined" && ngI18nClosureMode) {
const $MSG_EXTERNAL_ID_WITH_INVALID_CHARS$$APP_SPEC_TS_1$ = goog.getMsg("Element title");
$I18N_0$ = $MSG_EXTERNAL_ID_WITH_INVALID_CHARS$$APP_SPEC_TS_1$;
Expand All @@ -960,7 +960,7 @@ describe('i18n support in the template compiler', () => {
$I18N_0$ = $localize \`:@@ID.WITH.INVALID.CHARS:Element title\`;
}
…
var $I18N_2$;
let $I18N_2$;
if (typeof ngI18nClosureMode !== "undefined" && ngI18nClosureMode) {
const $MSG_EXTERNAL_ID_WITH_INVALID_CHARS_2$$APP_SPEC_TS_4$ = goog.getMsg(" Some content ");
$I18N_2$ = $MSG_EXTERNAL_ID_WITH_INVALID_CHARS_2$$APP_SPEC_TS_4$;
Expand Down Expand Up @@ -1018,7 +1018,7 @@ describe('i18n support in the template compiler', () => {

// Keeping raw content (avoiding `i18nMsg`) to illustrate quotes escaping.
const output = String.raw`
var $I18N_0$;
let $I18N_0$;
if (typeof ngI18nClosureMode !== "undefined" && ngI18nClosureMode) {
const $MSG_EXTERNAL_4924931801512133405$$APP_SPEC_TS_0$ = goog.getMsg("Some text 'with single quotes', \"with double quotes\", ` +
'`with backticks`' + String.raw` and without quotes.");
Expand All @@ -1036,7 +1036,7 @@ describe('i18n support in the template compiler', () => {
const input = '<div i18n>`{{ count }}`</div>';
// Keeping raw content (avoiding `i18nMsg`) to illustrate backticks escaping.
const output = String.raw`
var $I18N_0$;
let $I18N_0$;
if (typeof ngI18nClosureMode !== "undefined" && ngI18nClosureMode) {
const $MSG_APP_SPEC_TS_1$ = goog.getMsg("` +
'`{$interpolation}`' + String.raw`", { "interpolation": "\uFFFD0\uFFFD" });
Expand Down Expand Up @@ -1108,7 +1108,7 @@ describe('i18n support in the template compiler', () => {
// Keeping raw content (avoiding `i18nMsg`) to illustrate how named interpolations are
// generated.
const i18n_0 = String.raw`
var $I18N_0$;
let $I18N_0$;
if (typeof ngI18nClosureMode !== "undefined" && ngI18nClosureMode) {
const $MSG_EXTERNAL_7597881511811528589$$APP_SPEC_TS_0$ = goog.getMsg(" Named interpolation: {$phA} Named interpolation with spaces: {$phB} ", {
"phA": "\uFFFD0\uFFFD",
Expand Down Expand Up @@ -1209,7 +1209,7 @@ describe('i18n support in the template compiler', () => {
$r3$.ɵɵelementEnd();
}
if (rf & 2) {
var $tmp_2_0$ = null;
let $tmp_2_0$ = null;
$r3$.ɵɵadvance(2);
$r3$.ɵɵi18nExp($r3$.ɵɵpipeBind1(2, 3, ctx.valueA))
(ctx.valueA == null ? null : ctx.valueA.a == null ? null : ctx.valueA.a.b)
Expand Down Expand Up @@ -2487,7 +2487,7 @@ describe('i18n support in the template compiler', () => {
// Keeping raw content (avoiding `i18nMsg`) to illustrate message layout
// in case of whitespace preserving mode.
const i18n_0 = String.raw`
var $I18N_0$;
let $I18N_0$;
if (typeof ngI18nClosureMode !== "undefined" && ngI18nClosureMode) {
const $MSG_EXTERNAL_963542717423364282$$APP_SPEC_TS_0$ = goog.getMsg("\n Some text\n {$startTagSpan}Text inside span{$closeTagSpan}\n ", {
"startTagSpan": "\uFFFD#3\uFFFD",
Expand Down Expand Up @@ -2571,7 +2571,7 @@ describe('i18n support in the template compiler', () => {
`;

const output = String.raw`
var $I18N_0$;
let $I18N_0$;
if (typeof ngI18nClosureMode !== "undefined" && ngI18nClosureMode) {
const $MSG_EXTERNAL_4166854826696768832$$APP_SPEC_TS_0$ = goog.getMsg("{VAR_SELECT, select, single {'single quotes'} double {\"double quotes\"} other {other}}");
$I18N_0$ = $MSG_EXTERNAL_4166854826696768832$$APP_SPEC_TS_0$;
Expand Down Expand Up @@ -2914,7 +2914,7 @@ describe('i18n support in the template compiler', () => {
// Keeping raw content here to illustrate the difference in placeholders generated for
// goog.getMsg and $localize calls (see last i18n block).
const i18n_0 = String.raw`
var $I18N_1$;
let $I18N_1$;
if (typeof ngI18nClosureMode !== "undefined" && ngI18nClosureMode) {
const $MSG_APP_SPEC_TS_1$ = goog.getMsg("{VAR_SELECT, select, male {male} female {female} other {other}}");
$I18N_1$ = $MSG_APP_SPEC_TS_1$;
Expand All @@ -2925,7 +2925,7 @@ describe('i18n support in the template compiler', () => {
$I18N_1$ = $r3$.ɵɵi18nPostprocess($I18N_1$, {
"VAR_SELECT": "\uFFFD0\uFFFD"
});
var $I18N_2$;
let $I18N_2$;
if (typeof ngI18nClosureMode !== "undefined" && ngI18nClosureMode) {
const $MSG_APP_SPEC_TS_2$ = goog.getMsg("{VAR_SELECT, select, male {male} female {female} other {other}}");
$I18N_2$ = $MSG_APP_SPEC_TS_2$;
Expand All @@ -2936,7 +2936,7 @@ describe('i18n support in the template compiler', () => {
$I18N_2$ = $r3$.ɵɵi18nPostprocess($I18N_2$, {
"VAR_SELECT": "\uFFFD1\uFFFD"
});
var $I18N_4$;
let $I18N_4$;
if (typeof ngI18nClosureMode !== "undefined" && ngI18nClosureMode) {
const $MSG_APP_SPEC_TS__4$ = goog.getMsg("{VAR_SELECT, select, male {male} female {female} other {other}}");
$I18N_4$ = $MSG_APP_SPEC_TS__4$;
Expand All @@ -2947,7 +2947,7 @@ describe('i18n support in the template compiler', () => {
$I18N_4$ = $r3$.ɵɵi18nPostprocess($I18N_4$, {
"VAR_SELECT": "\uFFFD0:1\uFFFD"
});
var $I18N_0$;
let $I18N_0$;
if (typeof ngI18nClosureMode !== "undefined" && ngI18nClosureMode) {
const $MSG_APP_SPEC_TS_0$ = goog.getMsg(" {$icu} {$startTagDiv} {$icu} {$closeTagDiv}{$startTagDiv_1} {$icu} {$closeTagDiv}", {
"startTagDiv": "\uFFFD#2\uFFFD",
Expand Down Expand Up @@ -3345,7 +3345,7 @@ describe('i18n support in the template compiler', () => {
const input = `<div i18n>Some Message</div>`;

const output = String.raw`
var $I18N_0$;
let $I18N_0$;
if (typeof ngI18nClosureMode !== "undefined" && ngI18nClosureMode) { … }
else {
$I18N_0$ = $localize \`:␟ec93160d6d6a8822214060dd7938bf821c22b226␟6795333002533525253:Some Message\`;
Expand All @@ -3360,7 +3360,7 @@ describe('i18n support in the template compiler', () => {
const input = `<div i18n>Some Message</div>`;

const output = String.raw`
var $I18N_0$;
let $I18N_0$;
if (typeof ngI18nClosureMode !== "undefined" && ngI18nClosureMode) { … }
else {
$I18N_0$ = $localize \`:␟ec93160d6d6a8822214060dd7938bf821c22b226␟6795333002533525253:Some Message\`;
Expand Down Expand Up @@ -3523,7 +3523,7 @@ $` + String.raw`{$I18N_4$}:ICU:\`;
`;

const i18n_0 = String.raw`
var $I18N_0$;
let $I18N_0$;
if (typeof ngI18nClosureMode !== "undefined" && ngI18nClosureMode) {
const $MSG_EXTERNAL_7128002169381370313$$APP_SPEC_TS_1$ = goog.getMsg("{$startTagXhtmlDiv} Count: {$startTagXhtmlSpan}5{$closeTagXhtmlSpan}{$closeTagXhtmlDiv}", {
"startTagXhtmlDiv": "\uFFFD#3\uFFFD",
Expand Down Expand Up @@ -3584,7 +3584,7 @@ $` + String.raw`{$I18N_4$}:ICU:\`;
`;

const i18n_0 = String.raw`
var $I18N_0$;
let $I18N_0$;
if (typeof ngI18nClosureMode !== "undefined" && ngI18nClosureMode) {
const $MSG_EXTERNAL_7428861019045796010$$APP_SPEC_TS_1$ = goog.getMsg(" Count: {$startTagXhtmlSpan}5{$closeTagXhtmlSpan}", {
"startTagXhtmlSpan": "\uFFFD#4\uFFFD",
Expand Down

0 comments on commit 123bff7

Please sign in to comment.