refactor: remove import circular dependencies (#20855)

This PR fixes a circular dependency among those files in Renderer3:
`query` -> `di` -> `instructions` -> `query` -> ...

Looking at the above dependencies the `di` -> `instructions` import is
a problematic one. Previously `di` had an import from `instructions`
since we can known about "current node" only in `instructions`
(and we need "current node" to create node injector instances).

This commit refactors the code in the way that functions in the
`di` file don't depend on any info stored module-global variables
in `instructions`.

PR Close #20855

packages/core/src/render3/di.ts

@@ -10,12 +10,103 @@
 // correctly implementing its interfaces for backwards compatibility.
 import * as viewEngine from '../core';
 
-import {BLOOM_SIZE, NG_ELEMENT_ID, getOrCreateNodeInjector} from './instructions';
 import {LContainer, LElement, LNodeFlags, LNodeInjector} from './l_node';
 import {ComponentTemplate, DirectiveDef} from './public_interfaces';
 import {notImplemented, stringify} from './util';
 
 
+/**
+ * If a directive is diPublic, bloomAdd sets a property on the instance with this constant as
+ * the key and the directive's unique ID as the value. This allows us to map directives to their
+ * bloom filter bit for DI.
+ */
+const NG_ELEMENT_ID = '__NG_ELEMENT_ID__';
+
+/**
+ * The number of slots in each bloom filter (used by DI). The larger this number, the fewer
+ * directives that will share slots, and thus, the fewer false positives when checking for
+ * the existence of a directive.
+ */
+const BLOOM_SIZE = 128;
+
+/** Counter used to generate unique IDs for directives. */
+let nextNgElementId = 0;
+
+/**
+ * Registers this directive as present in its node's injector by flipping the directive's
+ * corresponding bit in the injector's bloom filter.
+ *
+ * @param injector The node injector in which the directive should be registered
+ * @param type The directive to register
+ */
+export function bloomAdd(injector: LNodeInjector, type: viewEngine.Type<any>): void {
+  let id: number|undefined = (type as any)[NG_ELEMENT_ID];
+
+  // Set a unique ID on the directive type, so if something tries to inject the directive,
+  // we can easily retrieve the ID and hash it into the bloom bit that should be checked.
+  if (id == null) {
+    id = (type as any)[NG_ELEMENT_ID] = nextNgElementId++;
+  }
+
+  // We only have BLOOM_SIZE (128) slots in our bloom filter (4 buckets * 32 bits each),
+  // so all unique IDs must be modulo-ed into a number from 0 - 127 to fit into the filter.
+  // This means that after 128, some directives will share slots, leading to some false positives
+  // when checking for a directive's presence.
+  const bloomBit = id % BLOOM_SIZE;
+
+  // Create a mask that targets the specific bit associated with the directive.
+  // JS bit operations are 32 bits, so this will be a number between 2^0 and 2^31, corresponding
+  // to bit positions 0 - 31 in a 32 bit integer.
+  const mask = 1 << bloomBit;
+
+  // Use the raw bloomBit number to determine which bloom filter bucket we should check
+  // e.g: bf0 = [0 - 31], bf1 = [32 - 63], bf2 = [64 - 95], bf3 = [96 - 127]
+  if (bloomBit < 64) {
+    if (bloomBit < 32) {
+      // Then use the mask to flip on the bit (0-31) associated with the directive in that bucket
+      injector.bf0 |= mask;
+    } else {
+      injector.bf1 |= mask;
+    }
+  } else {
+    if (bloomBit < 96) {
+      injector.bf2 |= mask;
+    } else {
+      injector.bf3 |= mask;
+    }
+  }
+}
+
+/**
+ * Creates (or gets an existing) injector for a given element or container.
+ *
+ * @param {LElement | LContainer} node for which an injector should be retrieved / created.
+ * @returns {LNodeInjector} Node injector
+ */
+export function getOrCreateNodeInjectorForNode(node: LElement | LContainer): LNodeInjector {
+  const nodeInjector = node.nodeInjector;
+  const parentInjector = node.parent && node.parent.nodeInjector;
+  if (nodeInjector != parentInjector) {
+    return nodeInjector !;
+  }
+  return node.nodeInjector = {
+    parent: parentInjector,
+    node: node,
+    bf0: 0,
+    bf1: 0,
+    bf2: 0,
+    bf3: 0,
+    cbf0: parentInjector == null ? 0 : parentInjector.cbf0 | parentInjector.bf0,
+    cbf1: parentInjector == null ? 0 : parentInjector.cbf1 | parentInjector.bf1,
+    cbf2: parentInjector == null ? 0 : parentInjector.cbf2 | parentInjector.bf2,
+    cbf3: parentInjector == null ? 0 : parentInjector.cbf3 | parentInjector.bf3,
+    injector: null,
+    templateRef: null,
+    viewContainerRef: null,
+    elementRef: null
+  };
+}
+
 /** Injection flags for DI. */
 export const enum InjectFlags {
   /** Dependency is not required. Null will be injected if there is no provider for the dependency.
@@ -40,6 +131,16 @@ function createInjectionError(text: string, token: any) {
   return new Error(`ElementInjector: ${text} [${stringify(token)}]`);
 }
 
+/**
+ * Makes a directive public to the DI system by adding it to an injector's bloom filter.
+ *
+ * @param di The node injector in which a directive will be added
+ * @param def The definition of the directive to be made public
+ */
+export function diPublicInInjector(di: LNodeInjector, def: DirectiveDef<any>): void {
+  bloomAdd(di, def.type);
+}
+
 /**
  * Searches for an instance of the given directive type up the injector tree and returns
  * that instance if found.
@@ -52,23 +153,13 @@ function createInjectionError(text: string, token: any) {
  * If not found, it will propagate up to the next parent injector until the token
  * is found or the top is reached.
  *
- * Usage example (in factory function):
+ * @param di Node injector where the search should start
- *
- * class SomeDirective {
- *   constructor(directive: DirectiveA) {}
- *
- *   static ngDirectiveDef = defineDirective({
- *     type: SomeDirective,
- *     factory: () => new SomeDirective(inject(DirectiveA))
- *   });
- * }
- *
  * @param token The directive type to search for
  * @param flags Injection flags (e.g. CheckParent)
  * @returns The instance found
  */
-export function inject<T>(token: viewEngine.Type<T>, flags?: InjectFlags): T {
+export function getOrCreateInjectable<T>(
-  const di = getOrCreateNodeInjector();
+    di: LNodeInjector, token: viewEngine.Type<T>, flags?: InjectFlags): T {
   const bloomHash = bloomHashBit(token);
 
   // If the token has a bloom hash, then it is a directive that is public to the injection system
@@ -161,7 +252,7 @@ function bloomHashBit(type: viewEngine.Type<any>): number|null {
  * injectors, a 0 in the bloom bit indicates that the parents definitely do not contain
  * the directive and do not need to be checked.
  *
- * @param injector The starting injector to check
+ * @param injector The starting node injector to check
  * @param  bloomBit The bit to check in each injector's bloom filter
  * @returns An injector that might have the directive
  */
@@ -201,24 +292,16 @@ export function bloomFindPossibleInjector(
 }
 
 /**
- * Creates an ElementRef for a given node and stores it on the injector.
+ * Creates an ElementRef for a given node injector and stores it on the injector.
  * Or, if the ElementRef already exists, retrieves the existing ElementRef.
  *
+ * @param di The node injector where we should store a created ElementRef
  * @returns The ElementRef instance to use
  */
-export function injectElementRefForNode(node?: LElement | LContainer): viewEngine.ElementRef {
+export function getOrCreateElementRef(di: LNodeInjector): viewEngine.ElementRef {
-  let di = getOrCreateNodeInjector(node);
   return di.elementRef || (di.elementRef = new ElementRef(di.node.native));
 }
 
-/**
- * Creates an ElementRef and stores it on the injector. Or, if the ElementRef already
- * exists, retrieves the existing ElementRef.
- *
- * @returns The ElementRef instance to use
- */
-export const injectElementRef: () => viewEngine.ElementRef = injectElementRefForNode;
-
 /** A ref to a node's native element. */
 class ElementRef implements viewEngine.ElementRef {
   readonly nativeElement: any;
@@ -229,16 +312,16 @@ class ElementRef implements viewEngine.ElementRef {
  * Creates a TemplateRef and stores it on the injector. Or, if the TemplateRef already
  * exists, retrieves the existing TemplateRef.
  *
+ * @param di The node injector where we should store a created TemplateRef
  * @returns The TemplateRef instance to use
  */
-export function injectTemplateRef(): viewEngine.TemplateRef<any> {
+export function getOrCreateTemplateRef<T>(di: LNodeInjector): viewEngine.TemplateRef<T> {
-  let di = getOrCreateNodeInjector();
   const data = (di.node as LContainer).data;
   if (data === null || data.template === null) {
     throw createInjectionError('Directive does not have a template.', null);
   }
   return di.templateRef ||
-      (di.templateRef = new TemplateRef<any>(injectElementRef(), data.template));
+      (di.templateRef = new TemplateRef<any>(getOrCreateElementRef(di), data.template));
 }
 
 /** A ref to a particular template. */
@@ -258,8 +341,7 @@ class TemplateRef<T> implements viewEngine.TemplateRef<T> {
  *
  * @returns The ViewContainerRef instance to use
  */
-export function injectViewContainerRef(): viewEngine.ViewContainerRef {
+export function getOrCreateContainerRef(di: LNodeInjector): viewEngine.ViewContainerRef {
-  let di = getOrCreateNodeInjector();
   return di.viewContainerRef || (di.viewContainerRef = new ViewContainerRef(di.node as LContainer));
 }
 

packages/core/src/render3/index.ts

@@ -7,7 +7,6 @@
  */
 
 import {createComponentRef, detectChanges, getHostElement, markDirty, renderComponent} from './component';
-import {inject, injectElementRef, injectTemplateRef, injectViewContainerRef} from './di';
 import {ComponentDef, ComponentTemplate, ComponentType, DirectiveDef, DirectiveDefFlags, NgOnChangesFeature, PublicFeature, defineComponent, defineDirective} from './public_interfaces';
 
 // Naming scheme:
@@ -20,6 +19,8 @@ import {ComponentDef, ComponentTemplate, ComponentType, DirectiveDef, DirectiveD
 // - lower case for closing: c(containerEnd), e(elementEnd), v(viewEnd)
 // clang-format off
 export {
+  inject, injectElementRef, injectTemplateRef, injectViewContainerRef,
+
   LifecycleHook,
 
   NO_CHANGE as NC,
@@ -69,7 +70,6 @@ export {
 } from './instructions';
 // clang-format on
 export {QueryList} from './query';
-export {inject, injectElementRef, injectTemplateRef, injectViewContainerRef};
 export {
   ComponentDef,
   ComponentTemplate,

packages/core/src/render3/instructions.ts

@@ -8,7 +8,7 @@
 
 import './ng_dev_mode';
 
-import {Type} from '../core';
+import {ElementRef, TemplateRef, Type, ViewContainerRef} from '../core';
 
 import {assertEqual, assertLessThan, assertNotEqual, assertNotNull} from './assert';
 import {ContainerState, CssSelector, ProjectionState, QueryState, ViewState} from './interfaces';
@@ -19,6 +19,7 @@ import {assertNodeType} from './node_assert';
 import {appendChild, insertChild, insertView, processProjectedNode, removeView} from './node_manipulation';
 import {isNodeMatchingSelector} from './node_selector_matcher';
 import {ComponentDef, ComponentTemplate, ComponentType, DirectiveDef} from './public_interfaces';
+import {InjectFlags, diPublicInInjector, getOrCreateNodeInjectorForNode, getOrCreateElementRef, getOrCreateTemplateRef, getOrCreateContainerRef, getOrCreateInjectable} from './di';
 import {QueryList, QueryState_} from './query';
 import {RComment, RElement, RText, Renderer3, RendererFactory3, ProceduralRenderer3, ObjectOrientedRenderer3, RendererStyleFlags3} from './renderer';
 import {isDifferent, stringify} from './util';
@@ -38,23 +39,6 @@ export const enum LifecycleHook {ON_INIT = 1, ON_DESTROY = 2, ON_CHANGES = 4}
  */
 export const NG_HOST_SYMBOL = '__ngHostLNode__';
 
-/**
- * If a directive is diPublic, bloomAdd sets a property on the instance with this constant as
- * the key and the directive's unique ID as the value. This allows us to map directives to their
- * bloom filter bit for DI.
- */
-export const NG_ELEMENT_ID = '__NG_ELEMENT_ID__';
-
-/**
- * The number of slots in each bloom filter (used by DI). The larger this number, the fewer
- * directives that will share slots, and thus, the fewer false positives when checking for
- * the existence of a directive.
- */
-export const BLOOM_SIZE = 128;
-
-/** Counter used to generate unique IDs for directives. */
-let nextNgElementId = 0;
-
 /**
  * This property gets set before entering a template.
  *
@@ -321,77 +305,75 @@ export function renderComponentOrTemplate<T>(
   }
 }
 
+export function getOrCreateNodeInjector(): LNodeInjector {
+  ngDevMode && assertPreviousIsParent();
+  return getOrCreateNodeInjectorForNode(previousOrParentNode as LElement | LContainer);
+}
+
 /**
- * Registers this directive as present in its node's injector by flipping the directive's
+ * Makes a directive public to the DI system by adding it to an injector's bloom filter.
- * corresponding bit in the injector's bloom filter.
  *
- * @param injector The injector in which the directive should be registered
+ * @param def The definition of the directive to be made public
- * @param type The directive to register
  */
-export function bloomAdd(injector: LNodeInjector, type: Type<any>): void {
+export function diPublic(def: DirectiveDef<any>): void {
-  let id: number|undefined = (type as any)[NG_ELEMENT_ID];
+  diPublicInInjector(getOrCreateNodeInjector(), def);
-
-  // Set a unique ID on the directive type, so if something tries to inject the directive,
-  // we can easily retrieve the ID and hash it into the bloom bit that should be checked.
-  if (id == null) {
-    id = (type as any)[NG_ELEMENT_ID] = nextNgElementId++;
 }
 
-  // We only have BLOOM_SIZE (128) slots in our bloom filter (4 buckets * 32 bits each),
+/**
-  // so all unique IDs must be modulo-ed into a number from 0 - 127 to fit into the filter.
+ * Searches for an instance of the given directive type up the injector tree and returns
-  // This means that after 128, some directives will share slots, leading to some false positives
+ * that instance if found.
-  // when checking for a directive's presence.
+ *
-  const bloomBit = id % BLOOM_SIZE;
+ * If not found, it will propagate up to the next parent injector until the token
-
+ * is found or the top is reached.
-  // Create a mask that targets the specific bit associated with the directive.
+ *
-  // JS bit operations are 32 bits, so this will be a number between 2^0 and 2^31, corresponding
+ * Usage example (in factory function):
-  // to bit positions 0 - 31 in a 32 bit integer.
+ *
-  const mask = 1 << bloomBit;
+ * class SomeDirective {
-
+ *   constructor(directive: DirectiveA) {}
-  // Use the raw bloomBit number to determine which bloom filter bucket we should check
+ *
-  // e.g: bf0 = [0 - 31], bf1 = [32 - 63], bf2 = [64 - 95], bf3 = [96 - 127]
+ *   static ngDirectiveDef = defineDirective({
-  if (bloomBit < 64) {
+ *     type: SomeDirective,
-    if (bloomBit < 32) {
+ *     factory: () => new SomeDirective(inject(DirectiveA))
-      // Then use the mask to flip on the bit (0-31) associated with the directive in that bucket
+ *   });
-      injector.bf0 |= mask;
+ * }
-    } else {
+ *
-      injector.bf1 |= mask;
+ * @param token The directive type to search for
-    }
+ * @param flags Injection flags (e.g. CheckParent)
-  } else {
+ * @returns The instance found
-    if (bloomBit < 96) {
+ */
-      injector.bf2 |= mask;
+export function inject<T>(token: Type<T>, flags?: InjectFlags): T {
-    } else {
+  return getOrCreateInjectable<T>(getOrCreateNodeInjector(), token, flags);
-      injector.bf3 |= mask;
-    }
-  }
 }
 
-export function getOrCreateNodeInjector(node?: LElement | LContainer): LNodeInjector {
+/**
-  ngDevMode && !node && assertPreviousIsParent();
+ * Creates an ElementRef and stores it on the injector.
-  node = node || previousOrParentNode as LElement | LContainer;
+ * Or, if the ElementRef already exists, retrieves the existing ElementRef.
-  const nodeInjector = node.nodeInjector;
+ *
-  const parentInjector = node.parent && node.parent.nodeInjector;
+ * @returns The ElementRef instance to use
-  if (nodeInjector != parentInjector) {
+ */
-    return nodeInjector !;
+export function injectElementRef(): ElementRef {
-  }
+  return getOrCreateElementRef(getOrCreateNodeInjector());
-  return node.nodeInjector = {
-    parent: parentInjector,
-    node: node,
-    bf0: 0,
-    bf1: 0,
-    bf2: 0,
-    bf3: 0,
-    cbf0: parentInjector == null ? 0 : parentInjector.cbf0 | parentInjector.bf0,
-    cbf1: parentInjector == null ? 0 : parentInjector.cbf1 | parentInjector.bf1,
-    cbf2: parentInjector == null ? 0 : parentInjector.cbf2 | parentInjector.bf2,
-    cbf3: parentInjector == null ? 0 : parentInjector.cbf3 | parentInjector.bf3,
-    injector: null,
-    templateRef: null,
-    viewContainerRef: null,
-    elementRef: null
-  };
 }
 
+/**
+ * Creates a TemplateRef and stores it on the injector. Or, if the TemplateRef already
+ * exists, retrieves the existing TemplateRef.
+ *
+ * @returns The TemplateRef instance to use
+ */
+export function injectTemplateRef<T>(): TemplateRef<T> {
+  return getOrCreateTemplateRef<T>(getOrCreateNodeInjector());
+}
+
+/**
+ * Creates a ViewContainerRef and stores it on the injector. Or, if the ViewContainerRef
+ * already exists, retrieves the existing ViewContainerRef.
+ *
+ * @returns The ViewContainerRef instance to use
+ */
+export function injectViewContainerRef(): ViewContainerRef {
+  return getOrCreateContainerRef(getOrCreateNodeInjector());
+}
 
 //////////////////////////
 //// Element
@@ -938,15 +920,6 @@ function generateInitialInputs(
   return initialInputData;
 }
 
-/**
- * Makes a directive public to the DI system by adding it to an injector's bloom filter.
- *
- * @param def The definition of the directive to be made public
- */
-export function diPublic(def: DirectiveDef<any>): void {
-  bloomAdd(getOrCreateNodeInjector(), def.type);
-}
-
 /**
  * Accepts a lifecycle hook type and determines when and how the related lifecycle hook
  * callback should run.

packages/core/src/render3/query.ts

@@ -11,7 +11,7 @@ import {Observable} from 'rxjs/Observable';
 import * as viewEngine from '../core';
 
 import {assertNotNull} from './assert';
-import {injectElementRefForNode} from './di';
+import {getOrCreateElementRef, getOrCreateNodeInjectorForNode} from './di';
 import {QueryState} from './interfaces';
 import {LContainer, LElement, LNode, LNodeFlags, LView} from './l_node';
 import {DirectiveDef} from './public_interfaces';
@@ -117,7 +117,8 @@ function add(predicate: QueryPredicate<any>| null, node: LNode) {
         const selector = predicate.selector !;
         for (let i = 0; i < selector.length; i++) {
           if (selector[i] === staticData.localName) {
-            predicate.values.push(injectElementRefForNode(node as LElement | LContainer));
+            predicate.values.push(getOrCreateElementRef(
+                getOrCreateNodeInjectorForNode(node as LElement | LContainer)));
           }
         }
       }

packages/core/test/render3/di_spec.ts

@@ -8,9 +8,9 @@
 
 import {ElementRef, TemplateRef, ViewContainerRef} from '@angular/core';
 
-import {bloomFindPossibleInjector} from '../../src/render3/di';
+import {bloomAdd, bloomFindPossibleInjector} from '../../src/render3/di';
 import {C, D, E, PublicFeature, T, V, b, b2, c, cR, cr, defineDirective, e, inject, injectElementRef, injectTemplateRef, injectViewContainerRef, t, v} from '../../src/render3/index';
-import {bloomAdd, createLNode, createViewState, enterView, getOrCreateNodeInjector, leaveView} from '../../src/render3/instructions';
+import {createLNode, createViewState, enterView, getOrCreateNodeInjector, leaveView} from '../../src/render3/instructions';
 import {LNodeFlags, LNodeInjector} from '../../src/render3/l_node';
 
 import {renderToHtml} from './render_util';