fix(compiler): emits quoted keys only iff they are quoted in the original template

fixes #14292
This commit is contained in:
Victor Berchet 2017-07-05 14:51:39 -07:00 committed by Jason Aden
parent 798947efa4
commit 9c1f6fd06f
11 changed files with 77 additions and 55 deletions

View File

@ -21,7 +21,7 @@ const sameModuleIdentifier = new o.ExternalReference(null, 'someLocalId', null);
const externalModuleIdentifier = new o.ExternalReference(anotherModuleUrl, 'someExternalId', null); const externalModuleIdentifier = new o.ExternalReference(anotherModuleUrl, 'someExternalId', null);
describe('TypeScriptEmitter', () => { describe('TypeScriptNodeEmitter', () => {
let context: MockAotContext; let context: MockAotContext;
let host: MockCompilerHost; let host: MockCompilerHost;
let emitter: TypeScriptNodeEmitter; let emitter: TypeScriptNodeEmitter;
@ -166,15 +166,13 @@ describe('TypeScriptEmitter', () => {
expect(emitStmt(o.literal(true).toStmt())).toEqual('true;'); expect(emitStmt(o.literal(true).toStmt())).toEqual('true;');
expect(emitStmt(o.literal('someStr').toStmt())).toEqual(`"someStr";`); expect(emitStmt(o.literal('someStr').toStmt())).toEqual(`"someStr";`);
expect(emitStmt(o.literalArr([o.literal(1)]).toStmt())).toEqual(`[1];`); expect(emitStmt(o.literalArr([o.literal(1)]).toStmt())).toEqual(`[1];`);
expect(emitStmt(o.literalMap([['someKey', o.literal(1)]]).toStmt())) expect(emitStmt(o.literalMap([
.toEqual(`({ someKey: 1 });`); {key: 'someKey', value: o.literal(1), quoted: false},
}); {key: 'a', value: o.literal('a'), quoted: false},
{key: '*', value: o.literal('star'), quoted: true},
it('should apply quotes to each entry within a map produced with literalMap when true', () => { ]).toStmt())
expect(emitStmt(
o.literalMap([['a', o.literal('a')], ['*', o.literal('star')]], null, true).toStmt())
.replace(/\s+/gm, '')) .replace(/\s+/gm, ''))
.toEqual(`({"a":"a","*":"star"});`); .toEqual(`({someKey:1,a:"a","*":"star"});`);
}); });
it('should support blank literals', () => { it('should support blank literals', () => {

View File

@ -298,7 +298,7 @@ class ForJitSerializer {
} }
visitStringMap(map: {[key: string]: any}, context: any): any { visitStringMap(map: {[key: string]: any}, context: any): any {
return new o.LiteralMapExpr(Object.keys(map).map( return new o.LiteralMapExpr(Object.keys(map).map(
(key) => new o.LiteralMapEntry(key, visitValue(map[key], this, context)))); (key) => new o.LiteralMapEntry(key, visitValue(map[key], this, context), false)));
} }
visitPrimitive(value: any, context: any): any { return o.literal(value); } visitPrimitive(value: any, context: any): any { return o.literal(value); }
visitOther(value: any, context: any): any { visitOther(value: any, context: any): any {

View File

@ -35,10 +35,16 @@ export function convertActionBinding(
// Note: no caching for literal arrays in actions. // Note: no caching for literal arrays in actions.
return (args: o.Expression[]) => o.literalArr(args); return (args: o.Expression[]) => o.literalArr(args);
}, },
createLiteralMapConverter: (keys: string[]) => { createLiteralMapConverter: (keys: {key: string, quoted: boolean}[]) => {
// Note: no caching for literal maps in actions. // Note: no caching for literal maps in actions.
return (args: o.Expression[]) => return (values: o.Expression[]) => {
o.literalMap(<[string, o.Expression][]>keys.map((key, i) => [key, args[i]])); const entries = keys.map((k, i) => ({
key: k.key,
value: values[i],
quoted: k.quoted,
}));
return o.literalMap(entries);
};
}, },
createPipeConverter: (name: string) => { createPipeConverter: (name: string) => {
throw new Error(`Illegal State: Actions are not allowed to contain pipes. Pipe: ${name}`); 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; }
export interface BuiltinConverterFactory { export interface BuiltinConverterFactory {
createLiteralArrayConverter(argCount: number): BuiltinConverter; createLiteralArrayConverter(argCount: number): BuiltinConverter;
createLiteralMapConverter(keys: string[]): BuiltinConverter; createLiteralMapConverter(keys: {key: string, quoted: boolean}[]): BuiltinConverter;
createPipeConverter(name: string, argCount: number): BuiltinConverter; createPipeConverter(name: string, argCount: number): BuiltinConverter;
} }
@ -169,8 +175,9 @@ class _BuiltinAstConverter extends cdAst.AstTransformer {
} }
visitLiteralMap(ast: cdAst.LiteralMap, context: any): any { visitLiteralMap(ast: cdAst.LiteralMap, context: any): any {
const args = ast.values.map(ast => ast.visit(this, context)); const args = ast.values.map(ast => ast.visit(this, context));
return new BuiltinFunctionCall( return new BuiltinFunctionCall(
ast.span, args, this._converterFactory.createLiteralMapConverter(ast.keys.map(k => k.key))); ast.span, args, this._converterFactory.createLiteralMapConverter(ast.keys));
} }
} }

View File

@ -476,7 +476,7 @@ export class LiteralArrayExpr extends Expression {
} }
export class LiteralMapEntry { export class LiteralMapEntry {
constructor(public key: string, public value: Expression, public quoted: boolean = false) {} constructor(public key: string, public value: Expression, public quoted: boolean) {}
} }
export class LiteralMapExpr extends Expression { export class LiteralMapExpr extends Expression {
@ -831,7 +831,7 @@ export class AstTransformer implements StatementVisitor, ExpressionVisitor {
visitLiteralMapExpr(ast: LiteralMapExpr, context: any): any { visitLiteralMapExpr(ast: LiteralMapExpr, context: any): any {
const entries = ast.entries.map( const entries = ast.entries.map(
(entry): LiteralMapEntry => new LiteralMapEntry( (entry): LiteralMapEntry => new LiteralMapEntry(
entry.key, entry.value.visitExpression(this, context), entry.quoted, )); entry.key, entry.value.visitExpression(this, context), entry.quoted));
const mapType = new MapType(ast.valueType, null); const mapType = new MapType(ast.valueType, null);
return this.transformExpr(new LiteralMapExpr(entries, mapType, ast.sourceSpan), context); return this.transformExpr(new LiteralMapExpr(entries, mapType, ast.sourceSpan), context);
} }
@ -1151,10 +1151,10 @@ export function literalArr(
} }
export function literalMap( export function literalMap(
values: [string, Expression][], type: MapType | null = null, values: {key: string, quoted: boolean, value: Expression}[],
quoted: boolean = false): LiteralMapExpr { type: MapType | null = null): LiteralMapExpr {
return new LiteralMapExpr( return new LiteralMapExpr(
values.map(entry => new LiteralMapEntry(entry[0], entry[1], quoted)), type, null); values.map(e => new LiteralMapEntry(e.key, e.value, e.quoted)), type, null);
} }
export function not(expr: Expression, sourceSpan?: ParseSourceSpan | null): NotExpr { export function not(expr: Expression, sourceSpan?: ParseSourceSpan | null): NotExpr {

View File

@ -311,9 +311,8 @@ class StatementInterpreter implements o.StatementVisitor, o.ExpressionVisitor {
return this.visitAllExpressions(ast.entries, ctx); return this.visitAllExpressions(ast.entries, ctx);
} }
visitLiteralMapExpr(ast: o.LiteralMapExpr, ctx: _ExecutionContext): any { visitLiteralMapExpr(ast: o.LiteralMapExpr, ctx: _ExecutionContext): any {
const result = {}; const result: {[k: string]: any} = {};
ast.entries.forEach( ast.entries.forEach(entry => result[entry.key] = entry.value.visitExpression(this, ctx));
(entry) => (result as any)[entry.key] = entry.value.visitExpression(this, ctx));
return result; return result;
} }
visitCommaExpr(ast: o.CommaExpr, context: any): any { visitCommaExpr(ast: o.CommaExpr, context: any): any {

View File

@ -50,7 +50,7 @@ class JitEmitterVisitor extends AbstractJsEmitterVisitor {
createReturnStmt(ctx: EmitterVisitorContext) { createReturnStmt(ctx: EmitterVisitorContext) {
const stmt = new o.ReturnStatement(new o.LiteralMapExpr(this._evalExportedVars.map( const stmt = new o.ReturnStatement(new o.LiteralMapExpr(this._evalExportedVars.map(
resultVar => new o.LiteralMapEntry(resultVar, o.variable(resultVar))))); resultVar => new o.LiteralMapEntry(resultVar, o.variable(resultVar), false))));
stmt.visitStatement(this, ctx); stmt.visitStatement(this, ctx);
} }

View File

@ -61,9 +61,9 @@ export class ViewCompiler {
outputCtx.statements.push( outputCtx.statements.push(
renderComponentVar renderComponentVar
.set(o.importExpr(Identifiers.createRendererType2).callFn([new o.LiteralMapExpr([ .set(o.importExpr(Identifiers.createRendererType2).callFn([new o.LiteralMapExpr([
new o.LiteralMapEntry('encapsulation', o.literal(template.encapsulation)), new o.LiteralMapEntry('encapsulation', o.literal(template.encapsulation), false),
new o.LiteralMapEntry('styles', styles), new o.LiteralMapEntry('styles', styles, false),
new o.LiteralMapEntry('data', new o.LiteralMapExpr(customRenderData)) new o.LiteralMapEntry('data', new o.LiteralMapExpr(customRenderData), false)
])])) ])]))
.toDeclStmt( .toDeclStmt(
o.importType(Identifiers.RendererType2), o.importType(Identifiers.RendererType2),
@ -160,8 +160,8 @@ class ViewBuilder implements TemplateAstVisitor, LocalResolver {
nodeFlags: flags, nodeFlags: flags,
nodeDef: o.importExpr(Identifiers.queryDef).callFn([ nodeDef: o.importExpr(Identifiers.queryDef).callFn([
o.literal(flags), o.literal(queryId), o.literal(flags), o.literal(queryId),
new o.LiteralMapExpr( new o.LiteralMapExpr([new o.LiteralMapEntry(
[new o.LiteralMapEntry(query.propertyName, o.literal(bindingType))]) query.propertyName, o.literal(bindingType), false)])
]) ])
})); }));
}); });
@ -504,8 +504,8 @@ class ViewBuilder implements TemplateAstVisitor, LocalResolver {
nodeFlags: flags, nodeFlags: flags,
nodeDef: o.importExpr(Identifiers.queryDef).callFn([ nodeDef: o.importExpr(Identifiers.queryDef).callFn([
o.literal(flags), o.literal(queryId), o.literal(flags), o.literal(queryId),
new o.LiteralMapExpr( new o.LiteralMapExpr([new o.LiteralMapEntry(
[new o.LiteralMapEntry(query.propertyName, o.literal(bindingType))]) query.propertyName, o.literal(bindingType), false)])
]), ]),
})); }));
}); });
@ -705,23 +705,26 @@ class ViewBuilder implements TemplateAstVisitor, LocalResolver {
return (args: o.Expression[]) => callCheckStmt(nodeIndex, args); return (args: o.Expression[]) => callCheckStmt(nodeIndex, args);
} }
createLiteralMapConverter(sourceSpan: ParseSourceSpan, keys: string[]): BuiltinConverter {
createLiteralMapConverter(sourceSpan: ParseSourceSpan, keys: {key: string, quoted: boolean}[]):
BuiltinConverter {
if (keys.length === 0) { if (keys.length === 0) {
const valueExpr = o.importExpr(Identifiers.EMPTY_MAP); const valueExpr = o.importExpr(Identifiers.EMPTY_MAP);
return () => valueExpr; return () => valueExpr;
} }
// function pureObjectDef(propToIndex: {[p: string]: number}): NodeDef
const map = o.literalMap(keys.map((e, i) => ({...e, value: o.literal(i)})));
const nodeIndex = this.nodes.length; const nodeIndex = this.nodes.length;
// function pureObjectDef(propertyNames: string[]): NodeDef
this.nodes.push(() => ({ this.nodes.push(() => ({
sourceSpan, sourceSpan,
nodeFlags: NodeFlags.TypePureObject, nodeFlags: NodeFlags.TypePureObject,
nodeDef: o.importExpr(Identifiers.pureObjectDef).callFn([o.literalArr( nodeDef: o.importExpr(Identifiers.pureObjectDef).callFn([map])
keys.map(key => o.literal(key)))])
})); }));
return (args: o.Expression[]) => callCheckStmt(nodeIndex, args); return (args: o.Expression[]) => callCheckStmt(nodeIndex, args);
} }
createPipeConverter(expression: UpdateExpression, name: string, argCount: number): createPipeConverter(expression: UpdateExpression, name: string, argCount: number):
BuiltinConverter { BuiltinConverter {
const pipe = this.usedPipes.find((pipeSummary) => pipeSummary.name === name) !; const pipe = this.usedPipes.find((pipeSummary) => pipeSummary.name === name) !;
@ -795,7 +798,8 @@ class ViewBuilder implements TemplateAstVisitor, LocalResolver {
createLiteralArrayConverter: (argCount: number) => this.createLiteralArrayConverter( createLiteralArrayConverter: (argCount: number) => this.createLiteralArrayConverter(
expression.sourceSpan, argCount), expression.sourceSpan, argCount),
createLiteralMapConverter: createLiteralMapConverter:
(keys: string[]) => this.createLiteralMapConverter(expression.sourceSpan, keys), (keys: {key: string, quoted: boolean}[]) =>
this.createLiteralMapConverter(expression.sourceSpan, keys),
createPipeConverter: (name: string, argCount: number) => createPipeConverter: (name: string, argCount: number) =>
this.createPipeConverter(expression, name, argCount) this.createPipeConverter(expression, name, argCount)
}, },

View File

@ -99,7 +99,13 @@ export function main() {
expect(emitStmt(o.literal(true).toStmt())).toEqual('true;'); expect(emitStmt(o.literal(true).toStmt())).toEqual('true;');
expect(emitStmt(o.literal('someStr').toStmt())).toEqual(`'someStr';`); expect(emitStmt(o.literal('someStr').toStmt())).toEqual(`'someStr';`);
expect(emitStmt(o.literalArr([o.literal(1)]).toStmt())).toEqual(`[1];`); expect(emitStmt(o.literalArr([o.literal(1)]).toStmt())).toEqual(`[1];`);
expect(emitStmt(o.literalMap([['someKey', o.literal(1)]]).toStmt())).toEqual(`{someKey:1};`); expect(emitStmt(o.literalMap([
{key: 'someKey', value: o.literal(1), quoted: false},
{key: 'a', value: o.literal('a'), quoted: false},
{key: '*', value: o.literal('star'), quoted: true},
]).toStmt())
.replace(/\s+/gm, ''))
.toEqual(`{someKey:1,a:'a','*':'star'};`);
}); });
it('should break expressions into multiple lines if they are too long', () => { it('should break expressions into multiple lines if they are too long', () => {

View File

@ -151,7 +151,13 @@ export function main() {
expect(emitStmt(o.literal(true).toStmt())).toEqual('true;'); expect(emitStmt(o.literal(true).toStmt())).toEqual('true;');
expect(emitStmt(o.literal('someStr').toStmt())).toEqual(`'someStr';`); expect(emitStmt(o.literal('someStr').toStmt())).toEqual(`'someStr';`);
expect(emitStmt(o.literalArr([o.literal(1)]).toStmt())).toEqual(`[1];`); expect(emitStmt(o.literalArr([o.literal(1)]).toStmt())).toEqual(`[1];`);
expect(emitStmt(o.literalMap([['someKey', o.literal(1)]]).toStmt())).toEqual(`{someKey:1};`); expect(emitStmt(o.literalMap([
{key: 'someKey', value: o.literal(1), quoted: false},
{key: 'a', value: o.literal('a'), quoted: false},
{key: '*', value: o.literal('star'), quoted: true},
]).toStmt())
.replace(/\s+/gm, ''))
.toEqual(`{someKey:1,a:'a','*':'star'};`);
}); });
it('should break expressions into multiple lines if they are too long', () => { it('should break expressions into multiple lines if they are too long', () => {
@ -166,14 +172,6 @@ export function main() {
].join('\n')); ].join('\n'));
}); });
it('should apply quotes to each entry within a map produced with literalMap when true', () => {
expect(
emitStmt(
o.literalMap([['a', o.literal('a')], ['*', o.literal('star')]], null, true).toStmt())
.replace(/\s+/gm, ''))
.toEqual(`{'a':'a','*':'star'};`);
});
it('should support blank literals', () => { it('should support blank literals', () => {
expect(emitStmt(o.literal(null).toStmt())).toEqual('(null as any);'); expect(emitStmt(o.literal(null).toStmt())).toEqual('(null as any);');
expect(emitStmt(o.literal(undefined).toStmt())).toEqual('(undefined as any);'); expect(emitStmt(o.literal(undefined).toStmt())).toEqual('(undefined as any);');

View File

@ -18,7 +18,16 @@ export function pureArrayDef(argCount: number): NodeDef {
return _pureExpressionDef(NodeFlags.TypePureArray, new Array(argCount)); return _pureExpressionDef(NodeFlags.TypePureArray, new Array(argCount));
} }
export function pureObjectDef(propertyNames: string[]): NodeDef { export function pureObjectDef(propToIndex: {[p: string]: number}): NodeDef {
const keys = Object.keys(propToIndex);
const nbKeys = keys.length;
const propertyNames = new Array(nbKeys);
for (let i = 0; i < nbKeys; i++) {
const key = keys[i];
const index = propToIndex[key];
propertyNames[index] = key;
}
return _pureExpressionDef(NodeFlags.TypePureObject, propertyNames); return _pureExpressionDef(NodeFlags.TypePureObject, propertyNames);
} }

View File

@ -6,9 +6,8 @@
* found in the LICENSE file at https://angular.io/license * found in the LICENSE file at https://angular.io/license
*/ */
import {Injector, PipeTransform, RenderComponentType, RootRenderer, Sanitizer, SecurityContext, ViewEncapsulation, WrappedValue} from '@angular/core'; import {PipeTransform} from '@angular/core';
import {ArgumentType, NodeDef, NodeFlags, RootData, Services, ViewData, ViewDefinition, ViewFlags, ViewHandleEventFn, ViewUpdateFn, anchorDef, asProviderData, asPureExpressionData, directiveDef, elementDef, nodeValue, pipeDef, pureArrayDef, pureObjectDef, purePipeDef, rootRenderNodes, textDef, viewDef} from '@angular/core/src/view/index'; import {NodeDef, NodeFlags, Services, ViewData, ViewDefinition, ViewFlags, ViewUpdateFn, asProviderData, directiveDef, elementDef, nodeValue, pipeDef, pureArrayDef, pureObjectDef, purePipeDef, rootRenderNodes, viewDef} from '@angular/core/src/view/index';
import {inject} from '@angular/core/testing';
import {ARG_TYPE_VALUES, checkNodeInlineOrDynamic, createRootView} from './helper'; import {ARG_TYPE_VALUES, checkNodeInlineOrDynamic, createRootView} from './helper';
@ -38,8 +37,9 @@ export function main() {
const {view, rootNodes} = createAndGetRootNodes(compViewDef( const {view, rootNodes} = createAndGetRootNodes(compViewDef(
[ [
elementDef(NodeFlags.None, null !, null !, 2, 'span'), pureArrayDef(2), elementDef(NodeFlags.None, null !, null !, 2, 'span'),
directiveDef(NodeFlags.None, null !, 0, Service, [], {data: [0, 'data']}) pureArrayDef(2),
directiveDef(NodeFlags.None, null !, 0, Service, [], {data: [0, 'data']}),
], ],
(check, view) => { (check, view) => {
const pureValue = checkNodeInlineOrDynamic(check, view, 1, inlineDynamic, values); const pureValue = checkNodeInlineOrDynamic(check, view, 1, inlineDynamic, values);
@ -75,7 +75,7 @@ export function main() {
const {view, rootNodes} = createAndGetRootNodes(compViewDef( const {view, rootNodes} = createAndGetRootNodes(compViewDef(
[ [
elementDef(NodeFlags.None, null !, null !, 2, 'span'), pureObjectDef(['a', 'b']), elementDef(NodeFlags.None, null !, null !, 2, 'span'), pureObjectDef({a: 0, b: 1}),
directiveDef(NodeFlags.None, null !, 0, Service, [], {data: [0, 'data']}) directiveDef(NodeFlags.None, null !, 0, Service, [], {data: [0, 'data']})
], ],
(check, view) => { (check, view) => {
@ -116,8 +116,9 @@ export function main() {
const {view, rootNodes} = createAndGetRootNodes(compViewDef( const {view, rootNodes} = createAndGetRootNodes(compViewDef(
[ [
elementDef(NodeFlags.None, null !, null !, 3, 'span'), elementDef(NodeFlags.None, null !, null !, 3, 'span'),
pipeDef(NodeFlags.None, SomePipe, []), purePipeDef(2), pipeDef(NodeFlags.None, SomePipe, []),
directiveDef(NodeFlags.None, null !, 0, Service, [], {data: [0, 'data']}) purePipeDef(2),
directiveDef(NodeFlags.None, null !, 0, Service, [], {data: [0, 'data']}),
], ],
(check, view) => { (check, view) => {
const pureValue = checkNodeInlineOrDynamic( const pureValue = checkNodeInlineOrDynamic(