Skip to content

Commit bbdea96

Browse files
pkozlowski-opensourceIgorMinar
authored andcommitted
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
1 parent d1de587 commit bbdea96

File tree

5 files changed

+182
-126
lines changed

5 files changed

+182
-126
lines changed

packages/core/src/render3/di.ts

Lines changed: 113 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,103 @@
1010
// correctly implementing its interfaces for backwards compatibility.
1111
import * as viewEngine from '../core';
1212

13-
import {BLOOM_SIZE, NG_ELEMENT_ID, getOrCreateNodeInjector} from './instructions';
1413
import {LContainer, LElement, LNodeFlags, LNodeInjector} from './l_node';
1514
import {ComponentTemplate, DirectiveDef} from './public_interfaces';
1615
import {notImplemented, stringify} from './util';
1716

1817

18+
/**
19+
* If a directive is diPublic, bloomAdd sets a property on the instance with this constant as
20+
* the key and the directive's unique ID as the value. This allows us to map directives to their
21+
* bloom filter bit for DI.
22+
*/
23+
const NG_ELEMENT_ID = '__NG_ELEMENT_ID__';
24+
25+
/**
26+
* The number of slots in each bloom filter (used by DI). The larger this number, the fewer
27+
* directives that will share slots, and thus, the fewer false positives when checking for
28+
* the existence of a directive.
29+
*/
30+
const BLOOM_SIZE = 128;
31+
32+
/** Counter used to generate unique IDs for directives. */
33+
let nextNgElementId = 0;
34+
35+
/**
36+
* Registers this directive as present in its node's injector by flipping the directive's
37+
* corresponding bit in the injector's bloom filter.
38+
*
39+
* @param injector The node injector in which the directive should be registered
40+
* @param type The directive to register
41+
*/
42+
export function bloomAdd(injector: LNodeInjector, type: viewEngine.Type<any>): void {
43+
let id: number|undefined = (type as any)[NG_ELEMENT_ID];
44+
45+
// Set a unique ID on the directive type, so if something tries to inject the directive,
46+
// we can easily retrieve the ID and hash it into the bloom bit that should be checked.
47+
if (id == null) {
48+
id = (type as any)[NG_ELEMENT_ID] = nextNgElementId++;
49+
}
50+
51+
// We only have BLOOM_SIZE (128) slots in our bloom filter (4 buckets * 32 bits each),
52+
// so all unique IDs must be modulo-ed into a number from 0 - 127 to fit into the filter.
53+
// This means that after 128, some directives will share slots, leading to some false positives
54+
// when checking for a directive's presence.
55+
const bloomBit = id % BLOOM_SIZE;
56+
57+
// Create a mask that targets the specific bit associated with the directive.
58+
// JS bit operations are 32 bits, so this will be a number between 2^0 and 2^31, corresponding
59+
// to bit positions 0 - 31 in a 32 bit integer.
60+
const mask = 1 << bloomBit;
61+
62+
// Use the raw bloomBit number to determine which bloom filter bucket we should check
63+
// e.g: bf0 = [0 - 31], bf1 = [32 - 63], bf2 = [64 - 95], bf3 = [96 - 127]
64+
if (bloomBit < 64) {
65+
if (bloomBit < 32) {
66+
// Then use the mask to flip on the bit (0-31) associated with the directive in that bucket
67+
injector.bf0 |= mask;
68+
} else {
69+
injector.bf1 |= mask;
70+
}
71+
} else {
72+
if (bloomBit < 96) {
73+
injector.bf2 |= mask;
74+
} else {
75+
injector.bf3 |= mask;
76+
}
77+
}
78+
}
79+
80+
/**
81+
* Creates (or gets an existing) injector for a given element or container.
82+
*
83+
* @param {LElement | LContainer} node for which an injector should be retrieved / created.
84+
* @returns {LNodeInjector} Node injector
85+
*/
86+
export function getOrCreateNodeInjectorForNode(node: LElement | LContainer): LNodeInjector {
87+
const nodeInjector = node.nodeInjector;
88+
const parentInjector = node.parent && node.parent.nodeInjector;
89+
if (nodeInjector != parentInjector) {
90+
return nodeInjector !;
91+
}
92+
return node.nodeInjector = {
93+
parent: parentInjector,
94+
node: node,
95+
bf0: 0,
96+
bf1: 0,
97+
bf2: 0,
98+
bf3: 0,
99+
cbf0: parentInjector == null ? 0 : parentInjector.cbf0 | parentInjector.bf0,
100+
cbf1: parentInjector == null ? 0 : parentInjector.cbf1 | parentInjector.bf1,
101+
cbf2: parentInjector == null ? 0 : parentInjector.cbf2 | parentInjector.bf2,
102+
cbf3: parentInjector == null ? 0 : parentInjector.cbf3 | parentInjector.bf3,
103+
injector: null,
104+
templateRef: null,
105+
viewContainerRef: null,
106+
elementRef: null
107+
};
108+
}
109+
19110
/** Injection flags for DI. */
20111
export const enum InjectFlags {
21112
/** 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) {
40131
return new Error(`ElementInjector: ${text} [${stringify(token)}]`);
41132
}
42133

134+
/**
135+
* Makes a directive public to the DI system by adding it to an injector's bloom filter.
136+
*
137+
* @param di The node injector in which a directive will be added
138+
* @param def The definition of the directive to be made public
139+
*/
140+
export function diPublicInInjector(di: LNodeInjector, def: DirectiveDef<any>): void {
141+
bloomAdd(di, def.type);
142+
}
143+
43144
/**
44145
* Searches for an instance of the given directive type up the injector tree and returns
45146
* that instance if found.
@@ -52,23 +153,13 @@ function createInjectionError(text: string, token: any) {
52153
* If not found, it will propagate up to the next parent injector until the token
53154
* is found or the top is reached.
54155
*
55-
* Usage example (in factory function):
56-
*
57-
* class SomeDirective {
58-
* constructor(directive: DirectiveA) {}
59-
*
60-
* static ngDirectiveDef = defineDirective({
61-
* type: SomeDirective,
62-
* factory: () => new SomeDirective(inject(DirectiveA))
63-
* });
64-
* }
65-
*
156+
* @param di Node injector where the search should start
66157
* @param token The directive type to search for
67158
* @param flags Injection flags (e.g. CheckParent)
68159
* @returns The instance found
69160
*/
70-
export function inject<T>(token: viewEngine.Type<T>, flags?: InjectFlags): T {
71-
const di = getOrCreateNodeInjector();
161+
export function getOrCreateInjectable<T>(
162+
di: LNodeInjector, token: viewEngine.Type<T>, flags?: InjectFlags): T {
72163
const bloomHash = bloomHashBit(token);
73164

74165
// 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 {
161252
* injectors, a 0 in the bloom bit indicates that the parents definitely do not contain
162253
* the directive and do not need to be checked.
163254
*
164-
* @param injector The starting injector to check
255+
* @param injector The starting node injector to check
165256
* @param bloomBit The bit to check in each injector's bloom filter
166257
* @returns An injector that might have the directive
167258
*/
@@ -201,24 +292,16 @@ export function bloomFindPossibleInjector(
201292
}
202293

203294
/**
204-
* Creates an ElementRef for a given node and stores it on the injector.
295+
* Creates an ElementRef for a given node injector and stores it on the injector.
205296
* Or, if the ElementRef already exists, retrieves the existing ElementRef.
206297
*
298+
* @param di The node injector where we should store a created ElementRef
207299
* @returns The ElementRef instance to use
208300
*/
209-
export function injectElementRefForNode(node?: LElement | LContainer): viewEngine.ElementRef {
210-
let di = getOrCreateNodeInjector(node);
301+
export function getOrCreateElementRef(di: LNodeInjector): viewEngine.ElementRef {
211302
return di.elementRef || (di.elementRef = new ElementRef(di.node.native));
212303
}
213304

214-
/**
215-
* Creates an ElementRef and stores it on the injector. Or, if the ElementRef already
216-
* exists, retrieves the existing ElementRef.
217-
*
218-
* @returns The ElementRef instance to use
219-
*/
220-
export const injectElementRef: () => viewEngine.ElementRef = injectElementRefForNode;
221-
222305
/** A ref to a node's native element. */
223306
class ElementRef implements viewEngine.ElementRef {
224307
readonly nativeElement: any;
@@ -229,16 +312,16 @@ class ElementRef implements viewEngine.ElementRef {
229312
* Creates a TemplateRef and stores it on the injector. Or, if the TemplateRef already
230313
* exists, retrieves the existing TemplateRef.
231314
*
315+
* @param di The node injector where we should store a created TemplateRef
232316
* @returns The TemplateRef instance to use
233317
*/
234-
export function injectTemplateRef(): viewEngine.TemplateRef<any> {
235-
let di = getOrCreateNodeInjector();
318+
export function getOrCreateTemplateRef<T>(di: LNodeInjector): viewEngine.TemplateRef<T> {
236319
const data = (di.node as LContainer).data;
237320
if (data === null || data.template === null) {
238321
throw createInjectionError('Directive does not have a template.', null);
239322
}
240323
return di.templateRef ||
241-
(di.templateRef = new TemplateRef<any>(injectElementRef(), data.template));
324+
(di.templateRef = new TemplateRef<any>(getOrCreateElementRef(di), data.template));
242325
}
243326

244327
/** A ref to a particular template. */
@@ -258,8 +341,7 @@ class TemplateRef<T> implements viewEngine.TemplateRef<T> {
258341
*
259342
* @returns The ViewContainerRef instance to use
260343
*/
261-
export function injectViewContainerRef(): viewEngine.ViewContainerRef {
262-
let di = getOrCreateNodeInjector();
344+
export function getOrCreateContainerRef(di: LNodeInjector): viewEngine.ViewContainerRef {
263345
return di.viewContainerRef || (di.viewContainerRef = new ViewContainerRef(di.node as LContainer));
264346
}
265347

packages/core/src/render3/index.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
*/
88

99
import {createComponentRef, detectChanges, getHostElement, markDirty, renderComponent} from './component';
10-
import {inject, injectElementRef, injectTemplateRef, injectViewContainerRef} from './di';
1110
import {ComponentDef, ComponentTemplate, ComponentType, DirectiveDef, DirectiveDefFlags, NgOnChangesFeature, PublicFeature, defineComponent, defineDirective} from './public_interfaces';
1211

1312
// Naming scheme:
@@ -20,6 +19,8 @@ import {ComponentDef, ComponentTemplate, ComponentType, DirectiveDef, DirectiveD
2019
// - lower case for closing: c(containerEnd), e(elementEnd), v(viewEnd)
2120
// clang-format off
2221
export {
22+
inject, injectElementRef, injectTemplateRef, injectViewContainerRef,
23+
2324
LifecycleHook,
2425

2526
NO_CHANGE as NC,
@@ -69,7 +70,6 @@ export {
6970
} from './instructions';
7071
// clang-format on
7172
export {QueryList} from './query';
72-
export {inject, injectElementRef, injectTemplateRef, injectViewContainerRef};
7373
export {
7474
ComponentDef,
7575
ComponentTemplate,

0 commit comments

Comments
 (0)