refactor(ivy): Remove `TNode.directives` in favor of `TData` (#35050)

`TNode.directives` was introduced in https://github.com/angular/angular/pull/34938. Turns out that it is unnecessary because the information is already present it `TData` when combining with `TNode.directiveStart` and `TNode.directiveEnd`

Mainly this is true (conceptually):
```
expect(tNode.directives).toEqual(
    tData.slice(
        tNode.directivesStart,
        tNode.directivesEnd - tNode.DirectivesStart -1
    )
);
```

The refactoring removes `TNode.directives` and adds `TNode.directiveStyling` as we still need to keep location in the directive in `TNode`

PR Close #35050

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

@@ -15,7 +15,7 @@ import {initNgDevMode} from '../../util/ng_dev_mode';
 import {ACTIVE_INDEX, ActiveIndexFlag, CONTAINER_HEADER_OFFSET, LContainer, MOVED_VIEWS, NATIVE} from '../interfaces/container';
 import {DirectiveDefList, PipeDefList, ViewQueriesFunction} from '../interfaces/definition';
 import {COMMENT_MARKER, ELEMENT_MARKER, I18nMutateOpCode, I18nMutateOpCodes, I18nUpdateOpCode, I18nUpdateOpCodes, TIcu} from '../interfaces/i18n';
-import {PropertyAliases, TConstants, TContainerNode, TDirectiveDefs, TElementNode, TNode as ITNode, TNodeFlags, TNodeProviderIndexes, TNodeType, TViewNode} from '../interfaces/node';
+import {PropertyAliases, TConstants, TContainerNode, TElementNode, TNode as ITNode, TNodeFlags, TNodeProviderIndexes, TNodeType, TViewNode} from '../interfaces/node';
 import {SelectorFlags} from '../interfaces/projection';
 import {TQueries} from '../interfaces/query';
 import {RComment, RElement, RNode} from '../interfaces/renderer';
@@ -159,6 +159,7 @@ class TNode implements ITNode {
       public injectorIndex: number,                                                  //
       public directiveStart: number,                                                 //
       public directiveEnd: number,                                                   //
+      public directiveStylingLast: number,                                           //
       public propertyBindings: number[]|null,                                        //
       public flags: TNodeFlags,                                                      //
       public providerIndexes: TNodeProviderIndexes,                                  //
@@ -181,7 +182,6 @@ class TNode implements ITNode {
       public residualClasses: KeyValueArray<any>|undefined|null,                     //
       public classBindings: TStylingRange,                                           //
       public styleBindings: TStylingRange,                                           //
-      public directives: TDirectiveDefs|null,                                        //
       ) {}
 
   get type_(): string {

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

@@ -24,7 +24,7 @@ import {executeCheckHooks, executeInitAndCheckHooks, incrementInitPhaseFlags} fr
 import {ACTIVE_INDEX, ActiveIndexFlag, CONTAINER_HEADER_OFFSET, LContainer, MOVED_VIEWS} from '../interfaces/container';
 import {ComponentDef, ComponentTemplate, DirectiveDef, DirectiveDefListOrFactory, PipeDefListOrFactory, RenderFlags, ViewQueriesFunction} from '../interfaces/definition';
 import {INJECTOR_BLOOM_PARENT_SIZE, NodeInjectorFactory} from '../interfaces/injector';
-import {AttributeMarker, DirectiveDefsValues, InitialInputData, InitialInputs, LocalRefExtractor, PropertyAliasValue, PropertyAliases, TAttributes, TConstants, TContainerNode, TDirectiveHostNode, TElementContainerNode, TElementNode, TIcuContainerNode, TNode, TNodeFlags, TNodeProviderIndexes, TNodeType, TProjectionNode, TViewNode} from '../interfaces/node';
+import {AttributeMarker, InitialInputData, InitialInputs, LocalRefExtractor, PropertyAliasValue, PropertyAliases, TAttributes, TConstants, TContainerNode, TDirectiveHostNode, TElementContainerNode, TElementNode, TIcuContainerNode, TNode, TNodeFlags, TNodeProviderIndexes, TNodeType, TProjectionNode, TViewNode} from '../interfaces/node';
 import {RComment, RElement, RNode, RText, Renderer3, RendererFactory3, isProceduralRenderer} from '../interfaces/renderer';
 import {SanitizerFn} from '../interfaces/sanitization';
 import {isComponentDef, isComponentHost, isContentQueryHost, isLContainer, isRootView} from '../interfaces/type_checks';
@@ -807,6 +807,7 @@ export function createTNode(
                          injectorIndex,  // injectorIndex: number
                          -1,             // directiveStart: number
                          -1,             // directiveEnd: number
+                         -1,             // directiveStylingLast: number
                          null,           // propertyBindings: number[]|null
                          0,              // flags: TNodeFlags
                          0,              // providerIndexes: TNodeProviderIndexes
@@ -829,7 +830,6 @@ export function createTNode(
                          undefined,  // residualClasses: string|null
                          0 as any,   // classBindings: TStylingRange;
                          0 as any,   // styleBindings: TStylingRange;
-                         null,       // directives: TDirectiveDefs|null;
                          ) :
                      {
                        type: type,
@@ -837,6 +837,7 @@ export function createTNode(
                        injectorIndex: injectorIndex,
                        directiveStart: -1,
                        directiveEnd: -1,
+                       directiveStylingLast: -1,
                        propertyBindings: null,
                        flags: 0,
                        providerIndexes: 0,
@@ -859,7 +860,6 @@ export function createTNode(
                        residualClasses: undefined,
                        classBindings: 0 as any,
                        styleBindings: 0 as any,
-                       directives: null
                      };
 }
 
@@ -1113,7 +1113,6 @@ export function resolveDirectives(
     const exportsMap: ({[key: string]: number} | null) = localRefs === null ? null : {'': -1};
 
     if (directiveDefs !== null) {
-      tNode.directives = [DirectiveDefsValues.INITIAL_STYLING_CURSOR_VALUE];
       let totalDirectiveHostVars = 0;
       hasDirectives = true;
       initTNodeFlags(tNode, tView.data.length, directiveDefs.length);
@@ -1132,7 +1131,6 @@ export function resolveDirectives(
       let preOrderCheckHooksFound = false;
       for (let i = 0; i < directiveDefs.length; i++) {
         const def = directiveDefs[i];
-        tNode.directives.push(def);
         // Merge the attrs in the order of matches. This assumes that the first directive is the
         // component itself, so that the component has the least priority.
         tNode.mergedAttrs = mergeHostAttrs(tNode.mergedAttrs, def.hostAttrs);

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

@@ -10,13 +10,13 @@ import {SafeValue, unwrapSafeValue} from '../../sanitization/bypass';
 import {stylePropNeedsSanitization, ɵɵsanitizeStyle} from '../../sanitization/sanitization';
 import {StyleSanitizeFn} from '../../sanitization/style_sanitizer';
 import {KeyValueArray, keyValueArrayGet, keyValueArraySet} from '../../util/array_utils';
-import {assertDefined, assertEqual, assertGreaterThanOrEqual, assertLessThan, assertNotEqual, assertNotSame, throwError} from '../../util/assert';
+import {assertDefined, assertEqual, assertLessThan, assertNotEqual, throwError} from '../../util/assert';
 import {EMPTY_ARRAY} from '../../util/empty';
 import {concatStringsWithSpace, stringify} from '../../util/stringify';
 import {assertFirstUpdatePass} from '../assert';
 import {bindingUpdated} from '../bindings';
 import {DirectiveDef} from '../interfaces/definition';
-import {AttributeMarker, DirectiveDefs, TAttributes, TNode, TNodeFlags, TNodeType} from '../interfaces/node';
+import {AttributeMarker, TAttributes, TNode, TNodeFlags, TNodeType} from '../interfaces/node';
 import {RElement, Renderer3} from '../interfaces/renderer';
 import {SanitizerFn} from '../interfaces/sanitization';
 import {TStylingKey, TStylingRange, getTStylingRangeNext, getTStylingRangeNextDuplicate, getTStylingRangePrev, getTStylingRangePrevDuplicate} from '../interfaces/styling';
@@ -27,9 +27,11 @@ import {insertTStylingBinding} from '../styling/style_binding_list';
 import {getLastParsedKey, getLastParsedValue, parseClassName, parseClassNameNext, parseStyle, parseStyleNext} from '../styling/styling_parser';
 import {NO_CHANGE} from '../tokens';
 import {getNativeByIndex} from '../util/view_utils';
+
 import {setDirectiveInputsWhichShadowsStyling} from './property';
 
 
+
 /**
  * Sets the current style sanitizer function which will then be used
  * within all follow-up prop and map-based style binding instructions
@@ -360,9 +362,9 @@ export function wrapInStaticStylingKey(
   } else {
     // We are in host binding node and there was no binding instruction in template node.
     // This means that we need to compute the residual.
-    const directives = tNode.directives;
+    const directiveStylingLast = tNode.directiveStylingLast;
-    const isFirstStylingInstructionInHostBinding = directives !== null &&
+    const isFirstStylingInstructionInHostBinding =
-        directives[directives[DirectiveDefs.STYLING_CURSOR]] !== hostDirectiveDef;
+        directiveStylingLast === -1 || tData[directiveStylingLast] !== hostDirectiveDef;
     if (isFirstStylingInstructionInHostBinding) {
       stylingKey =
           collectStylingFromDirectives(hostDirectiveDef, tData, tNode, stylingKey, isClassBased);
@@ -391,7 +393,7 @@ export function wrapInStaticStylingKey(
         //   the statics may have moved from the residual to the `stylingKey` and so we have to
         //   recompute.
         // - If `undefined` this is the first time we are running.
-        residual = collectResidual(tNode, isClassBased);
+        residual = collectResidual(tData, tNode, isClassBased);
       }
     }
   }
@@ -424,6 +426,58 @@ function getTemplateHeadTStylingKey(tData: TData, tNode: TNode, isClassBased: bo
   return tData[getTStylingRangePrev(bindings)] as TStylingKey;
 }
 
+/**
+ * Update the `TStylingKey` of the first template instruction in `TNode`.
+ *
+ * Logically `hostBindings` styling instructions are of lower priority than that of the template.
+ * However, they execute after the template styling instructions. This means that they get inserted
+ * in front of the template styling instructions.
+ *
+ * If we have a template styling instruction and a new `hostBindings` styling instruction is
+ * executed it means that it may need to steal static fields from the template instruction. This
+ * method allows us to update the first template instruction `TStylingKey` with a new value.
+ *
+ * Assume:
+ * ```
+ * <div my-dir style="color: red" [style.color]="tmplExp"></div>
+ *
+ * @Directive({
+ *   host: {
+ *     'style': 'width: 100px',
+ *     '[style.color]': 'dirExp',
+ *   }
+ * })
+ * class MyDir {}
+ * ```
+ *
+ * when `[style.color]="tmplExp"` executes it creates this data structure.
+ * ```
+ *  ['', 'color', 'color', 'red', 'width', '100px'],
+ * ```
+ *
+ * The reason for this is that the template instruction does not know if there are styling
+ * instructions and must assume that there are none and must collect all of the static styling.
+ * (both
+ * `color' and 'width`)
+ *
+ * When `'[style.color]': 'dirExp',` executes we need to insert a new data into the linked list.
+ * ```
+ *  ['', 'color', 'width', '100px'],  // newly inserted
+ *  ['', 'color', 'color', 'red', 'width', '100px'], // this is wrong
+ * ```
+ *
+ * Notice that the template statics is now wrong as it incorrectly contains `width` so we need to
+ * update it like so:
+ * ```
+ *  ['', 'color', 'width', '100px'],
+ *  ['', 'color', 'color', 'red'],    // UPDATE
+ * ```
+ *
+ * @param tData `TData` where the linked list is stored.
+ * @param tNode `TNode` for which the styling is being computed.
+ * @param isClassBased `true` if `class` (`false` if `style`)
+ * @param tStylingKey New `TStylingKey` which is replacing the old one.
+ */
 function setTemplateHeadTStylingKey(
     tData: TData, tNode: TNode, isClassBased: boolean, tStylingKey: TStylingKey): void {
   const bindings = isClassBased ? tNode.classBindings : tNode.styleBindings;
@@ -433,15 +487,29 @@ function setTemplateHeadTStylingKey(
   tData[getTStylingRangePrev(bindings)] = tStylingKey;
 }
 
-function collectResidual(tNode: TNode, isClassBased: boolean): KeyValueArray<any>|null {
+/**
+ * Collect all static values after the current `TNode.directiveStylingLast` index.
+ *
+ * Collect the remaining styling information which has not yet been collected by an existing
+ * styling instruction.
+ *
+ * @param tData `TData` where the `DirectiveDefs` are stored.
+ * @param tNode `TNode` which contains the directive range.
+ * @param isClassBased `true` if `class` (`false` if `style`)
+ */
+function collectResidual(tData: TData, tNode: TNode, isClassBased: boolean): KeyValueArray<any>|
+    null {
   let residual: KeyValueArray<any>|null|undefined = undefined;
-  const directives = tNode.directives;
+  const directiveEnd = tNode.directiveEnd;
-  if (directives) {
+  ngDevMode &&
-    for (let i = directives[DirectiveDefs.STYLING_CURSOR] + 1; i < directives.length; i++) {
+      assertNotEqual(
-      const attrs = (directives[i] as DirectiveDef<any>).hostAttrs;
+          tNode.directiveStylingLast, -1,
-      residual =
+          'By the time this function gets called at least one hostBindings-node styling instruction must have executed.');
-          collectStylingFromTAttrs(residual, attrs, isClassBased) as KeyValueArray<any>| null;
+  // We add `1 + tNode.directiveStart` because we need to skip the current directive (as we are
-    }
+  // collecting things after the last `hostBindings` directive which had a styling instruction.)
+  for (let i = 1 + tNode.directiveStylingLast; i < directiveEnd; i++) {
+    const attrs = (tData[i] as DirectiveDef<any>).hostAttrs;
+    residual = collectStylingFromTAttrs(residual, attrs, isClassBased) as KeyValueArray<any>| null;
   }
   return collectStylingFromTAttrs(residual, tNode.attrs, isClassBased) as KeyValueArray<any>| null;
 }
@@ -461,29 +529,28 @@ function collectResidual(tNode: TNode, isClassBased: boolean): KeyValueArray<any
 function collectStylingFromDirectives(
     hostDirectiveDef: DirectiveDef<any>| null, tData: TData, tNode: TNode, stylingKey: TStylingKey,
     isClassBased: boolean): TStylingKey {
-  const directives = tNode.directives;
+  // We need to loop because there can be directives which have `hostAttrs` but don't have
-  if (directives != null) {
+  // `hostBindings` so this loop catches up to the current directive..
-    ngDevMode && hostDirectiveDef &&
+  let currentDirective: DirectiveDef<any>|null = null;
-        assertGreaterThanOrEqual(
+  const directiveEnd = tNode.directiveEnd;
-            directives.indexOf(hostDirectiveDef, directives[DirectiveDefs.STYLING_CURSOR]), 0,
+  let directiveStylingLast = tNode.directiveStylingLast;
-            'Expecting that the current directive is in the directive list');
+  if (directiveStylingLast === -1) {
-    // We need to loop because there can be directives which have `hostAttrs` but don't have
+    directiveStylingLast = tNode.directiveStart;
-    // `hostBindings` so this loop catches up up to the current directive..
+  } else {
-    let currentDirective: DirectiveDef<any>|null = null;
+    directiveStylingLast++;
-    let index = directives[DirectiveDefs.STYLING_CURSOR];
+  }
-    while (index + 1 < directives.length) {
+  while (directiveStylingLast < directiveEnd) {
-      index++;
+    currentDirective = tData[directiveStylingLast] as DirectiveDef<any>;
-      currentDirective = directives[index] as DirectiveDef<any>;
+    ngDevMode && assertDefined(currentDirective, 'expected to be defined');
-      ngDevMode && assertDefined(currentDirective, 'expected to be defined');
+    stylingKey = collectStylingFromTAttrs(stylingKey, currentDirective.hostAttrs, isClassBased);
-      stylingKey = collectStylingFromTAttrs(stylingKey, currentDirective.hostAttrs, isClassBased);
+    if (currentDirective === hostDirectiveDef) break;
-      if (currentDirective === hostDirectiveDef) break;
+    directiveStylingLast++;
-    }
+  }
-    if (hostDirectiveDef !== null) {
+  if (hostDirectiveDef !== null) {
-      // we only advance the styling cursor if we are collecting data from host bindings.
+    // we only advance the styling cursor if we are collecting data from host bindings.
-      // Template executes before host bindings and so if we would update the index,
+    // Template executes before host bindings and so if we would update the index,
-      // host bindings would not get their statics.
+    // host bindings would not get their statics.
-      directives[DirectiveDefs.STYLING_CURSOR] = index;
+    tNode.directiveStylingLast = directiveStylingLast;
-    }
   }
   return stylingKey;
 }

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

@@ -294,6 +294,24 @@ export interface TNode {
    */
   directiveEnd: number;
 
+  /**
+   * Stores the last directive which had a styling instruction.
+   *
+   * Initial value of this is `-1` which means that no `hostBindings` styling instruction has
+   * executed. As `hostBindings` instructions execute they set the value to the index of the
+   * `DirectiveDef` which contained the last `hostBindings` styling instruction.
+   *
+   * Valid values are:
+   * - `-1` No `hostBindings` instruction has executed.
+   * - `directiveStart <= directiveStylingLast < directiveEnd`: Points to the `DirectiveDef` of the
+   *   last styling instruction which executed in the `hostBindings`.
+   *
+   * This data is needed so that styling instructions know which static styling data needs to be
+   * collected from the `DirectiveDef.hostAttrs`. A styling instruction needs to collect all data
+   * since last styling instruction.
+   */
+  directiveStylingLast: number;
+
   /**
    * Stores indexes of property bindings. This field is only set in the ngDevMode and holds indexes
    * of property bindings so TestBed can get bound property metadata for a given node.
@@ -346,13 +364,6 @@ export interface TNode {
    */
   mergedAttrs: TAttributes|null;
 
-  // TODO(misko): pre discussion with Kara, it seems that we don't need `directives` since the same
-  // information is already present in the TData. Maybe worth refactoring.
-  /**
-   * Stores the directive defs matched on the current TNode (along with style cursor.)
-   */
-  directives: TDirectiveDefs|null;
-
   /**
    * A set of local names under which a given element is exported in a template and
    * visible to queries. An entry in this array can be created for different reasons:
@@ -814,54 +825,3 @@ export function hasClassInput(tNode: TNode) {
 export function hasStyleInput(tNode: TNode) {
   return (tNode.flags & TNodeFlags.hasStyleInput) !== 0;
 }
-
-/**
- * Constant enums for accessing data in the `TDirectiveDefs`
- */
-export const enum DirectiveDefs {
-  /// Location where the STYLING_CURSOR is stored.
-  STYLING_CURSOR = 0,
-  /// Header offset from which iterating over `DirectiveDefs` should start.
-  HEADER_OFFSET = 1
-}
-
-/**
- * Constant enums for initial values in the `TDirectiveDefs`
- */
-export const enum DirectiveDefsValues {
-  // Initial value for the `STYLING_CURSOR`
-  INITIAL_STYLING_CURSOR_VALUE = 0,
-}
-
-/**
- * Stores `DirectiveDefs` associated with the current `TNode` as well as styling cursor.
- */
-export interface TDirectiveDefs extends Array<number|DirectiveDef<any>> {
-  /**
-   * As styling instructions (`ɵɵstyleProp`/`ɵɵclassProp`/`ɵɵstyleMap`/`ɵɵclassMap`) are executing
-   * they also need to get a hold of the `DirectiveDef.hostAttrs` and so that they know what
-   * static styling values to use. The styling instructions need this information so that they can
-   * lazily create `TStylingStatic`.
-   *
-   * When styling is executing it can get a hold of its `DirectiveDefs` but that alone is not
-   * sufficient for two reasons:
-   * 1. Styling instruction needs to coalesce other directives which came before it and which have
-   *    static value but may not have a styling instruction to attach the static values to.
-   * 2. There may be more than one styling instruction per `hostBindings` and only the first
-   *    styling instruction should create the `TStylingStatic`.
-   *
-   * The algorithm for doing this is:
-   * - look up the current `DirectiveDef` associated with the current instruction.
-   * - If `STYLING_CURSOR === 0 || tDirectiveDefs[stylingCursor] !== currentDirectiveDef` than
-   *   create `TStylingStatic` and:
-   *   - iterate over `TDirectiveDefs[++stylingCursor]` and insert them into the `TStylingStatic`
-   *     until you reach `DirectiveDef` associated with the current instruction.
-   * - If new `TStylingStatic` was created, recompute the residual styling values.
-   *
-   * The above algorithm will ensure that the styling instructions consume static styling values
-   * associated until a given instruction. After consuming instructions, it is always important to
-   * clear the residual (See `TNode.residualClass`/`TNode.residualStyle`), since this may be the
-   * last styling instruction, and we need to lazily recreate the residual value on as needed basis.
-   */
-  [DirectiveDefs.STYLING_CURSOR]: number;
-}

packages/core/test/render3/instructions/styling_spec.ts

@@ -9,7 +9,7 @@
 import {DirectiveDef} from '@angular/core/src/render3';
 import {ɵɵdefineDirective} from '@angular/core/src/render3/definition';
 import {classStringParser, styleStringParser, toStylingKeyValueArray, ɵɵclassProp, ɵɵstyleMap, ɵɵstyleProp, ɵɵstyleSanitizer} from '@angular/core/src/render3/instructions/styling';
-import {AttributeMarker, TAttributes, TDirectiveDefs} from '@angular/core/src/render3/interfaces/node';
+import {AttributeMarker, TAttributes} from '@angular/core/src/render3/interfaces/node';
 import {StylingRange, TStylingKey, TStylingRange, getTStylingRangeNext, getTStylingRangeNextDuplicate, getTStylingRangePrev, getTStylingRangePrevDuplicate, setTStylingRangeNext, setTStylingRangePrev, toTStylingRange} from '@angular/core/src/render3/interfaces/styling';
 import {HEADER_OFFSET, TVIEW} from '@angular/core/src/render3/interfaces/view';
 import {getLView, leaveView, setBindingRootForHostBindings} from '@angular/core/src/render3/state';
@@ -520,13 +520,13 @@ class MockDir {}
 function givenDirectiveAttrs(tAttrs: TAttributes[]) {
   const tNode = getTNode();
   const tData = getTData();
-  const directives: TDirectiveDefs = tNode.directives = [0];
+  tNode.directiveStart = getTDataIndexFromDirectiveIndex(0);
+  tNode.directiveEnd = getTDataIndexFromDirectiveIndex(tAttrs.length);
   for (let i = 0; i < tAttrs.length; i++) {
     const tAttr = tAttrs[i];
     const directiveDef = ɵɵdefineDirective({type: MockDir, hostAttrs: tAttr}) as DirectiveDef<any>;
     applyTAttributes(directiveDef.hostAttrs);
     tData[getTDataIndexFromDirectiveIndex(i)] = directiveDef;
-    directives.push(directiveDef);
   }
 }