refactor(core): remove assumptions that component will be first in di…

…rectives list (#47490)

`TNode`s have the `directiveStart` and `directiveEnd` properties that indicate the indexes at which directive instances (including components) have been stored. Currently there are several places throughout the codebase which assume that if a component matches a node, its index will always be `directiveStart`.

As far as I can tell, we probably ended up accumulating these assumptions, because we needed a quick way of accessing the component instance and it happened to be conventiently stored at `directiveStart`. The reason why it's always stored at `directiveStart` is likely to match the lifecycle hook execution order from ViewEngine.

With host directives these assumptions won't be valid anymore, because we want the host directives to _always_ execute before the host component that they're on so that the host has a chance to override them. To achieve this we have to insert host directives before the component.

These changes address the issue by introducing a new `TNode.componentOffset` property which indicates the offset after `TNode.directiveStart` at which the component is stored. Furthermore, I've removed the `isComponentHost` flag since it was duplicating the information from `TNode.componentOffset` and I've audited and fixed all the places where we read `directiveStart` to account for the changed data structure.

Reasons for some of the decisions I made along the way:
* In the case of host directives, I decided to go against our current convention of executing the component lifecycle hooks before the directive, because lifeycle hooks are a chance to change the component state (e.g. in `ngOnChanges`) and running the component hooks first would allow the host directives to undo any overrides made by the host.
* I decided to go with a `componentOffset`, instead of a `componentIndex` indicating the exact index the component is at, because as the runtime is set up at the moment, it would be difficult to know what index the component is going to end up at. Another problem is that we appear to have some logic that moves the entire "directive window" by incrementing both `directiveStart` and `directiveEnd`. By using an offset, we don't have to worry about the index remaining correct.

PR Close #47490

goldens/size-tracking/aio-payloads.json

@@ -12,7 +12,7 @@
   "aio-local": {
     "uncompressed": {
       "runtime": 4325,
-      "main": 454655,
+      "main": 455234,
       "polyfills": 33922,
       "styles": 73640,
       "light-theme": 78045,

goldens/size-tracking/integration-payloads.json

@@ -2,7 +2,7 @@
   "cli-hello-world": {
     "uncompressed": {
       "runtime": 1083,
-      "main": 124633,
+      "main": 125134,
       "polyfills": 33824
     }
   },
@@ -19,7 +19,7 @@
   "cli-hello-world-ivy-compat": {
     "uncompressed": {
       "runtime": 1102,
-      "main": 131619,
+      "main": 132120,
       "polyfills": 33957
     }
   },
@@ -55,7 +55,7 @@
   "standalone-bootstrap": {
     "uncompressed": {
       "runtime": 1090,
-      "main": 82906,
+      "main": 83452,
       "polyfills": 33945
     }
   },

packages/core/src/render3/component_ref.ts

@@ -364,7 +364,7 @@ export function createRootComponentView(
 
   if (tView.firstCreatePass) {
     diPublicInInjector(getOrCreateNodeInjectorForNode(tNode, rootView), tView, def.type);
-    markAsComponentHost(tView, tNode);
+    markAsComponentHost(tView, tNode, 0);
     initTNodeFlags(tNode, rootView.length, 1);
   }
 

packages/core/src/render3/context_discovery.ts

@@ -304,17 +304,21 @@ function findViaDirective(lView: LView, directiveInstance: {}): number {
 export function getDirectivesAtNodeIndex(
     nodeIndex: number, lView: LView, includeComponents: boolean): any[]|null {
   const tNode = lView[TVIEW].data[nodeIndex] as TNode;
-  let directiveStartIndex = tNode.directiveStart;
-  if (directiveStartIndex == 0) return EMPTY_ARRAY;
-  const directiveEndIndex = tNode.directiveEnd;
-  if (!includeComponents && tNode.flags & TNodeFlags.isComponentHost) directiveStartIndex++;
-  return lView.slice(directiveStartIndex, directiveEndIndex);
+  if (tNode.directiveStart === 0) return EMPTY_ARRAY;
+  const results: any[] = [];
+  for (let i = tNode.directiveStart; i < tNode.directiveEnd; i++) {
+    const directiveInstance = lView[i];
+    if (!isComponentInstance(directiveInstance) || includeComponents) {
+      results.push(directiveInstance);
+    }
+  }
+  return results;
 }
 
 export function getComponentAtNodeIndex(nodeIndex: number, lView: LView): {}|null {
   const tNode = lView[TVIEW].data[nodeIndex] as TNode;
-  let directiveStartIndex = tNode.directiveStart;
-  return tNode.flags & TNodeFlags.isComponentHost ? lView[directiveStartIndex] : null;
+  const {directiveStart, componentOffset} = tNode;
+  return componentOffset > -1 ? lView[directiveStart + componentOffset] : null;
 }
 
 /**

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

@@ -255,9 +255,8 @@ function wrapListener(
 
     // In order to be backwards compatible with View Engine, events on component host nodes
     // must also mark the component view itself dirty (i.e. the view that it owns).
-    const startView = tNode.flags & TNodeFlags.isComponentHost ?
-        getComponentLViewByIndex(tNode.index, lView) :
-        lView;
+    const startView =
+        tNode.componentOffset > -1 ? getComponentLViewByIndex(tNode.index, lView) : lView;
     markViewDirty(startView);
 
     let result = executeListenerWithErrorHandling(lView, context, listenerFn, e);

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

@@ -181,6 +181,7 @@ class TNode implements ITNode {
       public index: number,                                                          //
       public insertBeforeIndex: InsertBeforeIndex,                                   //
       public injectorIndex: number,                                                  //
+      public componentOffset: number,                                                //
       public directiveStart: number,                                                 //
       public directiveEnd: number,                                                   //
       public directiveStylingLast: number,                                           //
@@ -263,7 +264,6 @@ class TNode implements ITNode {
     if (this.flags & TNodeFlags.hasContentQuery) flags.push('TNodeFlags.hasContentQuery');
     if (this.flags & TNodeFlags.hasStyleInput) flags.push('TNodeFlags.hasStyleInput');
     if (this.flags & TNodeFlags.hasHostBindings) flags.push('TNodeFlags.hasHostBindings');
-    if (this.flags & TNodeFlags.isComponentHost) flags.push('TNodeFlags.isComponentHost');
     if (this.flags & TNodeFlags.isDirectiveHost) flags.push('TNodeFlags.isDirectiveHost');
     if (this.flags & TNodeFlags.isDetached) flags.push('TNodeFlags.isDetached');
     if (this.flags & TNodeFlags.isProjected) flags.push('TNodeFlags.isProjected');

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

@@ -14,7 +14,7 @@ import {SchemaMetadata} from '../../metadata/schema';
 import {ViewEncapsulation} from '../../metadata/view';
 import {validateAgainstEventAttributes, validateAgainstEventProperties} from '../../sanitization/sanitization';
 import {Sanitizer} from '../../sanitization/sanitizer';
-import {assertDefined, assertEqual, assertGreaterThanOrEqual, assertIndexInRange, assertNotEqual, assertNotSame, assertSame, assertString} from '../../util/assert';
+import {assertDefined, assertEqual, assertGreaterThan, assertGreaterThanOrEqual, assertIndexInRange, assertNotEqual, assertNotSame, assertSame, assertString} from '../../util/assert';
 import {escapeCommentText} from '../../util/dom';
 import {normalizeDebugBindingName, normalizeDebugBindingValue} from '../../util/ng_reflect';
 import {stringify} from '../../util/stringify';
@@ -789,6 +789,7 @@ export function createTNode(
           index,          // index: number
           null,           // insertBeforeIndex: null|-1|number|number[]
           injectorIndex,  // injectorIndex: number
+          -1,             // componentOffset: number
           -1,             // directiveStart: number
           -1,             // directiveEnd: number
           -1,             // directiveStylingLast: number
@@ -825,6 +826,7 @@ export function createTNode(
         directiveStart: -1,
         directiveEnd: -1,
         directiveStylingLast: -1,
+        componentOffset: -1,
         propertyBindings: null,
         flags: 0,
         providerIndexes: 0,
@@ -1040,8 +1042,9 @@ export function instantiateRootComponent<T>(tView: TView, lView: LView, def: Com
     configureViewWithDirective(tView, rootTNode, lView, directiveIndex, def);
     initializeInputAndOutputAliases(tView, rootTNode);
   }
-  const directive =
-      getNodeInjectable(lView, tView, rootTNode.directiveStart, rootTNode as TElementNode);
+  const directive = getNodeInjectable(
+      lView, tView, rootTNode.directiveStart + rootTNode.componentOffset,
+      rootTNode as TElementNode);
   attachPatchData(directive, lView);
   const native = getNativeByTNode(rootTNode, lView);
   if (native) {
@@ -1279,13 +1282,13 @@ function findDirectiveDefMatches(
                 `"${tNode.value}" tags cannot be used as component hosts. ` +
                     `Please use a different tag to activate the ${stringify(def.type)} component.`);
 
-            if (tNode.flags & TNodeFlags.isComponentHost) {
+            if (isComponentHost(tNode)) {
               // If another component has been matched previously, it's the first element in the
               // `matches` array, see how we store components/directives in `matches` below.
               throwMultipleComponentError(tNode, matches[0].type, def.type);
             }
           }
-          markAsComponentHost(tView, tNode);
+          markAsComponentHost(tView, tNode, 0);
           // The component is always stored first with directives after.
           matches.unshift(def);
         } else {
@@ -1299,12 +1302,13 @@ function findDirectiveDefMatches(
 
 /**
  * Marks a given TNode as a component's host. This consists of:
- * - setting appropriate TNode flags;
+ * - setting the component offset on the TNode.
  * - storing index of component's host element so it will be queued for view refresh during CD.
  */
-export function markAsComponentHost(tView: TView, hostTNode: TNode): void {
+export function markAsComponentHost(tView: TView, hostTNode: TNode, componentOffset: number): void {
   ngDevMode && assertFirstCreatePass(tView);
-  hostTNode.flags |= TNodeFlags.isComponentHost;
+  ngDevMode && assertGreaterThan(componentOffset, -1, 'componentOffset must be great than -1');
+  hostTNode.componentOffset = componentOffset;
   (tView.components || (tView.components = ngDevMode ? new TViewComponents() : []))
       .push(hostTNode.index);
 }

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

@@ -7,6 +7,7 @@
  */
 import {KeyValueArray} from '../../util/array_utils';
 import {TStylingRange} from '../interfaces/styling';
+
 import {TIcu} from './i18n';
 import {CssSelector} from './projection';
 import {RNode} from './renderer_dom';
@@ -102,35 +103,28 @@ export const enum TNodeFlags {
   /** Bit #1 - This bit is set if the node is a host for any directive (including a component) */
   isDirectiveHost = 0x1,
 
-  /**
-   * Bit #2 - This bit is set if the node is a host for a component.
-   *
-   * Setting this bit implies that the `isDirectiveHost` bit is set as well.
-   * */
-  isComponentHost = 0x2,
+  /** Bit #2 - This bit is set if the node has been projected */
+  isProjected = 0x2,
 
-  /** Bit #3 - This bit is set if the node has been projected */
-  isProjected = 0x4,
+  /** Bit #3 - This bit is set if any directive on this node has content queries */
+  hasContentQuery = 0x4,
 
-  /** Bit #4 - This bit is set if any directive on this node has content queries */
-  hasContentQuery = 0x8,
+  /** Bit #4 - This bit is set if the node has any "class" inputs */
+  hasClassInput = 0x8,
 
-  /** Bit #5 - This bit is set if the node has any "class" inputs */
-  hasClassInput = 0x10,
+  /** Bit #5 - This bit is set if the node has any "style" inputs */
+  hasStyleInput = 0x10,
 
-  /** Bit #6 - This bit is set if the node has any "style" inputs */
-  hasStyleInput = 0x20,
-
-  /** Bit #7 This bit is set if the node has been detached by i18n */
-  isDetached = 0x40,
+  /** Bit #6 This bit is set if the node has been detached by i18n */
+  isDetached = 0x20,
 
   /**
-   * Bit #8 - This bit is set if the node has directives with host bindings.
+   * Bit #7 - This bit is set if the node has directives with host bindings.
    *
    * This flags allows us to guard host-binding logic and invoke it only on nodes
    * that actually have directives with host bindings.
    */
-  hasHostBindings = 0x80,
+  hasHostBindings = 0x40,
 }
 
 /**
@@ -407,11 +401,7 @@ export interface TNode {
    */
   injectorIndex: number;
 
-  /**
-   * Stores starting index of the directives.
-   *
-   * NOTE: The first directive is always component (if present).
-   */
+  /** Stores starting index of the directives. */
   directiveStart: number;
 
   /**
@@ -423,6 +413,13 @@ export interface TNode {
    */
   directiveEnd: number;
 
+  /**
+   * Offset from the `directiveStart` at which the component (one at most) of the node is stored.
+   * Set to -1 if no components have been applied to the node. Component index can be found using
+   * `directiveStart + componentOffset`.
+   */
+  componentOffset: number;
+
   /**
    * Stores the last directive which had a styling instruction.
    *

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

@@ -34,7 +34,7 @@ export function isContentQueryHost(tNode: TNode): boolean {
 }
 
 export function isComponentHost(tNode: TNode): boolean {
-  return (tNode.flags & TNodeFlags.isComponentHost) === TNodeFlags.isComponentHost;
+  return tNode.componentOffset > -1;
 }
 
 export function isDirectiveHost(tNode: TNode): boolean {

packages/core/src/render3/node_manipulation.ts

@@ -576,10 +576,11 @@ export function getClosestRElement(tView: TView, tNode: TNode|null, lView: LView
     return lView[HOST];
   } else {
     ngDevMode && assertTNodeType(parentTNode, TNodeType.AnyRNode | TNodeType.Container);
-    if (parentTNode.flags & TNodeFlags.isComponentHost) {
+    const {componentOffset} = parentTNode;
+    if (componentOffset > -1) {
       ngDevMode && assertTNodeForLView(parentTNode, lView);
-      const encapsulation =
-          (tView.data[parentTNode.directiveStart] as ComponentDef<unknown>).encapsulation;
+      const {encapsulation} =
+          (tView.data[parentTNode.directiveStart + componentOffset] as ComponentDef<unknown>);
       // We've got a parent which is an element in the current view. We just need to verify if the
       // parent element is not a component. Component's content nodes are not inserted immediately
       // because they will be projected, and so doing insert at this point would be wasteful.

packages/core/src/render3/node_manipulation_i18n.ts

@@ -62,7 +62,7 @@ export function processI18nInsertBefore(
       anchorRNode = i18nParent;
       i18nParent = parentRElement;
     }
-    if (i18nParent !== null && (childTNode.flags & TNodeFlags.isComponentHost) === 0) {
+    if (i18nParent !== null && childTNode.componentOffset === -1) {
       for (let i = 1; i < tNodeInsertBeforeIndex.length; i++) {
         // No need to `unwrapRNode` because all of the indexes point to i18n text nodes.
         // see `assertDomNode` below.

packages/core/test/render3/is_shape_of.ts

@@ -154,6 +154,7 @@ const ShapeOfTNode: ShapeOf<TNode> = {
   directiveStart: true,
   directiveEnd: true,
   directiveStylingLast: true,
+  componentOffset: true,
   propertyBindings: true,
   flags: true,
   providerIndexes: true,