refactor(ivy): Switch styling to new reconcile algorithm (angular#34616)

NOTE: This change must be reverted with previous deletes so that it code remains in build-able state.

This change deletes old styling code and replaces it with a simplified styling algorithm.

The mental model for the new algorithm is:
- Create a linked list of styling bindings in the order of priority. All styling bindings ere executed in compiled order and than a linked list of bindings is created in priority order.
- Flush the style bindings at the end of `advance()` instruction. This implies that there are two flush events. One at the end of template `advance` instruction in the template. Second one at the end of `hostBindings` `advance` instruction when processing host bindings (if any).
- Each binding instructions effectively updates the string to represent the string at that location. Because most of the bindings are additive, this is a cheap strategy in most cases. In rare cases the strategy requires removing tokens from the styling up to this point. (We expect that to be rare case)S Because, the bindings are presorted in the order of priority, it is safe to resume the processing of the concatenated string from the last change binding.

PR Close angular#34616

Author: mhevery

aio/scripts/_payload-limits.json

@@ -12,7 +12,7 @@
     "master": {
       "uncompressed": {
         "runtime-es2015": 2987,
-        "main-es2015": 462235,
+        "main-es2015": 456890,
         "polyfills-es2015": 52503
       }
     }
@@ -21,7 +21,7 @@
     "master": {
       "uncompressed": {
         "runtime-es2015": 3097,
-        "main-es2015": 438671,
+        "main-es2015": 425216,
         "polyfills-es2015": 52503
       }
     }

integration/_payload-limits.json

@@ -12,7 +12,7 @@
     "master": {
       "uncompressed": {
         "runtime-es2015": 1485,
-        "main-es2015": 18214,
+        "main-es2015": 16787,
         "polyfills-es2015": 36808
       }
     }
@@ -30,7 +30,7 @@
     "master": {
       "uncompressed": {
         "runtime-es2015": 1485,
-        "main-es2015": 139487,
+        "main-es2015": 137226,
         "polyfills-es2015": 37494
       }
     }
@@ -39,7 +39,7 @@
     "master": {
       "uncompressed": {
         "runtime-es2015": 2289,
-        "main-es2015": 268796,
+        "main-es2015": 254857,
         "polyfills-es2015": 36808,
         "5-es2015": 751
       }
@@ -49,7 +49,7 @@
     "master": {
       "uncompressed": {
         "runtime-es2015": 2289,
-        "main-es2015": 228770,
+        "main-es2015": 226519,
         "polyfills-es2015": 36808,
         "5-es2015": 779
       }
@@ -60,7 +60,7 @@
       "uncompressed": {
         "bundle": "TODO(i): temporarily increase the payload size limit from 105779 - this is due to a closure issue related to ESM reexports that still needs to be investigated",
         "bundle": "TODO(i): we should define ngDevMode to false in Closure, but --define only works in the global scope.",
-        "bundle": 176433
+        "bundle": 175498
       }
     }
   }

modules/benchmarks/src/styling/ng2/styling.ts

@@ -18,7 +18,7 @@ import {BrowserModule} from '@angular/platform-browser';
     <ng-template #t3><button [class.bar]="exp ==='bar'"></button></ng-template>
     <ng-template #t4><button class="foo" [class.bar]="exp ==='bar'"></button></ng-template>
     <ng-template #t5><button class="foo" [ngClass]="{bar: exp ==='bar'}"></button></ng-template>
-    <ng-template #t6><button class="foo" [ngStyle]="staticStyle" [style.background-color]="exp"></button></ng-template>
+    <ng-template #t6><button class="foo" [ngStyle]="staticStyle" [style.background-color]="exp == 'bar' ? 'yellow': 'red'"></button></ng-template>
     <ng-template #t7><button style="color: red"></button></ng-template>
     <ng-template #t8><button [style.width.px]="exp ==='bar' ? 10 : 20" [style.color]="exp"></button></ng-template>
     <ng-template #t9><button style="width: 10px" [style.color]="exp"></button></ng-template>

modules/benchmarks/src/tree/render3_function/index.ts

@@ -25,7 +25,7 @@ export class TreeFunction {
     type: TreeFunction,
     selectors: [['tree']],
     decls: 5,
-    vars: 1,
+    vars: 2,
     template: function(rf: ɵRenderFlags, ctx: TreeFunction) {
       // bit of a hack
       TreeTpl(rf, ctx.data);
@@ -34,6 +34,7 @@ export class TreeFunction {
   });
 }
 
+const TreeFunctionCmpDef = TreeFunction.ɵcmp as{decls: number, vars: number};
 export function TreeTpl(rf: ɵRenderFlags, ctx: TreeNode) {
   if (rf & ɵRenderFlags.Create) {
     ɵɵelementStart(0, 'tree');
@@ -54,7 +55,7 @@ export function TreeTpl(rf: ɵRenderFlags, ctx: TreeNode) {
     ɵɵcontainerRefreshStart(3);
     {
       if (ctx.left != null) {
-        let rf0 = ɵɵembeddedViewStart(0, 5, 1);
+        let rf0 = ɵɵembeddedViewStart(0, 5, 2);
         { TreeTpl(rf0, ctx.left); }
         ɵɵembeddedViewEnd();
       }
@@ -63,7 +64,7 @@ export function TreeTpl(rf: ɵRenderFlags, ctx: TreeNode) {
     ɵɵcontainerRefreshStart(4);
     {
       if (ctx.right != null) {
-        let rf0 = ɵɵembeddedViewStart(0, 5, 1);
+        let rf0 = ɵɵembeddedViewStart(0, TreeFunctionCmpDef.decls, TreeFunctionCmpDef.vars);
         { TreeTpl(rf0, ctx.right); }
         ɵɵembeddedViewEnd();
       }

packages/common/src/directives/ng_class.ts

@@ -157,4 +157,11 @@ export class NgClass implements DoCheck {
       });
     }
   }
+
+  // TODO(misko): Delete this code after angula/flex-layout stops depending on private APIs
+  // We need to export this to make angular/flex-layout happy
+  // https://github.com/angular/flex-layout/blob/ec7b57eb6adf59ecfdfff1de5ccf1ab2f6652ed3/src/lib/extended/class/class.ts#L9
+  setClass(value: string) { this.klass = value; }
+  setNgClass(value: any) { this.ngClass = value; }
+  applyChanges() { this.ngDoCheck(); }
 }

packages/common/src/directives/ng_style.ts

@@ -85,4 +85,10 @@ export class NgStyle implements DoCheck {
     changes.forEachAddedItem((record) => this._setStyle(record.key, record.currentValue));
     changes.forEachChangedItem((record) => this._setStyle(record.key, record.currentValue));
   }
+
+  // TODO(misko): Delete this code after angula/flex-layout stops depending on private APIs
+  // We need to export this to make angular/flex-layout happy
+  // https://github.com/angular/flex-layout/blob/ec7b57eb6adf59ecfdfff1de5ccf1ab2f6652ed3/src/lib/extended/class/class.ts#L9
+  setNgStyle(value: any) { this.ngStyle = value; }
+  applyChanges() { this.ngDoCheck(); }
 }

packages/common/src/private_export.ts

@@ -6,5 +6,11 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
+// TODO(misko): Delete this code after angula/flex-layout stops depending on private APIs
+// We need to export this to make angular/flex-layout happy
+// https://github.com/angular/flex-layout/blob/ec7b57eb6adf59ecfdfff1de5ccf1ab2f6652ed3/src/lib/extended/class/class.ts#L9
+export {NgClass as ɵNgClassImpl, NgClass as ɵNgClassR2Impl} from './directives/ng_class';
+export {NgStyle as ɵNgStyleR2Impl} from './directives/ng_style';
+
 export {DomAdapter as ɵDomAdapter, getDOM as ɵgetDOM, setRootDomAdapter as ɵsetRootDomAdapter} from './dom_adapter';
 export {BrowserPlatformLocation as ɵBrowserPlatformLocation} from './location/platform_location';

packages/common/test/directives/ng_class_spec.ts

@@ -196,7 +196,7 @@ import {ComponentFixture, TestBed, async} from '@angular/core/testing';
         fixture = createTestComponent(`<div [ngClass]="['foo', {}]"></div>`);
         expect(() => fixture !.detectChanges())
             .toThrowError(
-                /NgClass can only toggle CSS classes expressed as strings, got: \[object Object\]/);
+                /NgClass can only toggle CSS classes expressed as strings, got \[object Object\]/);
       });
     });
 
