From 6f5c124fe96c9dc6311eea022f3479df686a9386 Mon Sep 17 00:00:00 2001
From: Andrew Kushnir <akushnir@google.com>
Date: Sat, 1 Dec 2018 10:33:23 -0800
Subject: [PATCH] fix(ivy): proper accounting of host vars in case of inherited
 Directives (#27392)

Prior to this change, the number of host vars stored for directives with `hostBindings` in expando block was incorrect for inherited directives (in case both parent and child directive have `hostBindings` defined). Now if we identify that we already added a `hostBinding` into expando block, we just increase the corresponding number of host binding vars

PR Close #27392
---
 packages/core/src/render3/instructions.ts     | 11 ++-
 .../core/test/render3/host_binding_spec.ts    | 78 ++++++++++++++++++-
 2 files changed, 85 insertions(+), 4 deletions(-)

diff --git a/packages/core/src/render3/instructions.ts b/packages/core/src/render3/instructions.ts
index 099768117b..f203189377 100644
--- a/packages/core/src/render3/instructions.ts
+++ b/packages/core/src/render3/instructions.ts
@@ -1624,10 +1624,15 @@ function queueHostBindingForCheck(
   ngDevMode &&
       assertEqual(getFirstTemplatePass(), true, 'Should only be called in first template pass.');
   const expando = tView.expandoInstructions !;
-  // check whether a given `hostBindings` function already exists in expandoInstructions,
+  const length = expando.length;
+  // Check whether a given `hostBindings` function already exists in expandoInstructions,
   // which can happen in case directive definition was extended from base definition (as a part of
-  // the `InheritDefinitionFeature` logic)
-  if (expando.length < 2 || expando[expando.length - 2] !== def.hostBindings) {
+  // the `InheritDefinitionFeature` logic). If we found the same `hostBindings` function in the
+  // list, we just increase the number of host vars associated with that function, but do not add it
+  // into the list again.
+  if (length >= 2 && expando[length - 2] === def.hostBindings) {
+    expando[length - 1] = (expando[length - 1] as number) + hostVars;
+  } else {
     expando.push(def.hostBindings !, hostVars);
   }
 }
diff --git a/packages/core/test/render3/host_binding_spec.ts b/packages/core/test/render3/host_binding_spec.ts
index 845b3ffa6d..26635727bb 100644
--- a/packages/core/test/render3/host_binding_spec.ts
+++ b/packages/core/test/render3/host_binding_spec.ts
@@ -8,7 +8,7 @@
 
 import {ElementRef, EventEmitter} from '@angular/core';
 
-import {AttributeMarker, defineComponent, template, defineDirective, ProvidersFeature, NgOnChangesFeature, QueryList} from '../../src/render3/index';
+import {AttributeMarker, defineComponent, template, defineDirective, InheritDefinitionFeature, ProvidersFeature, NgOnChangesFeature, QueryList} from '../../src/render3/index';
 import {allocHostVars, bind, directiveInject, element, elementEnd, elementProperty, elementStart, listener, load, text, textBinding, loadQueryList, registerContentQuery} from '../../src/render3/instructions';
 import {query, queryRefresh} from '../../src/render3/query';
 import {RenderFlags} from '../../src/render3/interfaces/definition';
@@ -780,6 +780,82 @@ describe('host bindings', () => {
     expect(hostElement.title).toBe('other title');
   });
 
+  it('should work correctly with inherited directives with hostBindings', () => {
+    let subDir !: SubDirective;
+    let superDir !: SuperDirective;
+
+    class SuperDirective {
+      id = 'my-id';
+
+      static ngDirectiveDef = defineDirective({
+        type: SuperDirective,
+        selectors: [['', 'superDir', '']],
+        hostBindings: (rf: RenderFlags, ctx: SuperDirective, elementIndex: number) => {
+          if (rf & RenderFlags.Create) {
+            allocHostVars(1);
+          }
+          if (rf & RenderFlags.Update) {
+            elementProperty(elementIndex, 'id', bind(ctx.id));
+          }
+        },
+        factory: () => superDir = new SuperDirective(),
+      });
+    }
+
+    class SubDirective extends SuperDirective {
+      title = 'my-title';
+
+      static ngDirectiveDef = defineDirective({
+        type: SubDirective,
+        selectors: [['', 'subDir', '']],
+        hostBindings: (rf: RenderFlags, ctx: SubDirective, elementIndex: number) => {
+          if (rf & RenderFlags.Create) {
+            allocHostVars(1);
+          }
+          if (rf & RenderFlags.Update) {
+            elementProperty(elementIndex, 'title', bind(ctx.title));
+          }
+        },
+        factory: () => subDir = new SubDirective(),
+        features: [InheritDefinitionFeature]
+      });
+    }
+
+    const App = createComponent('app', (rf: RenderFlags, ctx: any) => {
+      if (rf & RenderFlags.Create) {
+        element(0, 'div', ['subDir', '']);
+        element(1, 'div', ['superDir', '']);
+      }
+    }, 2, 0, [SubDirective, SuperDirective]);
+
+    const fixture = new ComponentFixture(App);
+    const els = fixture.hostElement.querySelectorAll('div') as NodeListOf<HTMLElement>;
+
+    const firstDivEl = els[0];
+    const secondDivEl = els[1];
+
+    // checking first div element with inherited directive
+    expect(firstDivEl.id).toEqual('my-id');
+    expect(firstDivEl.title).toEqual('my-title');
+
+    subDir.title = 'new-title';
+    fixture.update();
+    expect(firstDivEl.id).toEqual('my-id');
+    expect(firstDivEl.title).toEqual('new-title');
+
+    subDir.id = 'new-id';
+    fixture.update();
+    expect(firstDivEl.id).toEqual('new-id');
+    expect(firstDivEl.title).toEqual('new-title');
+
+    // checking second div element with simple directive
+    expect(secondDivEl.id).toEqual('my-id');
+
+    superDir.id = 'new-id';
+    fixture.update();
+    expect(secondDivEl.id).toEqual('new-id');
+  });
+
   it('should support host attributes', () => {
     // host: {
     //  'role': 'listbox'