From ab931cf872668ff5d14d35bb9b7831387bf44ea3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Mi=C5=A1ko=20Hevery?= <misko@hevery.com>
Date: Mon, 3 Feb 2020 12:23:00 -0800
Subject: [PATCH] fix(ivy): host-styling throws assert exception inside *ngFor
 (#35133)

Inside `*ngFor` the second run of the styling instructions can get into situation where it tries to read a value from a binding which has not yet executed. As a result the read is `NO_CHANGE` value and subsequent property read cause an exception as it is of wrong type.

Fix #35118

PR Close #35133
---
 .../core/src/render3/instructions/styling.ts  | 16 ++++++-
 packages/core/test/acceptance/styling_spec.ts | 46 +++++++++++++++++++
 .../bundling/todo/bundle.golden_symbols.json  |  3 ++
 3 files changed, 63 insertions(+), 2 deletions(-)

diff --git a/packages/core/src/render3/instructions/styling.ts b/packages/core/src/render3/instructions/styling.ts
index 85ad742bda..b07fb5499b 100644
--- a/packages/core/src/render3/instructions/styling.ts
+++ b/packages/core/src/render3/instructions/styling.ts
@@ -834,8 +834,20 @@ function findStylingValue(
     const containsStatics = Array.isArray(rawKey);
     // Unwrap the key if we contain static values.
     const key = containsStatics ? (rawKey as string[])[1] : rawKey;
-    let currentValue = key === null ? keyValueArrayGet(lView[index + 1], prop) :
-                                      key === prop ? lView[index + 1] : undefined;
+    const isStylingMap = key === null;
+    let valueAtLViewIndex = lView[index + 1];
+    if (valueAtLViewIndex === NO_CHANGE) {
+      // In firstUpdatePass the styling instructions create a linked list of styling.
+      // On subsequent passes it is possible for a styling instruction to try to read a binding
+      // which
+      // has not yet executed. In that case we will find `NO_CHANGE` and we should assume that
+      // we have `undefined` (or empty array in case of styling-map instruction) instead. This
+      // allows the resolution to apply the value (which may later be overwritten when the
+      // binding actually executes.)
+      valueAtLViewIndex = isStylingMap ? EMPTY_ARRAY : undefined;
+    }
+    let currentValue = isStylingMap ? keyValueArrayGet(valueAtLViewIndex, prop) :
+                                      key === prop ? valueAtLViewIndex : undefined;
     if (containsStatics && !isStylingValuePresent(currentValue)) {
       currentValue = keyValueArrayGet(rawKey as KeyValueArray<any>, prop);
     }
diff --git a/packages/core/test/acceptance/styling_spec.ts b/packages/core/test/acceptance/styling_spec.ts
index 0c3bbdc5c8..cee91e5718 100644
--- a/packages/core/test/acceptance/styling_spec.ts
+++ b/packages/core/test/acceptance/styling_spec.ts
@@ -3273,6 +3273,52 @@ describe('styling', () => {
 
     expect(logs).toEqual([]);
   });
+
+  describe('regression', () => {
+    onlyInIvy('styling priority resolution is Ivy only feature.')
+        .it('should allow lookahead binding on second pass #35118', () => {
+          @Component({
+            selector: 'my-cmp',
+            template: ``,
+            host: {
+              '[class.foo]': 'hostClass',
+            }
+          })
+          class MyCmp {
+            hostClass = true;
+          }
+
+          @Directive({
+            selector: '[host-styling]',
+            host: {
+              '[class]': 'hostClass',
+            }
+          })
+          class HostStylingsDir {
+            hostClass = {'bar': true};
+          }
+
+          @Component({template: `<my-cmp *ngFor="let i of [1,2]" host-styling></my-cmp>`})
+          class MyApp {
+            // When the first view in the list gets CD-ed, everything works.
+            // When the second view gets CD-ed, the styling has already created the data structures
+            // in the `TView`. As a result when
+            // `[class.foo]` runs it already knows that `[class]` is a duplicate and hence it
+            // should check with it. While the resolution is happening it reads the value of the
+            // `[class]`, however `[class]` has not yet executed and therefore it does not have
+            // normalized value in its `LView`. The result is that the assertions fails as it
+            // expects an
+            // `KeyValueArray`.
+          }
+
+          TestBed.configureTestingModule({declarations: [MyApp, MyCmp, HostStylingsDir]});
+          const fixture = TestBed.createComponent(MyApp);
+          expect(() => fixture.detectChanges()).not.toThrow();
+          const [cmp1, cmp2] = fixture.nativeElement.querySelectorAll('my-cmp');
+          expectClass(cmp1).toEqual({foo: true, bar: true});
+          expectClass(cmp2).toEqual({foo: true, bar: true});
+        });
+  });
 });
 
 function assertStyleCounters(countForSet: number, countForRemove: number) {
diff --git a/packages/core/test/bundling/todo/bundle.golden_symbols.json b/packages/core/test/bundling/todo/bundle.golden_symbols.json
index f8511aa06b..5898645355 100644
--- a/packages/core/test/bundling/todo/bundle.golden_symbols.json
+++ b/packages/core/test/bundling/todo/bundle.golden_symbols.json
@@ -47,6 +47,9 @@
   {
     "name": "EMPTY_ARRAY"
   },
+  {
+    "name": "EMPTY_ARRAY"
+  },
   {
     "name": "EMPTY_OBJ"
   },