@@ -372,15 +372,35 @@ import {ComponentFixture, TestBed, async} from '@angular/core/testing';
         detectChangesAndExpectClassName('color-red');
       });
 
+      it('should allow classes with trailing and leading spaces in [ngClass]', () => {
+        @Component({
+          template: `
+            <div leading-space [ngClass]="{' foo': applyClasses}"></div>
+            <div trailing-space [ngClass]="{'foo ': applyClasses}"></div>
+          `
+        })
+        class Cmp {
+          applyClasses = true;
+        }
+
+        TestBed.configureTestingModule({declarations: [Cmp]});
+        const fixture = TestBed.createComponent(Cmp);
+        fixture.detectChanges();
+
+        const leading = fixture.nativeElement.querySelector('[leading-space]');
+        const trailing = fixture.nativeElement.querySelector('[trailing-space]');
+        expect(leading.className).toBe('foo');
+        expect(trailing.className).toBe('foo');
+      });
+
     });
   });
 }
 
 @Component({selector: 'test-cmp', template: ''})
 class TestComponent {
   condition: boolean = true;
-  // TODO(issue/24571): remove '!'.
-  items !: any[];
+  items: any[]|undefined;
   arrExpr: string[] = ['foo'];
   setExpr: Set<string> = new Set<string>();
   objExpr: {[klass: string]: any}|null = {'foo': true, 'bar': false};

packages/compiler-cli/test/compliance/r3_view_compiler_binding_spec.ts

@@ -856,7 +856,7 @@ describe('compiler compliance: bindings', () => {
           type: HostAttributeDir,
           selectors: [["", "hostAttributeDir", ""]],
           hostAttrs: ["title", "hello there from directive", ${AttributeMarker.Classes}, "one", "two", ${AttributeMarker.Styles}, "width", "200px", "height", "500px"],
-          hostVars: 2,
+          hostVars: 4,
           hostBindings: function HostAttributeDir_HostBindings(rf, ctx, elIndex) {
             …
           }

packages/compiler-cli/test/compliance/r3_view_compiler_styling_spec.ts

@@ -1006,15 +1006,8 @@ describe('compiler compliance: styling', () => {
 
       const template = `
           hostAttrs: [${AttributeMarker.Classes}, "foo", "baz", ${AttributeMarker.Styles}, "width", "200px", "height", "500px"],
-          hostVars: 6,
+          hostVars: 8,
           hostBindings: function MyComponent_HostBindings(rf, ctx, elIndex) {
-<<<<<<< HEAD
-=======
-            if (rf & 1) {
-              $r3$.ɵɵallocHostVars(8);
-              $r3$.ɵɵelementHostAttrs($e0_attrs$);
-            }
->>>>>>> 3a14b06a3b... refactor(ivy): generate 2 slots per styling instruction
             if (rf & 2) {
               $r3$.ɵɵstyleMap(ctx.myStyle, $r3$.ɵɵdefaultStyleSanitizer);
               $r3$.ɵɵclassMap(ctx.myClass);
@@ -1068,14 +1061,8 @@ describe('compiler compliance: styling', () => {
       };
 
       const template = `
-          hostVars: 8,
+          hostVars: 12,
           hostBindings: function MyComponent_HostBindings(rf, ctx, elIndex) {
-<<<<<<< HEAD
-=======
-            if (rf & 1) {
-              $r3$.ɵɵallocHostVars(12);
-            }
->>>>>>> 3a14b06a3b... refactor(ivy): generate 2 slots per styling instruction
             if (rf & 2) {
               $r3$.ɵɵstyleMap(ctx.myStyle, $r3$.ɵɵdefaultStyleSanitizer);
               $r3$.ɵɵclassMap(ctx.myClasses);
@@ -1143,14 +1130,8 @@ describe('compiler compliance: styling', () => {
           `;
 
          const hostBindings = `
-            hostVars: 6,
+            hostVars: 8,
             hostBindings: function MyComponent_HostBindings(rf, ctx, elIndex) {
-<<<<<<< HEAD
-=======
-              if (rf & 1) {
-                $r3$.ɵɵallocHostVars(8);
-              }
->>>>>>> 3a14b06a3b... refactor(ivy): generate 2 slots per styling instruction
               if (rf & 2) {
                 $r3$.ɵɵstyleMap(ctx.myStyleExp, $r3$.ɵɵdefaultStyleSanitizer);
                 $r3$.ɵɵclassMap(ctx.myClassExp);
@@ -1212,22 +1193,23 @@ describe('compiler compliance: styling', () => {
          // NOTE: IF YOU ARE CHANGING THIS COMPILER SPEC, YOU MAY NEED TO CHANGE THE DIRECTIVE
          // DEF THAT'S HARD-CODED IN `ng_class.ts`.
          const template = `
+          …
           hostVars: 2,
           hostBindings: function ClassDirective_HostBindings(rf, ctx, elIndex) {
             if (rf & 2) {
               $r3$.ɵɵclassMap(ctx.myClassMap);
             }
           }
           …
-          hostVars: 2,
+          hostVars: 4,
           hostBindings: function WidthDirective_HostBindings(rf, ctx, elIndex) {
             if (rf & 2) {
               $r3$.ɵɵstyleProp("width", ctx.myWidth);
               $r3$.ɵɵclassProp("foo", ctx.myFooClass);
             }
           }
           …
-          hostVars: 2,
+          hostVars: 4,
           hostBindings: function HeightDirective_HostBindings(rf, ctx, elIndex) {
             if (rf & 2) {
               $r3$.ɵɵstyleProp("height", ctx.myHeight);
@@ -1917,7 +1899,7 @@ describe('compiler compliance: styling', () => {
     };
 
     const template = `
-    hostVars: 4,
+    hostVars: 6,
     hostBindings: function WidthDirective_HostBindings(rf, ctx, elIndex) {
         if (rf & 2) {
           $r3$.ɵɵhostProperty("id", ctx.id)("title", ctx.title);
@@ -2079,14 +2061,8 @@ describe('compiler compliance: styling', () => {
       };
 
       const template = `
-          hostVars: 9,
+          hostVars: 10,
           hostBindings: function MyDir_HostBindings(rf, ctx, elIndex) {
-<<<<<<< HEAD
-=======
-            …
-            $r3$.ɵɵallocHostVars(10);
-            …
->>>>>>> 3a14b06a3b... refactor(ivy): generate 2 slots per styling instruction
             if (rf & 2) {
               $r3$.ɵɵhostProperty("title", ctx.title);
               $r3$.ɵɵupdateSyntheticHostBinding("@anim",

packages/compiler-cli/test/ngtsc/ngtsc_spec.ts

@@ -2340,7 +2340,7 @@ runInEachFileSystem(os => {
       env.driveMain();
       const jsContents = env.getContents('test.js');
       const hostBindingsFn = `
-      hostVars: 3,
+      hostVars: 4,
       hostBindings: function FooCmp_HostBindings(rf, ctx, elIndex) {
         if (rf & 1) {
           i0.ɵɵlistener("click", function FooCmp_click_HostBindingHandler($event) { return ctx.onClick($event); })("click", function FooCmp_click_HostBindingHandler($event) { return ctx.onBodyClick($event); }, false, i0.ɵɵresolveBody)("change", function FooCmp_change_HostBindingHandler($event) { return ctx.onChange(ctx.arg1, ctx.arg2, ctx.arg3); });

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

@@ -524,9 +524,10 @@ function isStyleSanitizable(prop: string): boolean {
   // Note that browsers support both the dash case and
   // camel case property names when setting through JS.
   return prop === 'background-image' || prop === 'backgroundImage' || prop === 'background' ||
-      prop === 'border-image' || prop === 'borderImage' || prop === 'filter' ||
-      prop === 'list-style' || prop === 'listStyle' || prop === 'list-style-image' ||
-      prop === 'listStyleImage' || prop === 'clip-path' || prop === 'clipPath';
+      prop === 'border-image' || prop === 'borderImage' || prop === 'border-image-source' ||
+      prop === 'borderImageSource' || prop === 'filter' || prop === 'list-style' ||
+      prop === 'listStyle' || prop === 'list-style-image' || prop === 'listStyleImage' ||
+      prop === 'clip-path' || prop === 'clipPath';
 }
 
 /**

packages/compiler/test/render3/style_parser_spec.ts

@@ -21,6 +21,11 @@ describe('style parsing', () => {
     expect(result).toEqual(['width', '100px', 'height', '200px', 'opacity', '0']);
   });
 
+  it('should allow empty values', () => {
+    const result = parseStyle('width:;height:   ;');
+    expect(result).toEqual(['width', '', 'height', '']);
+  });
+
   it('should trim values and properties', () => {
     const result = parseStyle('width :333px ; height:666px    ; opacity: 0.5;');
     expect(result).toEqual(['width', '333px', 'height', '666px', 'opacity', '0.5']);

packages/core/src/render3/assert.ts

@@ -73,6 +73,11 @@ export function assertFirstCreatePass(tView: TView, errMessage?: string) {
       tView.firstCreatePass, true, errMessage || 'Should only be called in first create pass.');
 }
 
+export function assertFirstUpdatePass(tView: TView, errMessage?: string) {
+  assertEqual(
+      tView.firstUpdatePass, true, errMessage || 'Should only be called in first update pass.');
+}
+
 /**
  * This is a basic sanity check that an object is probably a directive def. DirectiveDef is
  * an interface, so we can't do a direct instanceof check.

packages/core/src/render3/bindings.ts

@@ -30,7 +30,19 @@ export function getBinding(lView: LView, bindingIndex: number): any {
   return lView[bindingIndex];
 }
 
-/** Updates binding if changed, then returns whether it was updated. */
+/**
+ * Updates binding if changed, then returns whether it was updated.
+ *
+ * This function also checks the `CheckNoChangesMode` and throws if changes are made.
+ * Some changes (Objects/iterables) during `CheckNoChangesMode` are exempt to comply with VE
+ * behavior.
+ *
+ * @param lView current `LView`
+ * @param bindingIndex The binding in the `LView` to check
+ * @param value New value to check against `lView[bindingIndex]`
+ * @returns `true` if the bindings has changed. (Throws if binding has changed during
+ *          `CheckNoChangesMode`)
+ */
 export function bindingUpdated(lView: LView, bindingIndex: number, value: any): boolean {
   ngDevMode && assertNotSame(value, NO_CHANGE, 'Incoming value should never be NO_CHANGE.');
   ngDevMode &&
@@ -50,6 +62,11 @@ export function bindingUpdated(lView: LView, bindingIndex: number, value: any):
         throwErrorIfNoChangesMode(
             oldValue === NO_CHANGE, details.oldValue, details.newValue, details.propName);
       }
+      // There was a change, but the `devModeEqual` decided that the change is exempt from an error.
+      // For this reason we exit as if no change. The early exit is needed to prevent the changed
+      // value to be written into `LView` (If we would write the new value that we would not see it
+      // as change on next CD.)
+      return false;
     }
     lView[bindingIndex] = value;
     return true;