fix(ivy): ensure ngClass works with [class] bindings (angular#26559)

PR Close angular#26559

Author: matsko

packages/core/src/render3/i18n.ts

@@ -6,8 +6,10 @@
  * found in the LICENSE file at https://angular.io/license
  */
 
+import {NO_CHANGE} from '../../src/render3/tokens';
+
 import {assertEqual, assertLessThan} from './assert';
-import {NO_CHANGE, _getViewData, adjustBlueprintForNewNode, bindingUpdated, bindingUpdated2, bindingUpdated3, bindingUpdated4, createNodeAtIndex, getRenderer, load, resetComponentState} from './instructions';
+import {_getViewData, adjustBlueprintForNewNode, bindingUpdated, bindingUpdated2, bindingUpdated3, bindingUpdated4, createNodeAtIndex, getRenderer, load, resetComponentState} from './instructions';
 import {LContainer, NATIVE, RENDER_PARENT} from './interfaces/container';
 import {TElementNode, TNode, TNodeType} from './interfaces/node';
 import {RComment, RElement} from './interfaces/renderer';

packages/core/src/render3/index.ts

@@ -20,9 +20,6 @@ export {CssSelectorList} from './interfaces/projection';
 
 // clang-format off
 export {
-
-  NO_CHANGE,
-
   bind,
   interpolation1,
   interpolation2,
@@ -175,3 +172,5 @@ export {
   renderComponent,
   whenRendered,
 };
+
+export {NO_CHANGE} from './tokens';

packages/core/src/render3/instructions.ts

@@ -24,13 +24,15 @@ import {PlayerFactory} from './interfaces/player';
 import {CssSelectorList, NG_PROJECT_AS_ATTR_NAME} from './interfaces/projection';
 import {LQueries} from './interfaces/query';
 import {ProceduralRenderer3, RComment, RElement, RNode, RText, Renderer3, RendererFactory3, isProceduralRenderer} from './interfaces/renderer';
+import {StylingIndex} from './interfaces/styling';
 import {BINDING_INDEX, CLEANUP, CONTAINER_INDEX, CONTENT_QUERIES, CONTEXT, CurrentMatchesList, DECLARATION_VIEW, FLAGS, HEADER_OFFSET, HOST, HOST_NODE, INJECTOR, LViewData, LViewFlags, NEXT, OpaqueViewState, PARENT, QUERIES, RENDERER, RootContext, RootContextFlags, SANITIZER, TAIL, TVIEW, TView} from './interfaces/view';
 import {assertNodeOfPossibleTypes, assertNodeType} from './node_assert';
 import {appendChild, appendProjectedNode, createTextNode, findComponentView, getLViewChild, getRenderParent, insertView, removeView} from './node_manipulation';
 import {isNodeMatchingSelectorList, matchingSelectorIndex} from './node_selector_matcher';
 import {createStylingContextTemplate, renderStyleAndClassBindings, updateClassProp as updateElementClassProp, updateStyleProp as updateElementStyleProp, updateStylingMap} from './styling/class_and_style_bindings';
 import {BoundPlayerFactory} from './styling/player_factory';
 import {getStylingContext} from './styling/util';
+import {NO_CHANGE} from './tokens';
 import {assertDataInRangeInternal, getComponentViewByIndex, getNativeByIndex, getNativeByTNode, getRootContext, getRootView, getTNode, isComponent, isContentQueryHost, isDifferent, loadInternal, readPatchedLViewData, stringify} from './util';
 
 
@@ -1337,14 +1339,7 @@ export function elementProperty<T>(
   if (value === NO_CHANGE) return;
   const element = getNativeByIndex(index, viewData) as RElement | RComment;
   const tNode = getTNode(index, viewData);
-  // if tNode.inputs is undefined, a listener has created outputs, but inputs haven't
-  // yet been checked
-  if (tNode && tNode.inputs === undefined) {
-    // mark inputs as checked
-    tNode.inputs = generatePropertyAliases(tNode.flags, BindingDirection.Input);
-  }
-
-  const inputData = tNode && tNode.inputs;
+  const inputData = initializeTNodeInputs(tNode);
   let dataValue: PropertyAliasValue|undefined;
   if (inputData && (dataValue = inputData[propName])) {
     setInputsForProperty(dataValue, value);
@@ -1543,14 +1538,28 @@ export function elementStyling(
     styleDeclarations?: (string | boolean | InitialStylingFlags)[] | null,
     styleSanitizer?: StyleSanitizeFn | null): void {
   const tNode = previousOrParentTNode;
+  const inputData = initializeTNodeInputs(tNode);
+
   if (!tNode.stylingTemplate) {
+    const hasClassInput = inputData && inputData.hasOwnProperty('class') ? true : false;
+    if (hasClassInput) {
+      tNode.flags |= TNodeFlags.hasClassInput;
+    }
+
     // initialize the styling template.
-    tNode.stylingTemplate =
-        createStylingContextTemplate(classDeclarations, styleDeclarations, styleSanitizer);
+    tNode.stylingTemplate = createStylingContextTemplate(
+        classDeclarations, styleDeclarations, styleSanitizer, hasClassInput);
   }
+
   if (styleDeclarations && styleDeclarations.length ||
       classDeclarations && classDeclarations.length) {
-    elementStylingApply(tNode.index - HEADER_OFFSET);
+    const index = tNode.index - HEADER_OFFSET;
+    if (delegateToClassInput(tNode)) {
+      const stylingContext = getStylingContext(index, viewData);
+      const initialClasses = stylingContext[StylingIndex.PreviousOrCachedMultiClassValue] as string;
+      setInputsForProperty(tNode.inputs !['class'] !, initialClasses);
+    }
+    elementStylingApply(index);
   }
 }
 
@@ -1640,9 +1649,17 @@ export function elementStyleProp(
  *        removed (unset) from the element's styling.
  */
 export function elementStylingMap<T>(
-    index: number, classes: {[key: string]: any} | string | null,
-    styles?: {[styleName: string]: any} | null): void {
-  updateStylingMap(getStylingContext(index, viewData), classes, styles);
+    index: number, classes: {[key: string]: any} | string | NO_CHANGE | null,
+    styles?: {[styleName: string]: any} | NO_CHANGE | null): void {
+  const tNode = getTNode(index, viewData);
+  const stylingContext = getStylingContext(index, viewData);
+  if (delegateToClassInput(tNode) && classes !== NO_CHANGE) {
+    const initialClasses = stylingContext[StylingIndex.PreviousOrCachedMultiClassValue] as string;
+    const classInputVal =
+        (initialClasses.length ? (initialClasses + ' ') : '') + (classes as string);
+    setInputsForProperty(tNode.inputs !['class'] !, classInputVal);
+  }
+  updateStylingMap(stylingContext, classes, styles);
 }
 
 //////////////////////////
@@ -2577,14 +2594,6 @@ export function markDirty<T>(component: T) {
 //// Bindings & interpolations
 ///////////////////////////////
 
-export interface NO_CHANGE {
-  // This is a brand that ensures that this type can never match anything else
-  brand: 'NO_CHANGE';
-}
-
-/** A special value which designates that a value has not changed. */
-export const NO_CHANGE = {} as NO_CHANGE;
-
 /**
  * Creates a single value binding.
  *
@@ -2871,3 +2880,20 @@ function assertDataNext(index: number, arr?: any[]) {
 }
 
 export const CLEAN_PROMISE = _CLEAN_PROMISE;
+
+function initializeTNodeInputs(tNode: TNode | null) {
+  // If tNode.inputs is undefined, a listener has created outputs, but inputs haven't
+  // yet been checked.
+  if (tNode) {
+    if (tNode.inputs === undefined) {
+      // mark inputs as checked
+      tNode.inputs = generatePropertyAliases(tNode.flags, BindingDirection.Input);
+    }
+    return tNode.inputs;
+  }
+  return null;
+}
+
+export function delegateToClassInput(tNode: TNode) {
+  return tNode.flags & TNodeFlags.hasClassInput;
+}

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

@@ -39,8 +39,11 @@ export const enum TNodeFlags {
   /** This bit is set if the node has any content queries */
   hasContentQuery = 0b00000000000000000100000000000000,
 
+  /** This bit is set if the node has any directives that contain [class properties */
+  hasClassInput = 0b00000000000000001000000000000000,
+
   /** The index of the first directive on this node is encoded on the most significant bits  */
-  DirectiveStartingIndexShift = 15,
+  DirectiveStartingIndexShift = 16,
 }
 
 /**

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

@@ -156,7 +156,7 @@ export interface StylingContext extends Array<InitialStyles|{[key: string]: any}
    * The last class value that was interpreted by elementStylingMap. This is cached
    * So that the algorithm can exit early incase the value has not changed.
    */
-  [StylingIndex.PreviousMultiClassValue]: {[key: string]: any}|string|null;
+  [StylingIndex.PreviousOrCachedMultiClassValue]: {[key: string]: any}|string|null;
 
   /**
    * The last style value that was interpreted by elementStylingMap. This is cached
@@ -181,18 +181,21 @@ export interface InitialStyles extends Array<string|null|boolean> { [0]: null; }
  */
 export const enum StylingFlags {
   // Implies no configurations
-  None = 0b0000,
+  None = 0b00000,
   // Whether or not the entry or context itself is dirty
-  Dirty = 0b0001,
+  Dirty = 0b00001,
   // Whether or not this is a class-based assignment
-  Class = 0b0010,
+  Class = 0b00010,
   // Whether or not a sanitizer was applied to this property
-  Sanitize = 0b0100,
+  Sanitize = 0b00100,
   // Whether or not any player builders within need to produce new players
-  PlayerBuildersDirty = 0b1000,
+  PlayerBuildersDirty = 0b01000,
+  // If NgClass is present (or some other class handler) then it will handle the map expressions and
+  // initial classes
+  OnlyProcessSingleClasses = 0b10000,
   // The max amount of bits used to represent these configuration values
-  BitCountSize = 4,
-  // There are only three bits here
+  BitCountSize = 5,
+  // There are only five bits here
   BitMask = 0b1111
 }
 
@@ -211,8 +214,9 @@ export const enum StylingIndex {
   // Position of where the initial styles are stored in the styling context
   // This index must align with HOST, see interfaces/view.ts
   ElementPosition = 5,
-  // Position of where the last string-based CSS class value was stored
-  PreviousMultiClassValue = 6,
+  // Position of where the last string-based CSS class value was stored (or a cached version of the
+  // initial styles when a [class] directive is present)
+  PreviousOrCachedMultiClassValue = 6,
   // Position of where the last string-based CSS class value was stored
   PreviousMultiStyleValue = 7,
   // Location of single (prop) value entries are stored within the context

packages/core/src/render3/styling/class_and_style_bindings.ts

@@ -11,6 +11,7 @@ import {BindingStore, BindingType, Player, PlayerBuilder, PlayerFactory, PlayerI
 import {Renderer3, RendererStyleFlags3, isProceduralRenderer} from '../interfaces/renderer';
 import {InitialStyles, StylingContext, StylingFlags, StylingIndex} from '../interfaces/styling';
 import {LViewData, RootContext} from '../interfaces/view';
+import {NO_CHANGE} from '../tokens';
 import {getRootContext} from '../util';
 
 import {BoundPlayerFactory} from './player_factory';
@@ -45,7 +46,7 @@ const EMPTY_OBJ: {[key: string]: any} = {};
 export function createStylingContextTemplate(
     initialClassDeclarations?: (string | boolean | InitialStylingFlags)[] | null,
     initialStyleDeclarations?: (string | boolean | InitialStylingFlags)[] | null,
-    styleSanitizer?: StyleSanitizeFn | null): StylingContext {
+    styleSanitizer?: StyleSanitizeFn | null, onlyProcessSingleClasses?: boolean): StylingContext {
   const initialStylingValues: InitialStyles = [null];
   const context: StylingContext =
       createEmptyStylingContext(null, styleSanitizer, initialStylingValues);
@@ -80,6 +81,7 @@ export function createStylingContextTemplate(
   // make where the class offsets begin
   context[StylingIndex.ClassOffsetPosition] = totalStyleDeclarations;
 
+  const initialStaticClasses: string[]|null = onlyProcessSingleClasses ? [] : null;
   if (initialClassDeclarations) {
     let hasPassedDeclarations = false;
     for (let i = 0; i < initialClassDeclarations.length; i++) {
@@ -93,6 +95,7 @@ export function createStylingContextTemplate(
           const value = initialClassDeclarations[++i] as boolean;
           initialStylingValues.push(value);
           classesLookup[className] = initialStylingValues.length - 1;
+          initialStaticClasses && initialStaticClasses.push(className);
         } else {
           classesLookup[className] = 0;
         }
@@ -143,9 +146,15 @@ export function createStylingContextTemplate(
 
   // there is no initial value flag for the master index since it doesn't
   // reference an initial style value
-  setFlag(context, StylingIndex.MasterFlagPosition, pointers(0, 0, multiStart));
+  const masterFlag = pointers(0, 0, multiStart) |
+      (onlyProcessSingleClasses ? StylingFlags.OnlyProcessSingleClasses : 0);
+  setFlag(context, StylingIndex.MasterFlagPosition, masterFlag);
   setContextDirty(context, initialStylingValues.length > 1);
 
+  if (initialStaticClasses) {
+    context[StylingIndex.PreviousOrCachedMultiClassValue] = initialStaticClasses.join(' ');
+  }
+
   return context;
 }
 
@@ -164,8 +173,8 @@ export function createStylingContextTemplate(
  */
 export function updateStylingMap(
     context: StylingContext, classesInput: {[key: string]: any} | string |
-        BoundPlayerFactory<null|string|{[key: string]: any}>| null,
-    stylesInput?: {[key: string]: any} | BoundPlayerFactory<null|{[key: string]: any}>|
+        BoundPlayerFactory<null|string|{[key: string]: any}>| NO_CHANGE | null,
+    stylesInput?: {[key: string]: any} | BoundPlayerFactory<null|{[key: string]: any}>| NO_CHANGE |
         null): void {
   stylesInput = stylesInput || null;
 
@@ -181,13 +190,14 @@ export function updateStylingMap(
       (classesInput as BoundPlayerFactory<{[key: string]: any}|string>) !.value :
       classesInput;
   const stylesValue = stylesPlayerBuilder ? stylesInput !.value : stylesInput;
-
   // early exit (this is what's done to avoid using ctx.bind() to cache the value)
-  const ignoreAllClassUpdates = classesValue === context[StylingIndex.PreviousMultiClassValue];
-  const ignoreAllStyleUpdates = stylesValue === context[StylingIndex.PreviousMultiStyleValue];
+  const ignoreAllClassUpdates = limitToSingleClasses(context) || classesValue === NO_CHANGE ||
+      classesValue === context[StylingIndex.PreviousOrCachedMultiClassValue];
+  const ignoreAllStyleUpdates =
+      stylesValue === NO_CHANGE || stylesValue === context[StylingIndex.PreviousMultiStyleValue];
   if (ignoreAllClassUpdates && ignoreAllStyleUpdates) return;
 
-  context[StylingIndex.PreviousMultiClassValue] = classesValue;
+  context[StylingIndex.PreviousOrCachedMultiClassValue] = classesValue;
   context[StylingIndex.PreviousMultiStyleValue] = stylesValue;
 
   let classNames: string[] = EMPTY_ARR;
@@ -478,6 +488,8 @@ export function renderStyleAndClassBindings(
     const native = context[StylingIndex.ElementPosition] !;
     const multiStartIndex = getMultiStartIndex(context);
     const styleSanitizer = getStyleSanitizer(context);
+    const onlySingleClasses = limitToSingleClasses(context);
+
     for (let i = StylingIndex.SingleStylesStartPosition; i < context.length;
          i += StylingIndex.Size) {
       // there is no point in rendering styles that have not changed on screen
@@ -488,6 +500,7 @@ export function renderStyleAndClassBindings(
         const playerBuilder = getPlayerBuilder(context, i);
         const isClassBased = flag & StylingFlags.Class ? true : false;
         const isInSingleRegion = i < multiStartIndex;
+        const readInitialValue = !isClassBased || !onlySingleClasses;
 
         let valueToApply: string|boolean|null = value;
 
@@ -506,7 +519,7 @@ export function renderStyleAndClassBindings(
         // note that this should always be a falsy check since `false` is used
         // for both class and style comparisons (styles can't be false and false
         // classes are turned off and should therefore defer to their initial values)
-        if (!valueExists(valueToApply, isClassBased)) {
+        if (!valueExists(valueToApply, isClassBased) && readInitialValue) {
           valueToApply = getInitialValue(context, flag);
         }
 
@@ -765,6 +778,10 @@ export function isContextDirty(context: StylingContext): boolean {
   return isDirty(context, StylingIndex.MasterFlagPosition);
 }
 
+export function limitToSingleClasses(context: StylingContext) {
+  return context[StylingIndex.MasterFlagPosition] & StylingFlags.OnlyProcessSingleClasses;
+}
+
 export function setContextDirty(context: StylingContext, isDirtyYes: boolean): void {
   setDirty(context, StylingIndex.MasterFlagPosition, isDirtyYes);
 }

packages/core/src/render3/tokens.ts

@@ -0,0 +1,15 @@
+/**
+ * @license
+ * Copyright Google Inc. All Rights Reserved.
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.io/license
+ */
+
+export interface NO_CHANGE {
+  // This is a brand that ensures that this type can never match anything else
+  brand: 'NO_CHANGE';
+}
+
+/** A special value which designates that a value has not changed. */
+export const NO_CHANGE = {} as NO_CHANGE;

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

@@ -446,6 +446,9 @@
   {
     "name": "defineInjectable"
   },
+  {
+    "name": "delegateToClassInput"
+  },
   {
     "name": "destroyLView"
   },
@@ -740,6 +743,9 @@
   {
     "name": "hasValueChanged"
   },
+  {
+    "name": "initializeTNodeInputs"
+  },
   {
     "name": "inject"
   },
@@ -833,6 +839,9 @@
   {
     "name": "leaveView"
   },
+  {
+    "name": "limitToSingleClasses"
+  },
   {
     "name": "listener"
   },