fix(common): cleanup the StylingDiffer and related code (#34307)

Since I was learning the codebase and had a hard time understanding what was going on I've done a
bunch of changes in one commit that under normal circumstances should have been split into several
commits. Because this code is likely going to be overwritten with Misko's changes I'm not going to
spend the time with trying to split this up.

Overall I've done the following:
- I processed review feedback from #34307
- I did a bunch of renaming to make the code easier to understand
- I refactored some internal functions that were either inefficient or hard to read
- I also updated lots of type signatures to correct them and to remove many casts in the code

PR Close #34307

integration/_payload-limits.json

@@ -39,7 +39,7 @@
     "master": {
       "uncompressed": {
         "runtime-es2015": 2289,
-        "main-es2015": 266648,
+        "main-es2015": 267182,
         "polyfills-es2015": 36808,
         "5-es2015": 751
       }

packages/common/src/directives/ng_class_impl.ts

@@ -26,7 +26,7 @@ export abstract class NgClassImpl {
 }
 
 @Injectable()
-export class NgClassR2Impl implements NgClassImpl {
+export class NgClassR2Impl extends NgClassImpl {
   // TODO(issue/24571): remove '!'.
   private _iterableDiffer !: IterableDiffer<string>| null;
   // TODO(issue/24571): remove '!'.
@@ -37,7 +37,9 @@ export class NgClassR2Impl implements NgClassImpl {
 
   constructor(
       private _iterableDiffers: IterableDiffers, private _keyValueDiffers: KeyValueDiffers,
-      private _ngEl: ElementRef, private _renderer: Renderer2) {}
+      private _ngEl: ElementRef, private _renderer: Renderer2) {
+    super();
+  }
 
   getValue() { return null; }
 
@@ -48,7 +50,8 @@ export class NgClassR2Impl implements NgClassImpl {
     this._applyClasses(this._rawClass);
   }
 
-  setNgClass(value: string) {
+
+  setNgClass(value: string|string[]|Set<string>|{[klass: string]: any}) {
     this._removeClasses(this._rawClass);
     this._applyClasses(this._initialClasses);
 
@@ -96,7 +99,7 @@ export class NgClassR2Impl implements NgClassImpl {
         this._toggleClass(record.item, true);
       } else {
         throw new Error(
-            `NgClass can only toggle CSS classes expressed as strings, got ${stringify(record.item)}`);
+            `NgClass can only toggle CSS classes expressed as strings, got: ${stringify(record.item)}`);
       }
     });
 
@@ -150,13 +153,13 @@ export class NgClassR2Impl implements NgClassImpl {
 }
 
 @Injectable()
-export class NgClassR3Impl implements NgClassImpl {
+export class NgClassR3Impl extends NgClassImpl {
   private _value: {[key: string]: boolean}|null = null;
-  private _ngClassDiffer = new StylingDiffer<{[key: string]: boolean}|null>(
+  private _ngClassDiffer = new StylingDiffer<{[key: string]: true}>(
       'NgClass', StylingDifferOptions.TrimProperties|
                  StylingDifferOptions.AllowSubKeys|
                  StylingDifferOptions.AllowStringValue|StylingDifferOptions.ForceAsMap);
-  private _classStringDiffer: StylingDiffer<{[key: string]: boolean}>|null = null;
+  private _classStringDiffer: StylingDiffer<{[key: string]: true}>|null = null;
 
   getValue() { return this._value; }
 
@@ -168,26 +171,23 @@ export class NgClassR3Impl implements NgClassImpl {
     this._classStringDiffer = this._classStringDiffer ||
         new StylingDiffer('class',
                           StylingDifferOptions.AllowStringValue | StylingDifferOptions.ForceAsMap);
-    this._classStringDiffer.setValue(value);
+    this._classStringDiffer.setInput(value);
   }
 
   setNgClass(value: string|string[]|Set<string>|{[klass: string]: any}) {
-    this._ngClassDiffer.setValue(value);
+    this._ngClassDiffer.setInput(value);
   }
 
   applyChanges() {
-    const classChanged =
+    const classChanged = this._classStringDiffer ? this._classStringDiffer.updateValue() : false;
-        this._classStringDiffer ? this._classStringDiffer.hasValueChanged() : false;
+    const ngClassChanged = this._ngClassDiffer.updateValue();
-    const ngClassChanged = this._ngClassDiffer.hasValueChanged();
     if (classChanged || ngClassChanged) {
-      let value = this._ngClassDiffer.value;
+      let ngClassValue = this._ngClassDiffer.value;
-      if (this._classStringDiffer) {
+      let classValue = this._classStringDiffer ? this._classStringDiffer.value : null;
-        let classValue = this._classStringDiffer.value;
+
-        if (classValue) {
+      // merge classValue and ngClassValue and set value
-          value = value ? {...classValue, ...value} : classValue;
+      this._value = (classValue && ngClassValue) ? {...classValue, ...ngClassValue} :
-        }
+                                                   classValue || ngClassValue;
-      }
-      this._value = value;
     }
   }
 }

packages/common/src/directives/ng_style_impl.ts

@@ -83,16 +83,16 @@ export class NgStyleR2Impl implements NgStyleImpl {
 @Injectable()
 export class NgStyleR3Impl implements NgStyleImpl {
   private _differ =
-      new StylingDiffer<{[key: string]: any}|null>('NgStyle', StylingDifferOptions.AllowUnits);
+      new StylingDiffer<{[key: string]: any}>('NgStyle', StylingDifferOptions.AllowUnits);
 
   private _value: {[key: string]: any}|null = null;
 
   getValue() { return this._value; }
 
-  setNgStyle(value: {[key: string]: any}|null) { this._differ.setValue(value); }
+  setNgStyle(value: {[key: string]: any}|null) { this._differ.setInput(value); }
 
   applyChanges() {
-    if (this._differ.hasValueChanged()) {
+    if (this._differ.updateValue()) {
       this._value = this._differ.value;
     }
   }

packages/common/src/directives/styling_differ.ts

@@ -13,7 +13,7 @@
  * of how [style] and [class] behave in Angular.
  *
  * The differences are:
- *  - ngStyle and ngClass both **watch** their binding values for changes each time CD runs
+ *  - ngStyle and ngClass both **deep-watch** their binding values for changes each time CD runs
  *    while [style] and [class] bindings do not (they check for identity changes)
  *  - ngStyle allows for unit-based keys (e.g. `{'max-width.px':value}`) and [style] does not
  *  - ngClass supports arrays of class values and [class] only accepts map and string values
@@ -24,8 +24,9 @@
  * and unnecessary. Instead, ngClass and ngStyle should have their input values be converted
  * into something that the core-level [style] and [class] bindings understand.
  *
- * This [StylingDiffer] class handles this conversion by creating a new input value each time
+ * This [StylingDiffer] class handles this conversion by creating a new output value each time
- * the inner representation of the binding value have changed.
+ * the input value of the binding value has changed (either via identity change or deep collection
+ * content change).
  *
  * ## Why do we care about ngStyle/ngClass?
  * The styling algorithm code (documented inside of `render3/interfaces/styling.ts`) needs to
@@ -40,8 +41,8 @@
  * - If ngStyle/ngClass is used in combination with [style]/[class] bindings then the
  *   styles and classes would fall out of sync and be applied and updated at
  *   inconsistent times
- * - Both ngClass/ngStyle do not respect [class.name] and [style.prop] bindings
+ * - Both ngClass/ngStyle should respect [class.name] and [style.prop] bindings (and not arbitrarily
- *   (they will write over them given the right combination of events)
+ *   overwrite their changes)
  *
  *   ```
  *   <!-- if `w1` is updated then it will always override `w2`
@@ -57,37 +58,38 @@
  * - ngClass/ngStyle were written as a directives and made use of maps, closures and other
  *   expensive data structures which were evaluated each time CD runs
  */
-export class StylingDiffer<T> {
+export class StylingDiffer<T extends({[key: string]: string | null} | {[key: string]: true})> {
+  /**
+   * Normalized string map representing the last value set via `setValue()` or null if no value has
+   * been set or the last set value was null
+   */
   public readonly value: T|null = null;
 
   /**
    * The last set value that was applied via `setValue()`
    */
-  private _lastSetValue: {[key: string]: any}|string|string[]|undefined|null = undefined;
+  private _inputValue: T|string|string[]|Set<string>|null = null;
 
   /**
    * The type of value that the `_lastSetValue` variable is
    */
-  private _lastSetValueType: StylingDifferValueTypes = StylingDifferValueTypes.Null;
+  private _inputValueType: StylingDifferValueTypes = StylingDifferValueTypes.Null;
 
   /**
    * Whether or not the last value change occurred because the variable itself changed reference
    * (identity)
    */
-  private _lastSetValueIdentityChange = false;
+  private _inputValueIdentityChangeSinceLastCheck = false;
 
   constructor(private _name: string, private _options: StylingDifferOptions) {}
 
   /**
-   * Sets (updates) the styling value within the differ.
+   * Sets the input value for the differ and updates the output value if necessary.
    *
-   * Only when `hasValueChanged` is called then this new value will be evaluted
+   * @param value the new styling input value provided from the ngClass/ngStyle binding
-   * and checked against the previous value.
-   *
-   * @param value the new styling value provided from the ngClass/ngStyle binding
    */
-  setValue(value: {[key: string]: any}|string[]|string|null) {
+  setInput(value: T|string[]|string|Set<string>|null): void {
-    if (value !== this._lastSetValue) {
+    if (value !== this._inputValue) {
       let type: StylingDifferValueTypes;
       if (!value) {  // matches empty strings, null, false and undefined
         type = StylingDifferValueTypes.Null;
@@ -105,15 +107,15 @@ export class StylingDiffer<T> {
         type = StylingDifferValueTypes.StringMap;
       }
 
-      this._lastSetValueType = type;
+      this._inputValue = value;
-      this._lastSetValueIdentityChange = true;
+      this._inputValueType = type;
-      this._lastSetValue = value;
+      this._inputValueIdentityChangeSinceLastCheck = true;
       this._processValueChange(true);
     }
   }
 
   /**
-   * Determines whether or not the value has changed.
+   * Checks the input value for identity or deep changes and updates output value if necessary.
    *
    * This function can be called right after `setValue()` is called, but it can also be
    * called incase the existing value (if it's a collection) changes internally. If the
@@ -122,9 +124,10 @@ export class StylingDiffer<T> {
    *
    * @returns whether or not the value has changed in some way.
    */
-  hasValueChanged(): boolean {
+  updateValue(): boolean {
-    let valueHasChanged = this._lastSetValueIdentityChange;
+    let valueHasChanged = this._inputValueIdentityChangeSinceLastCheck;
-    if (!valueHasChanged && (this._lastSetValueType & StylingDifferValueTypes.Collection)) {
+    if (!this._inputValueIdentityChangeSinceLastCheck &&
+        (this._inputValueType & StylingDifferValueTypes.Collection)) {
       valueHasChanged = this._processValueChange(false);
     } else {
       // this is set to false in the event that the value is a collection.
@@ -132,86 +135,89 @@ export class StylingDiffer<T> {
       // diff the collection value to see if the contents have mutated
       // (otherwise the value change was processed during the time when
       // the variable changed).
-      this._lastSetValueIdentityChange = false;
+      this._inputValueIdentityChangeSinceLastCheck = false;
     }
     return valueHasChanged;
   }
 
   /**
-   * Examines the last set value to see if there was a change in data.
+   * Examines the last set value to see if there was a change in content.
    *
-   * @param hasIdentityChange whether or not the last set value changed in identity or not
+   * @param inputValueIdentityChanged whether or not the last set value changed in identity or not
-   *
+   * @returns `true` when the value has changed (either by identity or by shape if its a
-   * @returns true when the value has changed (either by identity or by shape if its a collection).
+   * collection)
    */
-  private _processValueChange(hasIdentityChange: boolean) {
+  private _processValueChange(inputValueIdentityChanged: boolean): boolean {
-    let valueHasChanged = hasIdentityChange;
+    // if the inputValueIdentityChanged then we know that input has changed
+    let inputChanged = inputValueIdentityChanged;
 
-    let finalValue: {[key: string]: any}|string|null = null;
+    let newOutputValue: T|string|null = null;
     const trimValues = (this._options & StylingDifferOptions.TrimProperties) ? true : false;
     const parseOutUnits = (this._options & StylingDifferOptions.AllowUnits) ? true : false;
     const allowSubKeys = (this._options & StylingDifferOptions.AllowSubKeys) ? true : false;
 
-    switch (this._lastSetValueType) {
+    switch (this._inputValueType) {
       // case 1: [input]="string"
-      case StylingDifferValueTypes.String:
+      case StylingDifferValueTypes.String: {
-        const tokens = (this._lastSetValue as string).split(/\s+/g);
+        if (inputValueIdentityChanged) {
-        if (this._options & StylingDifferOptions.ForceAsMap) {
+          // process string input only if the identity has changed since the strings are immutable
-          finalValue = {} as{[key: string]: any};
+          const keys = (this._inputValue as string).split(/\s+/g);
-          for (let i = 0; i < tokens.length; i++) {
+          if (this._options & StylingDifferOptions.ForceAsMap) {
-            finalValue[tokens[i]] = true;
+            newOutputValue = {} as T;
-          }
+            for (let i = 0; i < keys.length; i++) {
-        } else {
+              (newOutputValue as any)[keys[i]] = true;
-          finalValue = '';
+            }
-          for (let i = 0; i < tokens.length; i++) {
+          } else {
-            finalValue += (i !== 0 ? ' ' : '') + tokens[i];
+            newOutputValue = keys.join(' ');
           }
         }
         break;
-
+      }
       // case 2: [input]="{key:value}"
-      case StylingDifferValueTypes.StringMap:
+      case StylingDifferValueTypes.StringMap: {
-        const map: {[key: string]: any} = this._lastSetValue as{[key: string]: any};
+        const inputMap = this._inputValue as T;
-        const keys = Object.keys(map);
+        const inputKeys = Object.keys(inputMap);
-        if (!valueHasChanged) {
+
-          // we know that the classExp value exists and that it is
+        if (!inputValueIdentityChanged) {
-          // a map (otherwise an identity change would have occurred)
+          // if StringMap and the identity has not changed then output value must have already been
-          valueHasChanged =
+          // initialized to a StringMap, so we can safely compare the input and output maps
-              this.value ? mapHasChanged(keys, this.value as{[key: string]: any}, map) : true;
+          inputChanged = mapsAreEqual(inputKeys, inputMap, this.value as T);
         }
 
-        if (valueHasChanged) {
+        if (inputChanged) {
-          finalValue =
+          newOutputValue = bulidMapFromStringMap(
-              bulidMapFromValues(this._name, trimValues, parseOutUnits, allowSubKeys, map, keys);
+              trimValues, parseOutUnits, allowSubKeys, inputMap, inputKeys) as T;
         }
         break;
-
+      }
       // case 3a: [input]="[str1, str2, ...]"
       // case 3b: [input]="Set"
       case StylingDifferValueTypes.Array:
-      case StylingDifferValueTypes.Set:
+      case StylingDifferValueTypes.Set: {
-        const values = Array.from(this._lastSetValue as string[] | Set<string>);
+        const inputKeys = Array.from(this._inputValue as string[] | Set<string>);
-        if (!valueHasChanged) {
+        if (!inputValueIdentityChanged) {
-          const keys = Object.keys(this.value !);
+          const outputKeys = Object.keys(this.value !);
-          valueHasChanged = !arrayEqualsArray(keys, values);
+          inputChanged = !keyArraysAreEqual(outputKeys, inputKeys);
         }
-        if (valueHasChanged) {
+        if (inputChanged) {
-          finalValue =
+          newOutputValue =
-              bulidMapFromValues(this._name, trimValues, parseOutUnits, allowSubKeys, values);
+              bulidMapFromStringArray(this._name, trimValues, allowSubKeys, inputKeys) as T;
         }
         break;
-
+      }
       // case 4: [input]="null|undefined"
       default:
-        finalValue = null;
+        inputChanged = inputValueIdentityChanged;
+        newOutputValue = null;
         break;
     }
 
-    if (valueHasChanged) {
+    if (inputChanged) {
-      (this as any).value = finalValue !;
+      // update the readonly `value` property by casting it to `any` first
+      (this as any).value = newOutputValue;
     }
 
-    return valueHasChanged;
+    return inputChanged;
   }
 }
 
@@ -241,34 +247,55 @@ const enum StylingDifferValueTypes {
 
 
 /**
- * builds and returns a map based on the values input value
+ * @param trim whether the keys should be trimmed of leading or trailing whitespace
- *
+ * @param parseOutUnits whether units like "px" should be parsed out of the key name and appended to
- * If the `keys` param is provided then the `values` param is treated as a
+ *   the value
- * string map. Otherwise `values` is treated as a string array.
+ * @param allowSubKeys whether key needs to be subsplit by whitespace into multiple keys
+ * @param values values of the map
+ * @param keys keys of the map
+ * @return a normalized string map based on the input string map
  */
-function bulidMapFromValues(
+function bulidMapFromStringMap(
-    errorPrefix: string, trim: boolean, parseOutUnits: boolean, allowSubKeys: boolean,
+    trim: boolean, parseOutUnits: boolean, allowSubKeys: boolean,
-    values: {[key: string]: any} | string[], keys?: string[]) {
+    values: {[key: string]: string | null | true},
-  const map: {[key: string]: any} = {};
+    keys: string[]): {[key: string]: string | null | true} {
-  if (keys) {
+  const map: {[key: string]: string | null | true} = {};
-    // case 1: map
-    for (let i = 0; i < keys.length; i++) {
-      let key = keys[i];
-      const value = (values as{[key: string]: any})[key];
 
-      if (value !== undefined) {
+  for (let i = 0; i < keys.length; i++) {
-        // Map uses untrimmed keys, so don't trim until passing to `setMapValues`
+    let key = keys[i];
-        setMapValues(map, trim ? key.trim() : key, value, parseOutUnits, allowSubKeys);
+    let value = values[key];
+
+    if (value !== undefined) {
+      if (typeof value !== 'boolean') {
+        value = '' + value;
       }
+      // Map uses untrimmed keys, so don't trim until passing to `setMapValues`
+      setMapValues(map, trim ? key.trim() : key, value, parseOutUnits, allowSubKeys);
     }
-  } else {
+  }
-    // case 2: array
+
-    for (let i = 0; i < values.length; i++) {
+  return map;
-      let value = (values as string[])[i];
+}
-      assertValidValue(errorPrefix, value);
+
-      value = trim ? value.trim() : value;
+/**
-      setMapValues(map, value, true, false, allowSubKeys);
+ * @param trim whether the keys should be trimmed of leading or trailing whitespace
-    }
+ * @param parseOutUnits whether units like "px" should be parsed out of the key name and appended to
+ *   the value
+ * @param allowSubKeys whether key needs to be subsplit by whitespace into multiple keys
+ * @param values values of the map
+ * @param keys keys of the map
+ * @return a normalized string map based on the input string array
+ */
+function bulidMapFromStringArray(
+    errorPrefix: string, trim: boolean, allowSubKeys: boolean,
+    keys: string[]): {[key: string]: true} {
+  const map: {[key: string]: true} = {};
+
+  for (let i = 0; i < keys.length; i++) {
+    let key = keys[i];
+    ngDevMode && assertValidValue(errorPrefix, key);
+    key = trim ? key.trim() : key;
+    setMapValues(map, key, true, false, allowSubKeys);
   }
 
   return map;
@@ -277,12 +304,12 @@ function bulidMapFromValues(
 function assertValidValue(errorPrefix: string, value: any) {
   if (typeof value !== 'string') {
     throw new Error(
-        `${errorPrefix} can only toggle CSS classes expressed as strings, got ${value}`);
+        `${errorPrefix} can only toggle CSS classes expressed as strings, got: ${value}`);
   }
 }
 
 function setMapValues(
-    map: {[key: string]: any}, key: string, value: any, parseOutUnits: boolean,
+    map: {[key: string]: unknown}, key: string, value: string | null | true, parseOutUnits: boolean,
     allowSubKeys: boolean) {
   if (allowSubKeys && key.indexOf(' ') > 0) {
     const innerKeys = key.split(/\s+/g);
@@ -295,39 +322,41 @@ function setMapValues(
 }
 
 function setIndividualMapValue(
-    map: {[key: string]: any}, key: string, value: any, parseOutUnits: boolean) {
+    map: {[key: string]: unknown}, key: string, value: string | true | null,
-  if (parseOutUnits) {
+    parseOutUnits: boolean) {
-    const values = normalizeStyleKeyAndValue(key, value);
+  if (parseOutUnits && typeof value === 'string') {
-    value = values.value;
+    // parse out the unit (e.g. ".px") from the key and append it to the value
-    key = values.key;
+    // e.g. for [width.px]="40" => ["width","40px"]
+    const unitIndex = key.indexOf('.');
+    if (unitIndex > 0) {
+      const unit = key.substr(unitIndex + 1);  // skip over the "." in "width.px"
+      key = key.substring(0, unitIndex);
+      value += unit;
+    }
   }
   map[key] = value;
 }
 
-function normalizeStyleKeyAndValue(key: string, value: string | null) {
-  const index = key.indexOf('.');
-  if (index > 0) {
-    const unit = key.substr(index + 1);  // ignore the . ([width.px]="'40'" => "40px")
-    key = key.substring(0, index);
-    if (value != null) {  // we should not convert null values to string
-      value += unit;
-    }
-  }
-  return {key, value};
-}
 
-function mapHasChanged(keys: string[], a: {[key: string]: any}, b: {[key: string]: any}) {
+/**
-  const oldKeys = Object.keys(a);
+ * Compares two maps and returns true if they are equal
-  const newKeys = keys;
+ *
+ * @param inputKeys value of `Object.keys(inputMap)` it's unclear if this actually performs better
+ * @param inputMap map to compare
+ * @param outputMap map to compare
+ */
+function mapsAreEqual(
+    inputKeys: string[], inputMap: {[key: string]: unknown},
+    outputMap: {[key: string]: unknown}, ): boolean {
+  const outputKeys = Object.keys(outputMap);
 
-  // the keys are different which means the map changed
+  if (inputKeys.length !== outputKeys.length) {
-  if (!arrayEqualsArray(oldKeys, newKeys)) {
     return true;
   }
 
-  for (let i = 0; i < newKeys.length; i++) {
+  for (let i = 0, n = inputKeys.length; i <= n; i++) {
-    const key = newKeys[i];
+    let key = inputKeys[i];
-    if (a[key] !== b[key]) {
+    if (key !== outputKeys[i] || inputMap[key] !== outputMap[key]) {
       return true;
     }
   }
@@ -335,13 +364,27 @@ function mapHasChanged(keys: string[], a: {[key: string]: any}, b: {[key: string
   return false;
 }
 
-function arrayEqualsArray(a: any[] | null, b: any[] | null) {
+
-  if (a && b) {
+/**
-    if (a.length !== b.length) return false;
+ * Compares two Object.keys() arrays and returns true if they are equal.
-    for (let i = 0; i < a.length; i++) {
+ *
-      if (b.indexOf(a[i]) === -1) return false;
+ * @param keyArray1 Object.keys() array to compare
-    }
+ * @param keyArray1 Object.keys() array to compare
-    return true;
+ */
+function keyArraysAreEqual(keyArray1: string[] | null, keyArray2: string[] | null): boolean {
+  if (!Array.isArray(keyArray1) || !Array.isArray(keyArray2)) {
+    return false;
   }
-  return false;
+
+  if (keyArray1.length !== keyArray2.length) {
+    return false;
+  }
+
+  for (let i = 0; i < keyArray1.length; i++) {
+    if (keyArray1[i] !== keyArray2[i]) {
+      return false;
+    }
+  }
+
+  return true;
 }

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\]/);
       });
     });
 
@@ -353,17 +353,23 @@ import {ComponentFixture, TestBed, async} from '@angular/core/testing';
          }));
     });
 
-    describe('non-regression', () => {
+    describe('prevent regressions', () => {
 
       // https://github.com/angular/angular/issues/34336
-      it('should not write to native node when a bound expression doesnt change', () => {
+      it('should not write to the native node unless the bound expression has changed', () => {
-        fixture = createTestComponent(`<div [ngClass]="{'color-red': true}"></div>`);
+        fixture = createTestComponent(`<div [ngClass]="{'color-red': condition}"></div>`);
         detectChangesAndExpectClassName('color-red');
 
-        // Overwrite CSS classes to make sure that ngClass is not doing any DOM manipulation (as
+        // Overwrite CSS classes so that we can check if ngClass performed DOM manipulation to
-        // there was no change to the expression bound to [ngClass]).
+        // update it
         fixture.debugElement.children[0].nativeElement.className = '';
+        // Assert that the DOM node still has the same value after change detection
         detectChangesAndExpectClassName('');
+
+        fixture.componentInstance.condition = false;
+        fixture.detectChanges();
+        fixture.componentInstance.condition = true;
+        detectChangesAndExpectClassName('color-red');
       });
 
     });

packages/common/test/directives/ng_style_spec.ts

@@ -12,7 +12,7 @@ import {ComponentFixture, TestBed, async} from '@angular/core/testing';
 
 {
   describe('NgStyle', () => {
-    let fixture: ComponentFixture<any>;
+    let fixture: ComponentFixture<TestComponent>;
 
     function getComponent(): TestComponent { return fixture.componentInstance; }
 
@@ -158,21 +158,36 @@ import {ComponentFixture, TestBed, async} from '@angular/core/testing';
          expectNativeEl(fixture).toHaveCssStyle({'font-size': '12px'});
        }));
 
-    it('should not write to native node when a bound expression doesnt change', () => {
+    it('should not write to the native node unless the bound expression has changed', () => {
 
-      const template = `<div [ngStyle]="{'color': 'red'}"></div>`;
+      const template = `<div [ngStyle]="{'color': expr}"></div>`;
 
       fixture = createTestComponent(template);
+      fixture.componentInstance.expr = 'red';
 
       fixture.detectChanges();
       expectNativeEl(fixture).toHaveCssStyle({'color': 'red'});
 
-      // Overwrite native styles to make sure that ngClass is not doing any DOM manipulation (as
+      // Overwrite native styles so that we can check if ngStyle has performed DOM manupulation to
-      // there was no change to the expression bound to [ngStyle]).
+      // update it.
       fixture.debugElement.children[0].nativeElement.style.color = 'blue';
       fixture.detectChanges();
+      // Assert that the style hasn't been updated
       expectNativeEl(fixture).toHaveCssStyle({'color': 'blue'});
 
+      fixture.componentInstance.expr = 'yellow';
+      fixture.detectChanges();
+      // Assert that the style has changed now that the model has changed
+      expectNativeEl(fixture).toHaveCssStyle({'color': 'yellow'});
+    });
+
+    it('should correctly update style with units (.px) when the model is set to number', () => {
+      const template = `<div [ngStyle]="{'width.px': expr}"></div>`;
+      fixture = createTestComponent(template);
+      fixture.componentInstance.expr = 400;
+
+      fixture.detectChanges();
+      expectNativeEl(fixture).toHaveCssStyle({'width': '400px'});
     });
 
   });

packages/common/test/directives/styling_differ_spec.ts

@@ -13,101 +13,111 @@ describe('StylingDiffer', () => {
         'ngClass', StylingDifferOptions.ForceAsMap | StylingDifferOptions.AllowStringValue);
     expect(d.value).toEqual(null);
 
-    d.setValue('one two');
+    d.setInput('one two');
     expect(d.value).toEqual({one: true, two: true});
 
-    d.setValue('three');
+    d.setInput('three');
     expect(d.value).toEqual({three: true});
   });
 
-  it('should not emit that a value has changed if a new non-collection value was not set', () => {
-    const d = new StylingDiffer(
-        'ngClass', StylingDifferOptions.ForceAsMap | StylingDifferOptions.AllowStringValue);
-    expect(d.value).toEqual(null);
 
-    d.setValue('one two');
+  describe('setInput', () => {
-    expect(d.hasValueChanged()).toBeTruthy();
-    expect(d.value).toEqual({one: true, two: true});
-    expect(d.hasValueChanged()).toBeFalsy();
-    expect(d.value).toEqual({one: true, two: true});
 
-    d.setValue('three');
+    it('should not emit that a value has changed if a new non-collection value was not set', () => {
-    expect(d.hasValueChanged()).toBeTruthy();
+      const d = new StylingDiffer(
-    expect(d.value).toEqual({three: true});
+          'ngClass', StylingDifferOptions.ForceAsMap | StylingDifferOptions.AllowStringValue);
-    expect(d.hasValueChanged()).toBeFalsy();
+      expect(d.value).toEqual(null);
-    expect(d.value).toEqual({three: true});
 
-    d.setValue(null);
+      d.setInput('one two');
-    expect(d.hasValueChanged()).toBeTruthy();
+      expect(d.updateValue()).toBeTruthy();
-    expect(d.value).toEqual(null);
+      expect(d.value).toEqual({one: true, two: true});
-    expect(d.hasValueChanged()).toBeFalsy();
+      expect(d.updateValue()).toBeFalsy();
-    expect(d.value).toEqual(null);
+      expect(d.value).toEqual({one: true, two: true});
+
+      d.setInput('three');
+      expect(d.updateValue()).toBeTruthy();
+      expect(d.value).toEqual({three: true});
+      expect(d.updateValue()).toBeFalsy();
+      expect(d.value).toEqual({three: true});
+
+      d.setInput(null);
+      expect(d.updateValue()).toBeTruthy();
+      expect(d.value).toEqual(null);
+      expect(d.updateValue()).toBeFalsy();
+      expect(d.value).toEqual(null);
+    });
   });
 
-  it('should watch the contents of a StringMap value and emit new values if they change', () => {
-    const d = new StylingDiffer('ngClass', StylingDifferOptions.ForceAsMap);
 
-    const myMap: {[key: string]: any} = {};
+  describe('updateValue', () => {
-    myMap['abc'] = true;
 
-    d.setValue(myMap);
+    it('should update the differ value if the contents of a input StringMap change', () => {
-    expect(d.hasValueChanged()).toBeTruthy();
+      const d = new StylingDiffer('ngClass', StylingDifferOptions.ForceAsMap);
-    expect(d.value).toEqual({abc: true});
-    expect(d.hasValueChanged()).toBeFalsy();
 
-    myMap['def'] = true;
+      const myMap: {[key: string]: true} = {};
-    expect(d.hasValueChanged()).toBeTruthy();
+      myMap['abc'] = true;
-    expect(d.value).toEqual({abc: true, def: true});
-    expect(d.hasValueChanged()).toBeFalsy();
 
-    delete myMap['abc'];
+      d.setInput(myMap);
-    delete myMap['def'];
+      expect(d.updateValue()).toBeTruthy();
-    expect(d.hasValueChanged()).toBeTruthy();
+      expect(d.value).toEqual({abc: true});
-    expect(d.value).toEqual({});
+      expect(d.updateValue()).toBeFalsy();
-    expect(d.hasValueChanged()).toBeFalsy();
-  });
 
-  it('should watch the contents of an Array value and emit new values if they change', () => {
+      myMap['def'] = true;
-    const d = new StylingDiffer('ngClass', StylingDifferOptions.ForceAsMap);
+      expect(d.updateValue()).toBeTruthy();
+      expect(d.value).toEqual({abc: true, def: true});
+      expect(d.updateValue()).toBeFalsy();
 
-    const myArray: string[] = [];
+      delete myMap['abc'];
-    myArray.push('abc');
+      delete myMap['def'];
+      expect(d.updateValue()).toBeTruthy();
+      expect(d.value).toEqual({});
+      expect(d.updateValue()).toBeFalsy();
+    });
 
-    d.setValue(myArray);
-    expect(d.hasValueChanged()).toBeTruthy();
-    expect(d.value).toEqual({abc: true});
-    expect(d.hasValueChanged()).toBeFalsy();
 
-    myArray.push('def');
+    it('should update the differ value if the contents of an input Array change', () => {
-    expect(d.hasValueChanged()).toBeTruthy();
+      const d = new StylingDiffer('ngClass', StylingDifferOptions.ForceAsMap);
-    expect(d.value).toEqual({abc: true, def: true});
-    expect(d.hasValueChanged()).toBeFalsy();
 
-    myArray.length = 0;
+      const myArray: string[] = [];
-    expect(d.hasValueChanged()).toBeTruthy();
+      myArray.push('abc');
-    expect(d.value).toEqual({});
-    expect(d.hasValueChanged()).toBeFalsy();
-  });
 
-  it('should watch the contents of a Set value and emit new values if they change', () => {
+      d.setInput(myArray);
-    const d = new StylingDiffer('ngClass', StylingDifferOptions.ForceAsMap);
+      expect(d.updateValue()).toBeTruthy();
+      expect(d.value).toEqual({abc: true});
+      expect(d.updateValue()).toBeFalsy();
 
-    const mySet = new Set<string>();
+      myArray.push('def');
-    mySet.add('abc');
+      expect(d.updateValue()).toBeTruthy();
+      expect(d.value).toEqual({abc: true, def: true});
+      expect(d.updateValue()).toBeFalsy();
 
-    d.setValue(mySet);
+      myArray.length = 0;
-    expect(d.hasValueChanged()).toBeTruthy();
+      expect(d.updateValue()).toBeTruthy();
-    expect(d.value).toEqual({abc: true});
+      expect(d.value).toEqual({});
-    expect(d.hasValueChanged()).toBeFalsy();
+      expect(d.updateValue()).toBeFalsy();
+    });
 
-    mySet.add('def');
-    expect(d.hasValueChanged()).toBeTruthy();
-    expect(d.value).toEqual({abc: true, def: true});
-    expect(d.hasValueChanged()).toBeFalsy();
 
-    mySet.clear();
+    it('should update the differ value if the contents of an input Set change', () => {
-    expect(d.hasValueChanged()).toBeTruthy();
+      const d = new StylingDiffer('ngClass', StylingDifferOptions.ForceAsMap);
-    expect(d.value).toEqual({});
+
-    expect(d.hasValueChanged()).toBeFalsy();
+      const mySet = new Set<string>();
+      mySet.add('abc');
+
+      d.setInput(mySet);
+      expect(d.updateValue()).toBeTruthy();
+      expect(d.value).toEqual({abc: true});
+      expect(d.updateValue()).toBeFalsy();
+
+      mySet.add('def');
+      expect(d.updateValue()).toBeTruthy();
+      expect(d.value).toEqual({abc: true, def: true});
+      expect(d.updateValue()).toBeFalsy();
+
+      mySet.clear();
+      expect(d.updateValue()).toBeTruthy();
+      expect(d.value).toEqual({});
+      expect(d.updateValue()).toBeFalsy();
+    });
   });
 });

packages/compiler-cli/ngcc/test/integration/ngcc_spec.ts

@@ -257,8 +257,7 @@ runInEachFileSystem(() => {
 
          // We need to make sure that the flat typings file exports this directly
          const dtsContents = fs.readFile(_('/node_modules/@angular/common/common.d.ts'));
-         expect(dtsContents)
+         expect(dtsContents).toContain(`export declare class ${exportedName} extends ɵNgClassImpl`);
-             .toContain(`export declare class ${exportedName} implements ɵNgClassImpl`);
          // And that ngcc's modifications to that class use the correct (exported) name
          expect(dtsContents).toContain(`static ɵprov: ɵngcc0.ɵɵInjectableDef<${exportedName}>`);
        });