refactor(compiler): distinguish two-way bindings in the AST (#54065) · nikvarma/angular@28bcae3 · GitHub
Skip to content

Commit 28bcae3

Browse files
crisbetonikvarma
crisbeto
authored and
nikvarma
committed
refactor(compiler): distinguish two-way bindings in the AST (angular#54065)
During the template parsing stage two-way bindings are split up into a property and event binding. All the downstream code treats these binding the same as their one-way equivalents. For some future work we'll have to distinguish between the two so these changes update the `BoundElementProperty.type` and `ParsedEvent.type` to include a `TwoWay` type. All existing call-sites have been updated to treat `TwoWay` the same as `Property`/`Regular`, but more specialized logic will be added in the future. PR Close angular#54065
1 parent 5521ccf commit 28bcae3

8 files changed

Lines changed: 71 additions & 50 deletions

File tree

‎packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts‎

Lines changed: 19 additions & 13 deletions

‎packages/compiler/src/expression_parser/ast.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -842,14 +842,17 @@ export class ParsedProperty {
842842
export enum ParsedPropertyType {
843843
DEFAULT,
844844
LITERAL_ATTR,
845-
ANIMATION
845+
ANIMATION,
846+
TWO_WAY,
846847
}
847848

848849
export const enum ParsedEventType {
849850
// DOM or Directive event
850851
Regular,
851852
// Animation specific event
852853
Animation,
854+
// Event side of a two-way binding (e.g. `[(property)]="expression"`).
855+
TwoWay,
853856
}
854857

855858
export class ParsedEvent {
@@ -882,6 +885,8 @@ export const enum BindingType {
882885
Style,
883886
// A binding to an animation reference (e.g. `[animate.key]="expression"`).
884887
Animation,
888+
// Property side of a two-way binding (e.g. `[(property)]="expression"`).
889+
TwoWay,
885890
}
886891

887892
export class BoundElementProperty {

‎packages/compiler/src/render3/r3_template_transform.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -474,7 +474,7 @@ class HtmlAstToIvyAst implements html.Visitor {
474474
const identifier = bindParts[IDENT_KW_IDX];
475475
const keySpan = createKeySpan(srcSpan, bindParts[KW_BIND_IDX], identifier);
476476
this.bindingParser.parsePropertyBinding(
477-
identifier, value, false, srcSpan, absoluteOffset, attribute.valueSpan,
477+
identifier, value, false, false, srcSpan, absoluteOffset, attribute.valueSpan,
478478
matchableAttributes, parsedProperties, keySpan);
479479

480480
} else if (bindParts[KW_LET_IDX]) {
@@ -502,7 +502,7 @@ class HtmlAstToIvyAst implements html.Visitor {
502502
const identifier = bindParts[IDENT_KW_IDX];
503503
const keySpan = createKeySpan(srcSpan, bindParts[KW_BINDON_IDX], identifier);
504504
this.bindingParser.parsePropertyBinding(
505-
identifier, value, false, srcSpan, absoluteOffset, attribute.valueSpan,
505+
identifier, value, false, true, srcSpan, absoluteOffset, attribute.valueSpan,
506506
matchableAttributes, parsedProperties, keySpan);
507507
this.parseAssignmentEvent(
508508
identifier, value, srcSpan, attribute.valueSpan, matchableAttributes, boundEvents,
@@ -536,14 +536,14 @@ class HtmlAstToIvyAst implements html.Visitor {
536536
const keySpan = createKeySpan(srcSpan, delims.start, identifier);
537537
if (delims.start === BINDING_DELIMS.BANANA_BOX.start) {
538538
this.bindingParser.parsePropertyBinding(
539-
identifier, value, false, srcSpan, absoluteOffset, attribute.valueSpan,
539+
identifier, value, false, true, srcSpan, absoluteOffset, attribute.valueSpan,
540540
matchableAttributes, parsedProperties, keySpan);
541541
this.parseAssignmentEvent(
542542
identifier, value, srcSpan, attribute.valueSpan, matchableAttributes, boundEvents,
543543
keySpan);
544544
} else if (delims.start === BINDING_DELIMS.PROPERTY.start) {
545545
this.bindingParser.parsePropertyBinding(
546-
identifier, value, false, srcSpan, absoluteOffset, attribute.valueSpan,
546+
identifier, value, false, false, srcSpan, absoluteOffset, attribute.valueSpan,
547547
matchableAttributes, parsedProperties, keySpan);
548548
} else {
549549
const events: ParsedEvent[] = [];

‎packages/compiler/src/render3/view/template.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -713,7 +713,8 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
713713
element.inputs.forEach(input => {
714714
const stylingInputWasSet = stylingBuilder.registerBoundInput(input);
715715
if (!stylingInputWasSet) {
716-
if (input.type === BindingType.Property && input.i18n) {
716+
if ((input.type === BindingType.Property || input.type === BindingType.TwoWay) &&
717+
input.i18n) {
717718
boundI18nAttrs.push(input);
718719
} else {
719720
allOtherInputs.push(input);
@@ -865,7 +866,7 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
865866
}
866867
this.allocateBindingSlots(value);
867868

868-
if (inputType === BindingType.Property) {
869+
if (inputType === BindingType.Property || inputType === BindingType.TwoWay) {
869870
if (value instanceof Interpolation) {
870871
// prop="{{value}}" and friends
871872
this.interpolatedUpdateInstruction(

‎packages/compiler/src/render3/view/util.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -341,7 +341,7 @@ function getAttrsForDirectiveMatching(elOrTpl: t.Element|t.Template): {[name: st
341341
});
342342

343343
elOrTpl.inputs.forEach(i => {
344-
if (i.type === BindingType.Property) {
344+
if (i.type === BindingType.Property || i.type === BindingType.TwoWay) {
345345
attributesMap[i.name] = '';
346346
}
347347
});

‎packages/compiler/src/template/pipeline/src/ingest.ts‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -125,8 +125,8 @@ export function ingestHostAttribute(
125125
}
126126

127127
export function ingestHostEvent(job: HostBindingCompilationJob, event: e.ParsedEvent) {
128-
const [phase, target] = event.type === e.ParsedEventType.Regular ? [null, event.targetOrPhase] :
129-
[event.targetOrPhase, null];
128+
const [phase, target] = event.type !== e.ParsedEventType.Animation ? [null, event.targetOrPhase] :
129+
[event.targetOrPhase, null];
130130
const eventBinding = ir.createListenerOp(
131131
job.root.xref, new ir.SlotHandle(), event.name, null,
132132
makeListenerHandlerOps(job.root, event.handler, event.handlerSpan), phase, target, true,
@@ -846,6 +846,8 @@ function convertAstWithInterpolation(
846846
// TODO: Can we populate Template binding kinds in ingest?
847847
const BINDING_KINDS = new Map<e.BindingType, ir.BindingKind>([
848848
[e.BindingType.Property, ir.BindingKind.Property],
849+
// TODO(crisbeto): we'll need a different BindingKind for two-way bindings.
850+
[e.BindingType.TwoWay, ir.BindingKind.Property],
849851
[e.BindingType.Attribute, ir.BindingKind.Attribute],
850852
[e.BindingType.Class, ir.BindingKind.ClassName],
851853
[e.BindingType.Style, ir.BindingKind.StyleProperty],
@@ -1042,8 +1044,8 @@ function createTemplateBinding(
10421044
// update instruction.
10431045
if (templateKind === ir.TemplateKind.Structural) {
10441046
if (!isStructuralTemplateAttribute &&
1045-
(type === e.BindingType.Property || type === e.BindingType.Class ||
1046-
type === e.BindingType.Style)) {
1047+
(type === e.BindingType.Property || type === e.BindingType.TwoWay ||
1048+
type === e.BindingType.Class || type === e.BindingType.Style)) {
10471049
// Because this binding doesn't really target the ng-template, it must be a binding on an
10481050
// inner node of a structural template. We can't skip it entirely, because we still need it on
10491051
// the ng-template's consts (e.g. for the purposes of directive matching). However, we should

‎packages/compiler/src/template_parser/binding_parser.ts‎

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ export class BindingParser {
5151
const expression = properties[propName];
5252
if (typeof expression === 'string') {
5353
this.parsePropertyBinding(
54-
propName, expression, true, sourceSpan, sourceSpan.start.offset, undefined, [],
54+
propName, expression, true, false, sourceSpan, sourceSpan.start.offset, undefined, [],
5555
// Use the `sourceSpan` for `keySpan`. This isn't really accurate, but neither is the
5656
// sourceSpan, as it represents the sourceSpan of the host itself rather than the
5757
// source of the host binding (which doesn't exist in the template). Regardless,
@@ -169,7 +169,8 @@ export class BindingParser {
169169
const srcSpan = isIvyAst ? bindingSpan : sourceSpan;
170170
const valueSpan = moveParseSourceSpan(sourceSpan, binding.value.ast.sourceSpan);
171171
this._parsePropertyAst(
172-
key, binding.value, srcSpan, keySpan, valueSpan, targetMatchableAttrs, targetProps);
172+
key, binding.value, false, srcSpan, keySpan, valueSpan, targetMatchableAttrs,
173+
targetProps);
173174
} else {
174175
targetMatchableAttrs.push([key, '' /* value */]);
175176
// Since this is a literal attribute with no RHS, source span should be
@@ -239,8 +240,8 @@ export class BindingParser {
239240
}
240241

241242
parsePropertyBinding(
242-
name: string, expression: string, isHost: boolean, sourceSpan: ParseSourceSpan,
243-
absoluteOffset: number, valueSpan: ParseSourceSpan|undefined,
243+
name: string, expression: string, isHost: boolean, isPartOfAssignmentBinding: boolean,
244+
sourceSpan: ParseSourceSpan, absoluteOffset: number, valueSpan: ParseSourceSpan|undefined,
244245
targetMatchableAttrs: string[][], targetProps: ParsedProperty[], keySpan: ParseSourceSpan) {
245246
if (name.length === 0) {
246247
this._reportError(`Property name is missing in binding`, sourceSpan);
@@ -272,7 +273,8 @@ export class BindingParser {
272273
} else {
273274
this._parsePropertyAst(
274275
name, this.parseBinding(expression, isHost, valueSpan || sourceSpan, absoluteOffset),
275-
sourceSpan, keySpan, valueSpan, targetMatchableAttrs, targetProps);
276+
isPartOfAssignmentBinding, sourceSpan, keySpan, valueSpan, targetMatchableAttrs,
277+
targetProps);
276278
}
277279
}
278280

@@ -284,19 +286,21 @@ export class BindingParser {
284286
const expr = this.parseInterpolation(value, valueSpan || sourceSpan, interpolatedTokens);
285287
if (expr) {
286288
this._parsePropertyAst(
287-
name, expr, sourceSpan, keySpan, valueSpan, targetMatchableAttrs, targetProps);
289+
name, expr, false, sourceSpan, keySpan, valueSpan, targetMatchableAttrs, targetProps);
288290
return true;
289291
}
290292
return false;
291293
}
292294

293295
private _parsePropertyAst(
294-
name: string, ast: ASTWithSource, sourceSpan: ParseSourceSpan, keySpan: ParseSourceSpan,
295-
valueSpan: ParseSourceSpan|undefined, targetMatchableAttrs: string[][],
296-
targetProps: ParsedProperty[]) {
296+
name: string, ast: ASTWithSource, isPartOfAssignmentBinding: boolean,
297+
sourceSpan: ParseSourceSpan, keySpan: ParseSourceSpan, valueSpan: ParseSourceSpan|undefined,
298+
targetMatchableAttrs: string[][], targetProps: ParsedProperty[]) {
297299
targetMatchableAttrs.push([name, ast.source!]);
298-
targetProps.push(
299-
new ParsedProperty(name, ast, ParsedPropertyType.DEFAULT, sourceSpan, keySpan, valueSpan));
300+
targetProps.push(new ParsedProperty(
301+
name, ast,
302+
isPartOfAssignmentBinding ? ParsedPropertyType.TWO_WAY : ParsedPropertyType.DEFAULT,
303+
sourceSpan, keySpan, valueSpan));
300304
}
301305

302306
private _parseAnimation(
@@ -387,7 +391,8 @@ export class BindingParser {
387391
boundPropertyName = mapPropertyName ? mappedPropName : boundProp.name;
388392
securityContexts = calcPossibleSecurityContexts(
389393
this._schemaRegistry, elementSelector, mappedPropName, false);
390-
bindingType = BindingType.Property;
394+
bindingType =
395+
boundProp.type === ParsedPropertyType.TWO_WAY ? BindingType.TwoWay : BindingType.Property;
391396
if (!skipValidation) {
392397
this._validatePropertyOrAttributeName(mappedPropName, boundProp.sourceSpan, false);
393398
}
@@ -465,7 +470,8 @@ export class BindingParser {
465470
const ast = this._parseAction(expression, isAssignmentEvent, handlerSpan);
466471
targetMatchableAttrs.push([name!, ast.source!]);
467472
targetEvents.push(new ParsedEvent(
468-
eventName, target, ParsedEventType.Regular, ast, sourceSpan, handlerSpan, keySpan));
473+
eventName, target, isAssignmentEvent ? ParsedEventType.TwoWay : ParsedEventType.Regular,
474+
ast, sourceSpan, handlerSpan, keySpan));
469475
// Don't detect directives for event names for now,
470476
// so don't add the event name to the matchableAttrs
471477
}

‎packages/compiler/test/render3/r3_template_transform_spec.ts‎

Lines changed: 13 additions & 12 deletions

0 commit comments

Comments
 (0)