fix(ivy): host bindings should support array/object literals (angular#25583)

PR Close angular#25583

Author: kara

packages/core/src/render3/component.ts

@@ -112,7 +112,6 @@ export function renderComponent<T>(
   rootView[INJECTOR] = opts.injector || null;
 
   const oldView = enterView(rootView, null !);
-  rootView[BINDING_INDEX] = rootView[TVIEW].bindingStartIndex;
   let elementNode: LElementNode;
   let component: T;
   try {

packages/core/src/render3/component_ref.ts

@@ -127,7 +127,6 @@ export class ComponentFactory<T> extends viewEngine_ComponentFactory<T> {
 
     // rootView is the parent when bootstrapping
     const oldView = enterView(rootView, null !);
-    rootView[BINDING_INDEX] = rootView[TVIEW].bindingStartIndex;
 
     let component: T;
     let elementNode: LElementNode;

packages/core/src/render3/definition.ts

@@ -56,7 +56,7 @@ export function defineComponent<T>(componentDefinition: {
   /**
    * The number of nodes, local refs, and pipes in this component template.
    *
-   * Used to calculate the length of the component's LViewData array, so we
+   * Used to calculate the length of this component's LViewData array, so we
    * can pre-fill the array and set the binding start index.
    */
   // TODO(kara): remove queries from this count
@@ -65,11 +65,19 @@ export function defineComponent<T>(componentDefinition: {
   /**
    * The number of bindings in this component template (including pure fn bindings).
    *
-   * Used to calculate the length of the component's LViewData array, so we
+   * Used to calculate the length of this component's LViewData array, so we
    * can pre-fill the array and set the host binding start index.
    */
   vars: number;
 
+  /**
+   * The number of host bindings (including pure fn bindings) in this component.
+   *
+   * Used to calculate the length of the LViewData array for the *parent* component
+   * of this component.
+   */
+  hostVars?: number;
+
   /**
    * Static attributes to set on host element.
    *
@@ -264,6 +272,7 @@ export function defineComponent<T>(componentDefinition: {
     diPublic: null,
     consts: componentDefinition.consts,
     vars: componentDefinition.vars,
+    hostVars: componentDefinition.hostVars || 0,
     factory: componentDefinition.factory,
     template: componentDefinition.template || null !,
     hostBindings: componentDefinition.hostBindings || null,
@@ -577,6 +586,14 @@ export const defineDirective = defineComponent as any as<T>(directiveDefinition:
    */
   features?: DirectiveDefFeature[];
 
+  /**
+   * The number of host bindings (including pure fn bindings) in this directive.
+   *
+   * Used to calculate the length of the LViewData array for the *parent* component
+   * of this directive.
+   */
+  hostVars?: number;
+
   /**
    * Function executed by the parent template to allow child directive to apply host bindings.
    */

packages/core/src/render3/instructions.ts

@@ -239,6 +239,18 @@ let checkNoChangesMode = false;
 /** Whether or not this is the first time the current view has been processed. */
 let firstTemplatePass = true;
 
+/**
+ * The root index from which pure function instructions should calculate their binding
+ * indices. In component views, this is TView.bindingStartIndex. In a host binding
+ * context, this is the TView.hostBindingStartIndex + any hostVars before the given dir.
+ */
+let bindingRootIndex: number = -1;
+
+// top level variables should not be exported for performance reasons (PERF_NOTES.md)
+export function getBindingRoot() {
+  return bindingRootIndex;
+}
+
 const enum BindingDirection {
   Input,
   Output,
@@ -263,7 +275,7 @@ export function enterView(newView: LViewData, host: LElementNode | LViewNode | n
 
   creationMode = newView && (newView[FLAGS] & LViewFlags.CreationMode) === LViewFlags.CreationMode;
   firstTemplatePass = newView && tView.firstTemplatePass;
-
+  bindingRootIndex = newView && tView.bindingStartIndex;
   renderer = newView && newView[RENDERER];
 
   if (host != null) {
@@ -295,7 +307,7 @@ export function leaveView(newView: LViewData, creationOnly?: boolean): void {
     viewData[FLAGS] &= ~(LViewFlags.CreationMode | LViewFlags.Dirty);
   }
   viewData[FLAGS] |= LViewFlags.RunInit;
-  viewData[BINDING_INDEX] = -1;
+  viewData[BINDING_INDEX] = tView.bindingStartIndex;
   enterView(newView, null);
 }
 
@@ -329,11 +341,13 @@ function refreshDescendantViews() {
 /** Sets the host bindings for the current view. */
 export function setHostBindings(bindings: number[] | null): void {
   if (bindings != null) {
+    bindingRootIndex = viewData[BINDING_INDEX] = tView.hostBindingStartIndex;
     const defs = tView.directives !;
     for (let i = 0; i < bindings.length; i += 2) {
       const dirIndex = bindings[i];
       const def = defs[dirIndex] as DirectiveDefInternal<any>;
       def.hostBindings && def.hostBindings(dirIndex, bindings[i + 1]);
+      bindingRootIndex = viewData[BINDING_INDEX] = bindingRootIndex + def.hostVars;
     }
   }
 }
@@ -377,7 +391,7 @@ export function createLViewData<T>(
     null,                                                                        // queries
     flags | LViewFlags.CreationMode | LViewFlags.Attached | LViewFlags.RunInit,  // flags
     null !,                                                                      // hostNode
-    -1,                                                                          // bindingIndex
+    tView.bindingStartIndex,                                                     // bindingIndex
     null,                                                                        // directives
     null,                                                                        // cleanupInstances
     context,                                                                     // context
@@ -600,7 +614,6 @@ export function renderEmbeddedTemplate<T>(
 
       oldView = enterView(viewNode.data !, viewNode);
       namespaceHTML();
-      viewData[BINDING_INDEX] = tView.bindingStartIndex;
       tView.template !(rf, context);
       if (rf & RenderFlags.Update) {
         refreshDescendantViews();
@@ -644,7 +657,6 @@ export function renderComponentOrTemplate<T>(
     }
     if (templateFn) {
       namespaceHTML();
-      viewData[BINDING_INDEX] = tView.bindingStartIndex;
       templateFn(getRenderFlags(hostView), componentOrContext !);
       refreshDescendantViews();
     } else {
@@ -1042,14 +1054,16 @@ export function createTView(
     directives: DirectiveDefListOrFactory | null, pipes: PipeDefListOrFactory | null,
     viewQuery: ComponentQuery<any>| null): TView {
   ngDevMode && ngDevMode.tView++;
+  const bindingStartIndex = HEADER_OFFSET + consts;
   return {
     id: viewIndex,
     template: templateFn,
     viewQuery: viewQuery,
     node: null !,
     data: HEADER_FILLER.slice(),  // Fill in to match HEADER_OFFSET in LViewData
     childIndex: -1,               // Children set in addToViewTree(), if any
-    bindingStartIndex: HEADER_OFFSET + consts,
+    bindingStartIndex: bindingStartIndex,
+    hostBindingStartIndex: bindingStartIndex + vars,
     directives: null,
     firstTemplatePass: true,
     initHooks: null,
@@ -2062,7 +2076,6 @@ export function embeddedViewStart(viewBlockId: number, consts: number, vars: num
     }
     lContainer[ACTIVE_INDEX] !++;
   }
-  viewData[BINDING_INDEX] = tView.bindingStartIndex;
   return getRenderFlags(viewNode.data);
 }
 
@@ -2467,7 +2480,6 @@ export function detectChangesInternal<T>(
   const hostTView = hostView[TVIEW];
   const templateFn = hostTView.template !;
   const viewQuery = hostTView.viewQuery;
-  viewData[BINDING_INDEX] = tView.bindingStartIndex;
 
   try {
     namespaceHTML();

packages/core/src/render3/interfaces/definition.ts

@@ -141,6 +141,14 @@ export interface DirectiveDef<T, Selector extends string> extends BaseDef<T> {
   /** Refreshes content queries associated with directives in a given view */
   contentQueriesRefresh: ((directiveIndex: number, queryIndex: number) => void)|null;
 
+  /**
+   * The number of host bindings (including pure fn bindings) in this directive/component.
+   *
+   * Used to calculate the length of the LViewData array for the *parent* component
+   * of this directive/component.
+   */
+  hostVars: number;
+
   /** Refreshes host bindings on the associated directive. */
   hostBindings: ((directiveIndex: number, elementIndex: number) => void)|null;
 

packages/core/src/render3/interfaces/view.ts

@@ -289,6 +289,13 @@ export interface TView {
    */
   bindingStartIndex: number;
 
+  /**
+   * The index at which the data array begins to store host bindings for components
+   * or directives in its template. Saving this value ensures that we can set the
+   * binding root and binding index correctly before checking host bindings.
+   */
+  hostBindingStartIndex: number;
+
   /**
    * Index of the host node of the first LView or LContainer beneath this LView in
    * the hierarchy.

packages/core/src/render3/pure_function.ts

@@ -6,20 +6,22 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
-import {bindingUpdated, bindingUpdated2, bindingUpdated4, updateBinding, getBinding, getCreationMode, getTView, bindingUpdated3,} from './instructions';
+import {bindingUpdated, bindingUpdated2, bindingUpdated4, updateBinding, getBinding, getCreationMode, bindingUpdated3, getBindingRoot, getTView,} from './instructions';
 
 /**
  * Bindings for pure functions are stored after regular bindings.
  *
- *  ----------------------------------------------------------------------------
- *  |  LNodes / local refs / pipes ... | regular bindings / interpolations | pure function bindings
- *  ----------------------------------------------------------------------------
- *                                     ^
- *                          TView.bindingStartIndex
+ * |--------consts--------|----------------vars----------------|------ hostVars (dir1) ------|
+ * ---------------------------------------------------------------------------------------------
+ * | nodes / refs / pipes | bindings | pure function bindings  | host bindings  | host slots |
+ * ---------------------------------------------------------------------------------------------
+ *                        ^                                    ^
+ *             TView.bindingStartIndex            TView.hostBindingStartIndex
  *
- * Pure function instructions are given an offset from TView.bindingStartIndex.
- * Adding the offset to TView.bindingStartIndex gives the first index where the bindings
- * are stored.
+ * Pure function instructions are given an offset from the binding root. Adding the offset to the
+ * binding root gives the first index where the bindings are stored. In component views, the binding
+ * root is the bindingStartIndex. In host bindings, the binding root is the hostBindingStartIndex +
+ * any hostVars in directives evaluated before it.
  */
 
 /**
@@ -33,7 +35,7 @@ import {bindingUpdated, bindingUpdated2, bindingUpdated4, updateBinding, getBind
  */
 export function pureFunction0<T>(slotOffset: number, pureFn: () => T, thisArg?: any): T {
   // TODO(kara): use bindingRoot instead of bindingStartIndex when implementing host bindings
-  const bindingIndex = getTView().bindingStartIndex + slotOffset;
+  const bindingIndex = getBindingRoot() + slotOffset;
   return getCreationMode() ?
       updateBinding(bindingIndex, thisArg ? pureFn.call(thisArg) : pureFn()) :
       getBinding(bindingIndex);
@@ -52,7 +54,7 @@ export function pureFunction0<T>(slotOffset: number, pureFn: () => T, thisArg?:
 export function pureFunction1(
     slotOffset: number, pureFn: (v: any) => any, exp: any, thisArg?: any): any {
   // TODO(kara): use bindingRoot instead of bindingStartIndex when implementing host bindings
-  const bindingIndex = getTView().bindingStartIndex + slotOffset;
+  const bindingIndex = getBindingRoot() + slotOffset;
   return bindingUpdated(bindingIndex, exp) ?
       updateBinding(bindingIndex + 1, thisArg ? pureFn.call(thisArg, exp) : pureFn(exp)) :
       getBinding(bindingIndex + 1);
@@ -73,7 +75,7 @@ export function pureFunction2(
     slotOffset: number, pureFn: (v1: any, v2: any) => any, exp1: any, exp2: any,
     thisArg?: any): any {
   // TODO(kara): use bindingRoot instead of bindingStartIndex when implementing host bindings
-  const bindingIndex = getTView().bindingStartIndex + slotOffset;
+  const bindingIndex = getBindingRoot() + slotOffset;
   return bindingUpdated2(bindingIndex, exp1, exp2) ?
       updateBinding(
           bindingIndex + 2, thisArg ? pureFn.call(thisArg, exp1, exp2) : pureFn(exp1, exp2)) :
@@ -96,7 +98,7 @@ export function pureFunction3(
     slotOffset: number, pureFn: (v1: any, v2: any, v3: any) => any, exp1: any, exp2: any, exp3: any,
     thisArg?: any): any {
   // TODO(kara): use bindingRoot instead of bindingStartIndex when implementing host bindings
-  const bindingIndex = getTView().bindingStartIndex + slotOffset;
+  const bindingIndex = getBindingRoot() + slotOffset;
   return bindingUpdated3(bindingIndex, exp1, exp2, exp3) ?
       updateBinding(
           bindingIndex + 3,
@@ -121,7 +123,7 @@ export function pureFunction4(
     slotOffset: number, pureFn: (v1: any, v2: any, v3: any, v4: any) => any, exp1: any, exp2: any,
     exp3: any, exp4: any, thisArg?: any): any {
   // TODO(kara): use bindingRoot instead of bindingStartIndex when implementing host bindings
-  const bindingIndex = getTView().bindingStartIndex + slotOffset;
+  const bindingIndex = getBindingRoot() + slotOffset;
   return bindingUpdated4(bindingIndex, exp1, exp2, exp3, exp4) ?
       updateBinding(
           bindingIndex + 4,
@@ -147,7 +149,7 @@ export function pureFunction5(
     slotOffset: number, pureFn: (v1: any, v2: any, v3: any, v4: any, v5: any) => any, exp1: any,
     exp2: any, exp3: any, exp4: any, exp5: any, thisArg?: any): any {
   // TODO(kara): use bindingRoot instead of bindingStartIndex when implementing host bindings
-  const bindingIndex = getTView().bindingStartIndex + slotOffset;
+  const bindingIndex = getBindingRoot() + slotOffset;
   const different = bindingUpdated4(bindingIndex, exp1, exp2, exp3, exp4);
   return bindingUpdated(bindingIndex + 4, exp5) || different ?
       updateBinding(
@@ -175,7 +177,7 @@ export function pureFunction6(
     slotOffset: number, pureFn: (v1: any, v2: any, v3: any, v4: any, v5: any, v6: any) => any,
     exp1: any, exp2: any, exp3: any, exp4: any, exp5: any, exp6: any, thisArg?: any): any {
   // TODO(kara): use bindingRoot instead of bindingStartIndex when implementing host bindings
-  const bindingIndex = getTView().bindingStartIndex + slotOffset;
+  const bindingIndex = getBindingRoot() + slotOffset;
   const different = bindingUpdated4(bindingIndex, exp1, exp2, exp3, exp4);
   return bindingUpdated2(bindingIndex + 4, exp5, exp6) || different ?
       updateBinding(
@@ -205,7 +207,7 @@ export function pureFunction7(
     pureFn: (v1: any, v2: any, v3: any, v4: any, v5: any, v6: any, v7: any) => any, exp1: any,
     exp2: any, exp3: any, exp4: any, exp5: any, exp6: any, exp7: any, thisArg?: any): any {
   // TODO(kara): use bindingRoot instead of bindingStartIndex when implementing host bindings
-  const bindingIndex = getTView().bindingStartIndex + slotOffset;
+  const bindingIndex = getBindingRoot() + slotOffset;
   let different = bindingUpdated4(bindingIndex, exp1, exp2, exp3, exp4);
   return bindingUpdated3(bindingIndex + 4, exp5, exp6, exp7) || different ?
       updateBinding(
@@ -238,7 +240,7 @@ export function pureFunction8(
     exp1: any, exp2: any, exp3: any, exp4: any, exp5: any, exp6: any, exp7: any, exp8: any,
     thisArg?: any): any {
   // TODO(kara): use bindingRoot instead of bindingStartIndex when implementing host bindings
-  const bindingIndex = getTView().bindingStartIndex + slotOffset;
+  const bindingIndex = getBindingRoot() + slotOffset;
   const different = bindingUpdated4(bindingIndex, exp1, exp2, exp3, exp4);
   return bindingUpdated4(bindingIndex + 4, exp5, exp6, exp7, exp8) || different ?
       updateBinding(
@@ -264,7 +266,7 @@ export function pureFunction8(
 export function pureFunctionV(
     slotOffset: number, pureFn: (...v: any[]) => any, exps: any[], thisArg?: any): any {
   // TODO(kara): use bindingRoot instead of bindingStartIndex when implementing host bindings
-  let bindingIndex = getTView().bindingStartIndex + slotOffset;
+  let bindingIndex = getBindingRoot() + slotOffset;
   let different = false;
   for (let i = 0; i < exps.length; i++) {
     bindingUpdated(bindingIndex++, exps[i]) && (different = true);

packages/core/test/bundling/hello_world/bundle.golden_symbols.json

@@ -98,6 +98,9 @@
   {
     "name": "baseDirectiveCreate"
   },
+  {
+    "name": "bindingRootIndex"
+  },
   {
     "name": "bloomAdd"
   },

packages/core/test/bundling/todo/bundle.golden_symbols.json

@@ -326,6 +326,9 @@
   {
     "name": "bind"
   },
+  {
+    "name": "bindingRootIndex"
+  },
   {
     "name": "bindingUpdated"
   },

packages/core/test/render3/directive_spec.ts

@@ -15,36 +15,6 @@ import {TemplateFixture} from './render_util';
 
 describe('directive', () => {
 
-  describe('host', () => {
-
-    it('should support host bindings in directives', () => {
-      let directiveInstance: Directive|undefined;
-
-      class Directive {
-        klass = 'foo';
-        static ngDirectiveDef = defineDirective({
-          type: Directive,
-          selectors: [['', 'dir', '']],
-          factory: () => directiveInstance = new Directive,
-          hostBindings: (directiveIndex: number, elementIndex: number) => {
-            elementProperty(
-                elementIndex, 'className', bind(loadDirective<Directive>(directiveIndex).klass));
-          }
-        });
-      }
-
-      function Template() { element(0, 'span', [AttributeMarker.SelectOnly, 'dir']); }
-
-      const fixture = new TemplateFixture(Template, () => {}, 1, 0, [Directive]);
-      expect(fixture.html).toEqual('<span class="foo"></span>');
-
-      directiveInstance !.klass = 'bar';
-      fixture.update();
-      expect(fixture.html).toEqual('<span class="bar"></span>');
-    });
-
-  });
-
   describe('selectors', () => {
 
     it('should match directives with attribute selectors on bindings', () => {