fix(ivy): property bindings use correct indices (#30129)

- Extracts and documents code that will be common to interpolation instructions
- Ensures that binding indices are updated at the proper time during compilation
- Adds additional tests

Related #30011

PR Close #30129

packages/compiler/src/compiler_util/expression_converter.ts

@@ -155,16 +155,12 @@ export function convertPropertyBinding(
     localResolver = new DefaultLocalResolver();
   }
   const currValExpr = createCurrValueExpr(bindingId);
-  const stmts: o.Statement[] = [];
   const visitor =
       new _AstToIrVisitor(localResolver, implicitReceiver, bindingId, interpolationFunction);
   const outputExpr: o.Expression = expressionWithoutBuiltins.visit(visitor, _Mode.Expression);
+  const stmts: o.Statement[] = getStatementsFromVisitor(visitor, bindingId);
 
-  if (visitor.temporaryCount) {
-    for (let i = 0; i < visitor.temporaryCount; i++) {
-      stmts.push(temporaryDeclaration(bindingId, i));
-    }
-  } else if (form == BindingForm.TrySimple) {
+  if (visitor.temporaryCount === 0 && form == BindingForm.TrySimple) {
     return new ConvertPropertyBindingResult([], outputExpr);
   }
 
@@ -172,6 +168,58 @@ export function convertPropertyBinding(
   return new ConvertPropertyBindingResult(stmts, currValExpr);
 }
 
+/**
+ * Given some expression, such as a binding or interpolation expression, and a context expression to
+ * look values up on, visit each facet of the given expression resolving values from the context
+ * expression such that a list of arguments can be derived from the found values that can be used as
+ * arguments to an external update instruction.
+ *
+ * @param localResolver The resolver to use to look up expressions by name appropriately
+ * @param contextVariableExpression The expression representing the context variable used to create
+ * the final argument expressions
+ * @param expressionWithArgumentsToExtract The expression to visit to figure out what values need to
+ * be resolved and what arguments list to build.
+ * @param bindingId A name prefix used to create temporary variable names if they're needed for the
+ * arguments generated
+ * @returns An array of expressions that can be passed as arguments to instruction expressions like
+ * `o.importExpr(R3.propertyInterpolate).callFn(result)`
+ */
+export function convertUpdateArguments(
+    localResolver: LocalResolver, contextVariableExpression: o.Expression,
+    expressionWithArgumentsToExtract: cdAst.AST, bindingId: string) {
+  const visitor =
+      new _AstToIrVisitor(localResolver, contextVariableExpression, bindingId, undefined);
+  const outputExpr: o.InvokeFunctionExpr =
+      expressionWithArgumentsToExtract.visit(visitor, _Mode.Expression);
+
+  const stmts = getStatementsFromVisitor(visitor, bindingId);
+
+  // Removing the first argument, because it was a length for ViewEngine, not Ivy.
+  let args = outputExpr.args.slice(1);
+  if (expressionWithArgumentsToExtract instanceof cdAst.Interpolation) {
+    // If we're dealing with an interpolation of 1 value with an empty prefix and suffix, reduce the
+    // args returned to just the value, because we're going to pass it to a special instruction.
+    const strings = expressionWithArgumentsToExtract.strings;
+    if (args.length === 3 && strings[0] === '' && strings[1] === '') {
+      // Single argument interpolate instructions.
+      args = [args[1]];
+    } else if (args.length >= 19) {
+      // 19 or more arguments must be passed to the `interpolateV`-style instructions, which accept
+      // an array of arguments
+      args = [o.literalArr(args)];
+    }
+  }
+  return {stmts, args};
+}
+
+function getStatementsFromVisitor(visitor: _AstToIrVisitor, bindingId: string) {
+  const stmts: o.Statement[] = [];
+  for (let i = 0; i < visitor.temporaryCount; i++) {
+    stmts.push(temporaryDeclaration(bindingId, i));
+  }
+  return stmts;
+}
+
 function convertBuiltins(converterFactory: BuiltinConverterFactory, ast: cdAst.AST): cdAst.AST {
   const visitor = new _BuiltinAstConverter(converterFactory);
   return ast.visit(visitor);

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

@@ -7,7 +7,7 @@
  */
 
 import {flatten, sanitizeIdentifier} from '../../compile_metadata';
-import {BindingForm, BuiltinFunctionCall, LocalResolver, convertActionBinding, convertPropertyBinding} from '../../compiler_util/expression_converter';
+import {BindingForm, BuiltinFunctionCall, LocalResolver, convertActionBinding, convertPropertyBinding, convertUpdateArguments} from '../../compiler_util/expression_converter';
 import {ConstantPool} from '../../constant_pool';
 import * as core from '../../core';
 import {AST, AstMemoryEfficientTransformer, BindingPipe, BindingType, FunctionCall, ImplicitReceiver, Interpolation, LiteralArray, LiteralMap, LiteralPrimitive, ParsedEventType, PropertyRead} from '../../expression_parser/ast';
@@ -750,23 +750,13 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
 
           if (inputType === BindingType.Property) {
             if (value instanceof Interpolation) {
-              // Interpolated properties
-              const {currValExpr} = convertPropertyBinding(
-                  this, implicit, value, this.bindingContext(), BindingForm.TrySimple);
-
-              let args: o.Expression[] = (currValExpr as any).args;
-              args.shift();  // ViewEngine required a count, we don't need that.
-
-              // For interpolations like attr="{{foo}}", we don't need ["", foo, ""], just [foo].
-              if (args.length === 3 && isEmptyStringExpression(args[0]) &&
-                  isEmptyStringExpression(args[2])) {
-                args = [args[1]];
-              }
-
               this.updateInstruction(
-                  elementIndex, input.sourceSpan, propertyInterpolate(args.length), () => {
-                    return [o.literal(attrName), ...args, ...params];
-                  });
+                  elementIndex, input.sourceSpan, getPropertyInterpolationExpression(value),
+                  () =>
+                      [o.literal(attrName),
+                       ...this.getUpdateInstructionArguments(o.variable(CONTEXT_NAME), value),
+                       ...params]);
+
             } else {
               // Bound, un-interpolated properties
               this.updateInstruction(elementIndex, input.sourceSpan, R3.property, () => {
@@ -1076,6 +1066,21 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
                                                           o.importExpr(R3.bind).callFn([valExpr]);
   }
 
+  /**
+   * Gets a list of argument expressions to pass to an update instruction expression. Also updates
+   * the temp variables state with temp variables that were identified as needing to be created
+   * while visiting the arguments.
+   * @param contextExpression The expression for the context variable used to create arguments
+   * @param value The original expression we will be resolving an arguments list from.
+   */
+  private getUpdateInstructionArguments(contextExpression: o.Expression, value: AST):
+      o.Expression[] {
+    const {args, stmts} =
+        convertUpdateArguments(this, contextExpression, value, this.bindingContext());
+    this._tempVariables.push(...stmts);
+    return args;
+  }
+
   private matchDirectives(tagName: string, elOrTpl: t.Element|t.Template) {
     if (this.directiveMatcher) {
       const selector = createCssSelector(tagName, getAttrsForDirectiveMatching(elOrTpl));
@@ -1646,16 +1651,12 @@ function interpolate(args: o.Expression[]): o.Expression {
   return o.importExpr(R3.interpolationV).callFn([o.literalArr(args)]);
 }
 
-function isEmptyStringExpression(exp: o.Expression) {
-  return exp instanceof o.LiteralExpr && exp.value === '';
-}
-
-function propertyInterpolate(argsLength: number) {
-  if (argsLength % 2 !== 1) {
-    error(`Invalid propertyInterpolate argument length ${argsLength}`);
-  }
-
-  switch (argsLength) {
+/**
+ * Gets the instruction to generate for an interpolated property
+ * @param interpolation An Interpolation AST
+ */
+function getPropertyInterpolationExpression(interpolation: Interpolation) {
+  switch (getInterpolationArgsLength(interpolation)) {
     case 1:
       return R3.propertyInterpolate;
     case 3:
@@ -1679,6 +1680,22 @@ function propertyInterpolate(argsLength: number) {
   }
 }
 
+/**
+ * Gets the number of arguments expected to be passed to a generated instruction in the case of
+ * interpolation instructions.
+ * @param interpolation An interpolation ast
+ */
+function getInterpolationArgsLength(interpolation: Interpolation) {
+  const {expressions, strings} = interpolation;
+  if (expressions.length === 1 && strings.length === 2 && strings[0] === '' && strings[1] === '') {
+    // If the interpolation has one interpolated value, but the prefix and suffix are both empty
+    // strings, we only pass one argument, to a special instruction like `propertyInterpolate` or
+    // `textInterpolate`.
+    return 1;
+  } else {
+    return expressions.length + strings.length;
+  }
+}
 /**
  * Options that can be used to modify how a template is parsed by `parseTemplate()`.
  */