fix(ivy): ensure parent/sub-class components evaluate styling correctly (angular#29602)

The new styling algorithm in angular is designed to evaluate host
bindings stylinh priority in order of directive evaluation order. This,
however, does not work with respect to parent/sub-class directives
because sub-class host bindings are run after the parent host bindings
but still have priority. This patch ensures that the host styling bindings
for parent and sub-class components/directives are executed with respect
to the styling algorithm prioritization.

Jira Issue: FW-1132

PR Close angular#29602

Author: matsko

integration/_payload-limits.json

@@ -12,7 +12,7 @@
     "master": {
       "uncompressed": {
         "runtime": 1440,
-        "main": 14106,
+        "main": 14287,
         "polyfills": 43567
       }
     }

packages/core/src/render3/component.ts

@@ -23,7 +23,7 @@ import {PlayerHandler} from './interfaces/player';
 import {RElement, Renderer3, RendererFactory3, domRendererFactory3} from './interfaces/renderer';
 import {CONTEXT, FLAGS, HEADER_OFFSET, HOST, LView, LViewFlags, RENDERER, RootContext, RootContextFlags, TVIEW} from './interfaces/view';
 import {applyOnCreateInstructions} from './node_util';
-import {enterView, getPreviousOrParentTNode, leaveView, resetComponentState} from './state';
+import {enterView, getPreviousOrParentTNode, leaveView, resetComponentState, setActiveHostElement} from './state';
 import {renderInitialClasses, renderInitialStyles} from './styling/class_and_style_bindings';
 import {publishDefaultGlobalUtils} from './util/global_utils';
 import {defaultScheduler, renderStringify} from './util/misc_utils';
@@ -210,10 +210,15 @@ export function createRootComponent<T>(
 
   const rootTNode = getPreviousOrParentTNode();
   if (tView.firstTemplatePass && componentDef.hostBindings) {
+    const elementIndex = rootTNode.index - HEADER_OFFSET;
+    setActiveHostElement(elementIndex);
+
     const expando = tView.expandoInstructions !;
     invokeHostBindingsInCreationMode(
         componentDef, expando, component, rootTNode, tView.firstTemplatePass);
     rootTNode.onElementCreationFns && applyOnCreateInstructions(rootTNode);
+
+    setActiveHostElement(null);
   }
 
   if (rootTNode.stylingTemplate) {

packages/core/src/render3/features/inherit_definition_feature.ts

@@ -10,6 +10,7 @@ import {Type} from '../../interface/type';
 import {fillProperties} from '../../util/property';
 import {EMPTY_ARRAY, EMPTY_OBJ} from '../empty';
 import {ComponentDef, DirectiveDef, DirectiveDefFeature, RenderFlags} from '../interfaces/definition';
+import {adjustActiveDirectiveSuperClassDepthPosition} from '../state';
 import {isComponentDef} from '../util/view_utils';
 
 import {NgOnChangesFeature} from './ng_onchanges_feature';
@@ -63,8 +64,24 @@ export function InheritDefinitionFeature(definition: DirectiveDef<any>| Componen
       const superHostBindings = superDef.hostBindings;
       if (superHostBindings) {
         if (prevHostBindings) {
+          // because inheritance is unknown during compile time, the runtime code
+          // needs to be informed of the super-class depth so that instruction code
+          // can distinguish one host bindings function from another. The reason why
+          // relying on the directive uniqueId exclusively is not enough is because the
+          // uniqueId value and the directive instance stay the same between hostBindings
+          // calls throughout the directive inheritance chain. This means that without
+          // a super-class depth value, there is no way to know whether a parent or
+          // sub-class host bindings function is currently being executed.
           definition.hostBindings = (rf: RenderFlags, ctx: any, elementIndex: number) => {
-            superHostBindings(rf, ctx, elementIndex);
+            // The reason why we increment first and then decrement is so that parent
+            // hostBindings calls have a higher id value compared to sub-class hostBindings
+            // calls (this way the leaf directive is always at a super-class depth of 0).
+            adjustActiveDirectiveSuperClassDepthPosition(1);
+            try {
+              superHostBindings(rf, ctx, elementIndex);
+            } finally {
+              adjustActiveDirectiveSuperClassDepthPosition(-1);
+            }
             prevHostBindings(rf, ctx, elementIndex);
           };
         } else {

packages/core/src/render3/instructions/element.ts

@@ -11,20 +11,23 @@ import {assertHasParent} from '../assert';
 import {attachPatchData} from '../context_discovery';
 import {registerPostOrderHooks} from '../hooks';
 import {TAttributes, TNodeFlags, TNodeType} from '../interfaces/node';
-import {RElement, Renderer3, isProceduralRenderer} from '../interfaces/renderer';
+import {RElement, isProceduralRenderer} from '../interfaces/renderer';
 import {SanitizerFn} from '../interfaces/sanitization';
 import {BINDING_INDEX, QUERIES, RENDERER, TVIEW} from '../interfaces/view';
 import {assertNodeType} from '../node_assert';
 import {appendChild} from '../node_manipulation';
 import {applyOnCreateInstructions} from '../node_util';
-import {decreaseElementDepthCount, getActiveHostContext, getElementDepthCount, getIsParent, getLView, getNamespace, getPreviousOrParentTNode, increaseElementDepthCount, setIsParent, setPreviousOrParentTNode} from '../state';
+import {decreaseElementDepthCount, getActiveDirectiveId, getElementDepthCount, getIsParent, getLView, getNamespace, getPreviousOrParentTNode, getSelectedIndex, increaseElementDepthCount, setIsParent, setPreviousOrParentTNode} from '../state';
 import {getInitialClassNameValue, getInitialStyleStringValue, initializeStaticContext, patchContextWithStaticAttrs, renderInitialClasses, renderInitialStyles} from '../styling/class_and_style_bindings';
 import {getStylingContext, hasClassInput, hasStyleInput} from '../styling/util';
 import {NO_CHANGE} from '../tokens';
 import {attrsStylingIndexOf, setUpAttributes} from '../util/attrs_utils';
 import {renderStringify} from '../util/misc_utils';
 import {getNativeByIndex, getNativeByTNode, getTNode} from '../util/view_utils';
+
 import {createDirectivesAndLocals, createNodeAtIndex, elementCreate, executeContentQueries, initializeTNodeInputs, setInputsForProperty, setNodeStylingTemplate} from './shared';
+import {getActiveDirectiveStylingIndex} from './styling_instructions';
+
 
 /**
  * Create DOM element. The instruction must later be followed by `elementEnd()` call.
@@ -256,17 +259,26 @@ export function elementAttribute(
  * @publicApi
  */
 export function elementHostAttrs(attrs: TAttributes) {
-  const tNode = getPreviousOrParentTNode();
+  const hostElementIndex = getSelectedIndex();
   const lView = getLView();
-  const native = getNativeByTNode(tNode, lView) as RElement;
-  const lastAttrIndex = setUpAttributes(native, attrs);
-  const stylingAttrsStartIndex = attrsStylingIndexOf(attrs, lastAttrIndex);
-  if (stylingAttrsStartIndex >= 0) {
-    const directive = getActiveHostContext();
-    if (tNode.stylingTemplate) {
-      patchContextWithStaticAttrs(tNode.stylingTemplate, attrs, stylingAttrsStartIndex, directive);
-    } else {
-      tNode.stylingTemplate = initializeStaticContext(attrs, stylingAttrsStartIndex, directive);
+  const tNode = getTNode(hostElementIndex, lView);
+
+  // non-element nodes (e.g. `<ng-container>`) are not rendered as actual
+  // element nodes and adding styles/classes on to them will cause runtime
+  // errors...
+  if (tNode.type === TNodeType.Element) {
+    const native = getNativeByTNode(tNode, lView) as RElement;
+    const lastAttrIndex = setUpAttributes(native, attrs);
+    const stylingAttrsStartIndex = attrsStylingIndexOf(attrs, lastAttrIndex);
+    if (stylingAttrsStartIndex >= 0) {
+      const directiveStylingIndex = getActiveDirectiveStylingIndex();
+      if (tNode.stylingTemplate) {
+        patchContextWithStaticAttrs(
+            tNode.stylingTemplate, attrs, stylingAttrsStartIndex, directiveStylingIndex);
+      } else {
+        tNode.stylingTemplate =
+            initializeStaticContext(attrs, stylingAttrsStartIndex, directiveStylingIndex);
+      }
     }
   }
 }

packages/core/src/render3/instructions/element_container.ts

@@ -13,6 +13,7 @@ import {TAttributes, TNodeType} from '../interfaces/node';
 import {BINDING_INDEX, QUERIES, RENDERER, TVIEW} from '../interfaces/view';
 import {assertNodeType} from '../node_assert';
 import {appendChild} from '../node_manipulation';
+import {applyOnCreateInstructions} from '../node_util';
 import {getIsParent, getLView, getPreviousOrParentTNode, setIsParent, setPreviousOrParentTNode} from '../state';
 import {createDirectivesAndLocals, createNodeAtIndex, executeContentQueries, setNodeStylingTemplate} from './shared';
 
@@ -83,5 +84,9 @@ export function elementContainerEnd(): void {
     lView[QUERIES] = currentQueries.parent;
   }
 
+  // this is required for all host-level styling-related instructions to run
+  // in the correct order
+  previousOrParentTNode.onElementCreationFns && applyOnCreateInstructions(previousOrParentTNode);
+
   registerPostOrderHooks(tView, previousOrParentTNode);
 }

packages/core/src/render3/instructions/shared.ts

@@ -27,14 +27,15 @@ import {StylingContext} from '../interfaces/styling';
 import {BINDING_INDEX, CHILD_HEAD, CHILD_TAIL, CLEANUP, CONTEXT, DECLARATION_VIEW, ExpandoInstructions, FLAGS, HEADER_OFFSET, HOST, INJECTOR, InitPhaseState, LView, LViewFlags, NEXT, PARENT, QUERIES, RENDERER, RENDERER_FACTORY, RootContext, RootContextFlags, SANITIZER, TVIEW, TView, T_HOST} from '../interfaces/view';
 import {assertNodeOfPossibleTypes, assertNodeType} from '../node_assert';
 import {isNodeMatchingSelectorList} from '../node_selector_matcher';
-import {enterView, getBindingsEnabled, getCheckNoChangesMode, getIsParent, getLView, getNamespace, getPreviousOrParentTNode, isCreationMode, leaveView, namespaceHTML, resetComponentState, setActiveHost, setBindingRoot, setCheckNoChangesMode, setCurrentDirectiveDef, setCurrentQueryIndex, setIsParent, setPreviousOrParentTNode, setSelectedIndex} from '../state';
+import {enterView, getBindingsEnabled, getCheckNoChangesMode, getIsParent, getLView, getNamespace, getPreviousOrParentTNode, incrementActiveDirectiveId, isCreationMode, leaveView, namespaceHTML, resetComponentState, setActiveHostElement, setBindingRoot, setCheckNoChangesMode, setCurrentDirectiveDef, setCurrentQueryIndex, setIsParent, setPreviousOrParentTNode, setSelectedIndex} from '../state';
 import {initializeStaticContext as initializeStaticStylingContext} from '../styling/class_and_style_bindings';
 import {NO_CHANGE} from '../tokens';
 import {attrsStylingIndexOf} from '../util/attrs_utils';
 import {INTERPOLATION_DELIMITER, renderStringify} from '../util/misc_utils';
 import {getLViewParent, getRootContext} from '../util/view_traversal_utils';
 import {getComponentViewByIndex, getNativeByTNode, isComponentDef, isContentQueryHost, isRootView, readPatchedLView, resetPreOrderHookFlags, unwrapRNode, viewAttachedToChangeDetector} from '../util/view_utils';
 
+
 /**
  * A permanent marker promise which signifies that the current CD tree is
  * clean.
@@ -107,6 +108,8 @@ export function setHostBindings(tView: TView, viewData: LView): void {
           // Negative numbers mean that we are starting new EXPANDO block and need to update
           // the current element and directive index.
           currentElementIndex = -instruction;
+          setActiveHostElement(currentElementIndex);
+
           // Injector block and providers are taken into account.
           const providerCount = (tView.expandoInstructions[++i] as number);
           bindingRootIndex += INJECTOR_BLOOM_PARENT_SIZE + providerCount;
@@ -124,14 +127,20 @@ export function setHostBindings(tView: TView, viewData: LView): void {
         if (instruction !== null) {
           viewData[BINDING_INDEX] = bindingRootIndex;
           const hostCtx = unwrapRNode(viewData[currentDirectiveIndex]);
-          setActiveHost(hostCtx, currentElementIndex);
           instruction(RenderFlags.Update, hostCtx, currentElementIndex);
-          setActiveHost(null);
+
+          // Each directive gets a uniqueId value that is the same for both
+          // create and update calls when the hostBindings function is called. The
+          // directive uniqueId is not set anywhere--it is just incremented between
+          // each hostBindings call and is useful for helping instruction code
+          // uniquely determine which directive is currently active when executed.
+          incrementActiveDirectiveId();
         }
         currentDirectiveIndex++;
       }
     }
   }
+  setActiveHostElement(null);
 }
 
 /** Refreshes content queries for all directives in the given view. */
@@ -897,15 +906,27 @@ function invokeDirectivesHostBindings(tView: TView, viewData: LView, tNode: TNod
   const end = tNode.directiveEnd;
   const expando = tView.expandoInstructions !;
   const firstTemplatePass = tView.firstTemplatePass;
+  const elementIndex = tNode.index - HEADER_OFFSET;
+  setActiveHostElement(elementIndex);
+
   for (let i = start; i < end; i++) {
     const def = tView.data[i] as DirectiveDef<any>;
     const directive = viewData[i];
     if (def.hostBindings) {
       invokeHostBindingsInCreationMode(def, expando, directive, tNode, firstTemplatePass);
+
+      // Each directive gets a uniqueId value that is the same for both
+      // create and update calls when the hostBindings function is called. The
+      // directive uniqueId is not set anywhere--it is just incremented between
+      // each hostBindings call and is useful for helping instruction code
+      // uniquely determine which directive is currently active when executed.
+      incrementActiveDirectiveId();
     } else if (firstTemplatePass) {
       expando.push(null);
     }
   }
+
+  setActiveHostElement(null);
 }
 
 export function invokeHostBindingsInCreationMode(
@@ -914,9 +935,7 @@ export function invokeHostBindingsInCreationMode(
   const previousExpandoLength = expando.length;
   setCurrentDirectiveDef(def);
   const elementIndex = tNode.index - HEADER_OFFSET;
-  setActiveHost(directive, elementIndex);
   def.hostBindings !(RenderFlags.Create, directive, elementIndex);
-  setActiveHost(null);
   setCurrentDirectiveDef(null);
   // `hostBindings` function may or may not contain `allocHostVars` call
   // (e.g. it may not if it only contains host listeners), so we need to check whether