Skip to content

Commit 45ae14c

Browse files
vicbjasonaden
vicb
authored and
jasonaden
committed
fix(compiler): emits quoted keys only iff they are quoted in the original template
fixes #14292
1 parent 5ea9b62 commit 45ae14c

File tree

11 files changed

+76
-54
lines changed

11 files changed

+76
-54
lines changed

‎packages/compiler-cli/test/transformers/node_emitter_spec.ts

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ const sameModuleIdentifier = new o.ExternalReference(null, 'someLocalId', null);
2121

2222
const externalModuleIdentifier = new o.ExternalReference(anotherModuleUrl, 'someExternalId', null);
2323

24-
describe('TypeScriptEmitter', () => {
24+
describe('TypeScriptNodeEmitter', () => {
2525
let context: MockAotContext;
2626
let host: MockCompilerHost;
2727
let emitter: TypeScriptNodeEmitter;
@@ -167,15 +167,13 @@ describe('TypeScriptEmitter', () => {
167167
expect(emitStmt(o.literal(true).toStmt())).toEqual('true;');
168168
expect(emitStmt(o.literal('someStr').toStmt())).toEqual(`"someStr";`);
169169
expect(emitStmt(o.literalArr([o.literal(1)]).toStmt())).toEqual(`[1];`);
170-
expect(emitStmt(o.literalMap([['someKey', o.literal(1)]]).toStmt()))
171-
.toEqual(`({ someKey: 1 });`);
172-
});
173-
174-
it('should apply quotes to each entry within a map produced with literalMap when true', () => {
175-
expect(emitStmt(
176-
o.literalMap([['a', o.literal('a')], ['*', o.literal('star')]], null, true).toStmt())
170+
expect(emitStmt(o.literalMap([
171+
{key: 'someKey', value: o.literal(1), quoted: false},
172+
{key: 'a', value: o.literal('a'), quoted: false},
173+
{key: '*', value: o.literal('star'), quoted: true},
174+
]).toStmt())
177175
.replace(/\s+/gm, ''))
178-
.toEqual(`({"a":"a","*":"star"});`);
176+
.toEqual(`({someKey:1,a:"a","*":"star"});`);
179177
});
180178

181179
it('should support blank literals', () => {

‎packages/compiler/src/aot/summary_serializer.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -299,7 +299,7 @@ class ForJitSerializer {
299299
}
300300
visitStringMap(map: {[key: string]: any}, context: any): any {
301301
return new o.LiteralMapExpr(Object.keys(map).map(
302-
(key) => new o.LiteralMapEntry(key, visitValue(map[key], this, context))));
302+
(key) => new o.LiteralMapEntry(key, visitValue(map[key], this, context), false)));
303303
}
304304
visitPrimitive(value: any, context: any): any { return o.literal(value); }
305305
visitOther(value: any, context: any): any {

‎packages/compiler/src/compiler_util/expression_converter.ts

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,16 @@ export function convertActionBinding(
3535
// Note: no caching for literal arrays in actions.
3636
return (args: o.Expression[]) => o.literalArr(args);
3737
},
38-
createLiteralMapConverter: (keys: string[]) => {
38+
createLiteralMapConverter: (keys: {key: string, quoted: boolean}[]) => {
3939
// Note: no caching for literal maps in actions.
40-
return (args: o.Expression[]) =>
41-
o.literalMap(<[string, o.Expression][]>keys.map((key, i) => [key, args[i]]));
40+
return (values: o.Expression[]) => {
41+
const entries = keys.map((k, i) => ({
42+
key: k.key,
43+
value: values[i],
44+
quoted: k.quoted,
45+
}));
46+
return o.literalMap(entries);
47+
};
4248
},
4349
createPipeConverter: (name: string) => {
4450
throw new Error(`Illegal State: Actions are not allowed to contain pipes. Pipe: ${name}`);
@@ -71,7 +77,7 @@ export interface BuiltinConverter { (args: o.Expression[]): o.Expression; }
7177

7278
export interface BuiltinConverterFactory {
7379
createLiteralArrayConverter(argCount: number): BuiltinConverter;
74-
createLiteralMapConverter(keys: string[]): BuiltinConverter;
80+
createLiteralMapConverter(keys: {key: string, quoted: boolean}[]): BuiltinConverter;
7581
createPipeConverter(name: string, argCount: number): BuiltinConverter;
7682
}
7783

@@ -169,6 +175,7 @@ class _BuiltinAstConverter extends cdAst.AstTransformer {
169175
}
170176
visitLiteralMap(ast: cdAst.LiteralMap, context: any): any {
171177
const args = ast.values.map(ast => ast.visit(this, context));
178+
172179
return new BuiltinFunctionCall(
173180
ast.span, args, this._converterFactory.createLiteralMapConverter(ast.keys));
174181
}

‎packages/compiler/src/output/output_ast.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -476,7 +476,7 @@ export class LiteralArrayExpr extends Expression {
476476
}
477477

478478
export class LiteralMapEntry {
479-
constructor(public key: string, public value: Expression, public quoted: boolean = false) {}
479+
constructor(public key: string, public value: Expression, public quoted: boolean) {}
480480
}
481481

482482
export class LiteralMapExpr extends Expression {
@@ -831,7 +831,7 @@ export class AstTransformer implements StatementVisitor, ExpressionVisitor {
831831
visitLiteralMapExpr(ast: LiteralMapExpr, context: any): any {
832832
const entries = ast.entries.map(
833833
(entry): LiteralMapEntry => new LiteralMapEntry(
834-
entry.key, entry.value.visitExpression(this, context), entry.quoted, ));
834+
entry.key, entry.value.visitExpression(this, context), entry.quoted));
835835
const mapType = new MapType(ast.valueType, null);
836836
return this.transformExpr(new LiteralMapExpr(entries, mapType, ast.sourceSpan), context);
837837
}
@@ -1151,10 +1151,10 @@ export function literalArr(
11511151
}
11521152

11531153
export function literalMap(
1154-
values: [string, Expression][], type: MapType | null = null,
1155-
quoted: boolean = false): LiteralMapExpr {
1154+
values: {key: string, quoted: boolean, value: Expression}[],
1155+
type: MapType | null = null): LiteralMapExpr {
11561156
return new LiteralMapExpr(
1157-
values.map(entry => new LiteralMapEntry(entry[0], entry[1], quoted)), type, null);
1157+
values.map(e => new LiteralMapEntry(e.key, e.value, e.quoted)), type, null);
11581158
}
11591159

11601160
export function not(expr: Expression, sourceSpan?: ParseSourceSpan | null): NotExpr {

‎packages/compiler/src/output/output_interpreter.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -311,9 +311,8 @@ class StatementInterpreter implements o.StatementVisitor, o.ExpressionVisitor {
311311
return this.visitAllExpressions(ast.entries, ctx);
312312
}
313313
visitLiteralMapExpr(ast: o.LiteralMapExpr, ctx: _ExecutionContext): any {
314-
const result = {};
315-
ast.entries.forEach(
316-
(entry) => (result as any)[entry.key] = entry.value.visitExpression(this, ctx));
314+
const result: {[k: string]: any} = {};
315+
ast.entries.forEach(entry => result[entry.key] = entry.value.visitExpression(this, ctx));
317316
return result;
318317
}
319318
visitCommaExpr(ast: o.CommaExpr, context: any): any {

‎packages/compiler/src/output/output_jit.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ class JitEmitterVisitor extends AbstractJsEmitterVisitor {
5050

5151
createReturnStmt(ctx: EmitterVisitorContext) {
5252
const stmt = new o.ReturnStatement(new o.LiteralMapExpr(this._evalExportedVars.map(
53-
resultVar => new o.LiteralMapEntry(resultVar, o.variable(resultVar)))));
53+
resultVar => new o.LiteralMapEntry(resultVar, o.variable(resultVar), false))));
5454
stmt.visitStatement(this, ctx);
5555
}
5656

‎packages/compiler/src/view_compiler/view_compiler.ts

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,9 @@ export class ViewCompiler {
6161
outputCtx.statements.push(
6262
renderComponentVar
6363
.set(o.importExpr(Identifiers.createRendererType2).callFn([new o.LiteralMapExpr([
64-
new o.LiteralMapEntry('encapsulation', o.literal(template.encapsulation)),
65-
new o.LiteralMapEntry('styles', styles),
66-
new o.LiteralMapEntry('data', new o.LiteralMapExpr(customRenderData))
64+
new o.LiteralMapEntry('encapsulation', o.literal(template.encapsulation), false),
65+
new o.LiteralMapEntry('styles', styles, false),
66+
new o.LiteralMapEntry('data', new o.LiteralMapExpr(customRenderData), false)
6767
])]))
6868
.toDeclStmt(
6969
o.importType(Identifiers.RendererType2),
@@ -160,8 +160,8 @@ class ViewBuilder implements TemplateAstVisitor, LocalResolver {
160160
nodeFlags: flags,
161161
nodeDef: o.importExpr(Identifiers.queryDef).callFn([
162162
o.literal(flags), o.literal(queryId),
163-
new o.LiteralMapExpr(
164-
[new o.LiteralMapEntry(query.propertyName, o.literal(bindingType))])
163+
new o.LiteralMapExpr([new o.LiteralMapEntry(
164+
query.propertyName, o.literal(bindingType), false)])
165165
])
166166
}));
167167
});
@@ -504,8 +504,8 @@ class ViewBuilder implements TemplateAstVisitor, LocalResolver {
504504
nodeFlags: flags,
505505
nodeDef: o.importExpr(Identifiers.queryDef).callFn([
506506
o.literal(flags), o.literal(queryId),
507-
new o.LiteralMapExpr(
508-
[new o.LiteralMapEntry(query.propertyName, o.literal(bindingType))])
507+
new o.LiteralMapExpr([new o.LiteralMapEntry(
508+
query.propertyName, o.literal(bindingType), false)])
509509
]),
510510
}));
511511
});
@@ -705,23 +705,26 @@ class ViewBuilder implements TemplateAstVisitor, LocalResolver {
705705

706706
return (args: o.Expression[]) => callCheckStmt(nodeIndex, args);
707707
}
708-
createLiteralMapConverter(sourceSpan: ParseSourceSpan, keys: string[]): BuiltinConverter {
708+
709+
createLiteralMapConverter(sourceSpan: ParseSourceSpan, keys: {key: string, quoted: boolean}[]):
710+
BuiltinConverter {
709711
if (keys.length === 0) {
710712
const valueExpr = o.importExpr(Identifiers.EMPTY_MAP);
711713
return () => valueExpr;
712714
}
713715

716+
// function pureObjectDef(propToIndex: {[p: string]: number}): NodeDef
717+
const map = o.literalMap(keys.map((e, i) => ({...e, value: o.literal(i)})));
714718
const nodeIndex = this.nodes.length;
715-
// function pureObjectDef(propertyNames: string[]): NodeDef
716719
this.nodes.push(() => ({
717720
sourceSpan,
718721
nodeFlags: NodeFlags.TypePureObject,
719-
nodeDef: o.importExpr(Identifiers.pureObjectDef).callFn([o.literalArr(
720-
keys.map(key => o.literal(key)))])
722+
nodeDef: o.importExpr(Identifiers.pureObjectDef).callFn([map])
721723
}));
722724

723725
return (args: o.Expression[]) => callCheckStmt(nodeIndex, args);
724726
}
727+
725728
createPipeConverter(expression: UpdateExpression, name: string, argCount: number):
726729
BuiltinConverter {
727730
const pipe = this.usedPipes.find((pipeSummary) => pipeSummary.name === name) !;
@@ -795,7 +798,8 @@ class ViewBuilder implements TemplateAstVisitor, LocalResolver {
795798
createLiteralArrayConverter: (argCount: number) => this.createLiteralArrayConverter(
796799
expression.sourceSpan, argCount),
797800
createLiteralMapConverter:
798-
(keys: string[]) => this.createLiteralMapConverter(expression.sourceSpan, keys),
801+
(keys: {key: string, quoted: boolean}[]) =>
802+
this.createLiteralMapConverter(expression.sourceSpan, keys),
799803
createPipeConverter: (name: string, argCount: number) =>
800804
this.createPipeConverter(expression, name, argCount)
801805
},

‎packages/compiler/test/output/js_emitter_spec.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,13 @@ export function main() {
9999
expect(emitStmt(o.literal(true).toStmt())).toEqual('true;');
100100
expect(emitStmt(o.literal('someStr').toStmt())).toEqual(`'someStr';`);
101101
expect(emitStmt(o.literalArr([o.literal(1)]).toStmt())).toEqual(`[1];`);
102-
expect(emitStmt(o.literalMap([['someKey', o.literal(1)]]).toStmt())).toEqual(`{someKey:1};`);
102+
expect(emitStmt(o.literalMap([
103+
{key: 'someKey', value: o.literal(1), quoted: false},
104+
{key: 'a', value: o.literal('a'), quoted: false},
105+
{key: '*', value: o.literal('star'), quoted: true},
106+
]).toStmt())
107+
.replace(/\s+/gm, ''))
108+
.toEqual(`{someKey:1,a:'a','*':'star'};`);
103109
});
104110

105111
it('should break expressions into multiple lines if they are too long', () => {

‎packages/compiler/test/output/ts_emitter_spec.ts

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,13 @@ export function main() {
151151
expect(emitStmt(o.literal(true).toStmt())).toEqual('true;');
152152
expect(emitStmt(o.literal('someStr').toStmt())).toEqual(`'someStr';`);
153153
expect(emitStmt(o.literalArr([o.literal(1)]).toStmt())).toEqual(`[1];`);
154-
expect(emitStmt(o.literalMap([['someKey', o.literal(1)]]).toStmt())).toEqual(`{someKey:1};`);
154+
expect(emitStmt(o.literalMap([
155+
{key: 'someKey', value: o.literal(1), quoted: false},
156+
{key: 'a', value: o.literal('a'), quoted: false},
157+
{key: '*', value: o.literal('star'), quoted: true},
158+
]).toStmt())
159+
.replace(/\s+/gm, ''))
160+
.toEqual(`{someKey:1,a:'a','*':'star'};`);
155161
});
156162

157163
it('should break expressions into multiple lines if they are too long', () => {
@@ -166,14 +172,6 @@ export function main() {
166172
].join('\n'));
167173
});
168174

169-
it('should apply quotes to each entry within a map produced with literalMap when true', () => {
170-
expect(
171-
emitStmt(
172-
o.literalMap([['a', o.literal('a')], ['*', o.literal('star')]], null, true).toStmt())
173-
.replace(/\s+/gm, ''))
174-
.toEqual(`{'a':'a','*':'star'};`);
175-
});
176-
177175
it('should support blank literals', () => {
178176
expect(emitStmt(o.literal(null).toStmt())).toEqual('(null as any);');
179177
expect(emitStmt(o.literal(undefined).toStmt())).toEqual('(undefined as any);');

‎packages/core/src/view/pure_expression.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,16 @@ export function pureArrayDef(argCount: number): NodeDef {
1818
return _pureExpressionDef(NodeFlags.TypePureArray, new Array(argCount));
1919
}
2020

21-
export function pureObjectDef(propertyNames: string[]): NodeDef {
21+
export function pureObjectDef(propToIndex: {[p: string]: number}): NodeDef {
22+
const keys = Object.keys(propToIndex);
23+
const nbKeys = keys.length;
24+
const propertyNames = new Array(nbKeys);
25+
for (let i = 0; i < nbKeys; i++) {
26+
const key = keys[i];
27+
const index = propToIndex[key];
28+
propertyNames[index] = key;
29+
}
30+
2231
return _pureExpressionDef(NodeFlags.TypePureObject, propertyNames);
2332
}
2433

0 commit comments

Comments
 (0)