Skip to content

Commit

Permalink
fix(ivy): record correct absolute source span for ngForOf expressions (
Browse files Browse the repository at this point in the history
…#31813)

Expressions in an inline template binding are improperly recorded as
spaning an offset calculated from the start of the template binding
attribute key, whereas they should be calculated from the start of the
attribute value, which contains the actual binding AST.

PR Close #31813
  • Loading branch information
ayazhafiz authored and kara committed Dec 16, 2019
1 parent c9172cf commit 931cb5e
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 10 deletions.
2 changes: 1 addition & 1 deletion packages/compiler/src/expression_parser/parser.ts
Expand Up @@ -764,7 +764,7 @@ export class _ParseAST {
const ast = this.parsePipe(); const ast = this.parsePipe();
const source = this.input.substring(start - this.offset, this.inputIndex - this.offset); const source = this.input.substring(start - this.offset, this.inputIndex - this.offset);
expression = expression =
new ASTWithSource(ast, source, this.location, this.absoluteOffset, this.errors); new ASTWithSource(ast, source, this.location, this.absoluteOffset + start, this.errors);
} }


bindings.push(new TemplateBinding( bindings.push(new TemplateBinding(
Expand Down
11 changes: 8 additions & 3 deletions packages/compiler/src/render3/r3_template_transform.ts
Expand Up @@ -151,10 +151,15 @@ class HtmlAstToIvyAst implements html.Visitor {
const templateKey = normalizedName.substring(TEMPLATE_ATTR_PREFIX.length); const templateKey = normalizedName.substring(TEMPLATE_ATTR_PREFIX.length);


const parsedVariables: ParsedVariable[] = []; const parsedVariables: ParsedVariable[] = [];
const absoluteOffset = attribute.valueSpan ? attribute.valueSpan.start.offset : const absoluteValueOffset = attribute.valueSpan ?
attribute.sourceSpan.start.offset; attribute.valueSpan.start.offset :
// If there is no value span the attribute does not have a value, like `attr` in
//`<div attr></div>`. In this case, point to one character beyond the last character of
// the attribute name.
attribute.sourceSpan.start.offset + attribute.name.length;

this.bindingParser.parseInlineTemplateBinding( this.bindingParser.parseInlineTemplateBinding(
templateKey, templateValue, attribute.sourceSpan, absoluteOffset, [], templateKey, templateValue, attribute.sourceSpan, absoluteValueOffset, [],
templateParsedProperties, parsedVariables); templateParsedProperties, parsedVariables);
templateVariables.push( templateVariables.push(
...parsedVariables.map(v => new t.Variable(v.name, v.value, v.sourceSpan))); ...parsedVariables.map(v => new t.Variable(v.name, v.value, v.sourceSpan)));
Expand Down
30 changes: 24 additions & 6 deletions packages/compiler/src/template_parser/binding_parser.ts
Expand Up @@ -113,12 +113,22 @@ export class BindingParser {
} }
} }


// Parse an inline template binding. ie `<tag *tplKey="<tplValue>">` /**
* Parses an inline template binding, e.g.
* <tag *tplKey="<tplValue>">
* @param tplKey template binding name
* @param tplValue template binding value
* @param sourceSpan span of template binding relative to entire the template
* @param absoluteValueOffset start of the tplValue relative to the entire template
* @param targetMatchableAttrs potential attributes to match in the template
* @param targetProps target property bindings in the template
* @param targetVars target variables in the template
*/
parseInlineTemplateBinding( parseInlineTemplateBinding(
tplKey: string, tplValue: string, sourceSpan: ParseSourceSpan, absoluteOffset: number, tplKey: string, tplValue: string, sourceSpan: ParseSourceSpan, absoluteValueOffset: number,
targetMatchableAttrs: string[][], targetProps: ParsedProperty[], targetMatchableAttrs: string[][], targetProps: ParsedProperty[],
targetVars: ParsedVariable[]) { targetVars: ParsedVariable[]) {
const bindings = this._parseTemplateBindings(tplKey, tplValue, sourceSpan, absoluteOffset); const bindings = this._parseTemplateBindings(tplKey, tplValue, sourceSpan, absoluteValueOffset);


for (let i = 0; i < bindings.length; i++) { for (let i = 0; i < bindings.length; i++) {
const binding = bindings[i]; const binding = bindings[i];
Expand All @@ -131,20 +141,28 @@ export class BindingParser {
} else { } else {
targetMatchableAttrs.push([binding.key, '']); targetMatchableAttrs.push([binding.key, '']);
this.parseLiteralAttr( this.parseLiteralAttr(
binding.key, null, sourceSpan, absoluteOffset, undefined, targetMatchableAttrs, binding.key, null, sourceSpan, absoluteValueOffset, undefined, targetMatchableAttrs,
targetProps); targetProps);
} }
} }
} }


/**
* Parses the bindings in an inline template binding, e.g.
* <tag *tplKey="let value1 = prop; let value2 = localVar">
* @param tplKey template binding name
* @param tplValue template binding value
* @param sourceSpan span of template binding relative to entire the template
* @param absoluteValueOffset start of the tplValue relative to the entire template
*/
private _parseTemplateBindings( private _parseTemplateBindings(
tplKey: string, tplValue: string, sourceSpan: ParseSourceSpan, tplKey: string, tplValue: string, sourceSpan: ParseSourceSpan,
absoluteOffset: number): TemplateBinding[] { absoluteValueOffset: number): TemplateBinding[] {
const sourceInfo = sourceSpan.start.toString(); const sourceInfo = sourceSpan.start.toString();


try { try {
const bindingsResult = const bindingsResult =
this._exprParser.parseTemplateBindings(tplKey, tplValue, sourceInfo, absoluteOffset); this._exprParser.parseTemplateBindings(tplKey, tplValue, sourceInfo, absoluteValueOffset);
this._reportExpressionParserErrors(bindingsResult.errors, sourceSpan); this._reportExpressionParserErrors(bindingsResult.errors, sourceSpan);
bindingsResult.templateBindings.forEach((binding) => { bindingsResult.templateBindings.forEach((binding) => {
if (binding.expression) { if (binding.expression) {
Expand Down
14 changes: 14 additions & 0 deletions packages/compiler/test/render3/r3_ast_absolute_span_spec.ts
Expand Up @@ -322,4 +322,18 @@ describe('expression AST absolute source spans', () => {
'a:b', new AbsoluteSourceSpan(13, 16) 'a:b', new AbsoluteSourceSpan(13, 16)
]); ]);
}); });

describe('absolute offsets for template expressions', () => {
it('should work for simple cases', () => {
expect(
humanizeExpressionSource(parse('<div *ngFor="let item of items">{{item}}</div>').nodes))
.toContain(['items', new AbsoluteSourceSpan(25, 30)]);
});

it('should work with multiple bindings', () => {
expect(humanizeExpressionSource(parse('<div *ngFor="let a of As; let b of Bs"></div>').nodes))
.toEqual(jasmine.arrayContaining(
[['As', new AbsoluteSourceSpan(22, 24)], ['Bs', new AbsoluteSourceSpan(35, 37)]]));
});
});
}); });

0 comments on commit 931cb5e

Please sign in to comment.