8000 refactor(core): Remove host `TNode` from `getOrCreateTNode` (#38707) · angular/angular@bc6ff77 · GitHub
[go: up one dir, main page]
Skip to content

Commit bc6ff77

Browse files
mheverymhevery
committed
refactor(core): Remove host TNode from getOrCreateTNode (#38707)
Host `TNode` was passed into `getOrCreateTNode` just so that we can compute weather or not we are a root node. This was needed because `previousOrParentTNode` could have `TNode` from `TView` other then current `TView`. This is confusing mental model. Previous change ensured that `previousOrParentTNode` must always be part of `TView`, which enabled this change to remove the unneeded argument. PR Close #38707
1 parent 33aaa9e commit bc6ff77

File tree

14 files changed

+39
-40
lines changed

14 files changed

+39
-40
lines changed

‎packages/core/src/render3/component.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ export function createRootComponentView(
174174
const tView = rootView[TVIEW];
175175
ngDevMode && assertIndexInRange(rootView, 0 + HEADER_OFFSET);
176176
rootView[0 + HEADER_OFFSET] = rNode;
177-
const tNode: TElementNode = getOrCreateTNode(tView, null, 0, TNodeType.Element, null, null);
177+
const tNode: TElementNode = getOrCreateTNode(tView, 0, TNodeType.Element, null, null);
178178
const mergedAttrs = tNode.mergedAttrs = def.hostAttrs;
179179
if (mergedAttrs !== null) {
180180
computeStaticStyling(tNode, mergedAttrs, true);

‎packages/core/src/render3/di.ts

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -141,41 +141,41 @@ export function bloomAdd(
141141
* Creates (or gets an existing) injector for a given element or container.
142142
*
143143
* @param tNode for which an injector should be retrieved / created.
144-
* @param hostView View where the node is stored
144+
* @param lView View where the node is stored
145145
* @returns Node injector
146146
*/
147147
export function getOrCreateNodeInjectorForNode(
148-
tNode: TElementNode|TContainerNode|TElementContainerNode, hostView: LView): number {
149-
const existingInjectorIndex = getInjectorIndex(tNode, hostView);
148+
tNode: TElementNode|TContainerNode|TElementContainerNode, lView: LView): number {
149+
const existingInjectorIndex = getInjectorIndex(tNode, lView);
150150
if (existingInjectorIndex !== -1) {
151151
return existingInjectorIndex;
152152
}
153153

154-
const tView = hostView[TVIEW];
154+
const tView = lView[TVIEW];
155155
if (tView.firstCreatePass) {
156-
tNode.injectorIndex = hostView.length;
156+
tNode.injectorIndex = lView.length;
157157
insertBloom(tView.data, tNode); // foundation for node bloom
158-
insertBloom(hostView, null); // foundation for cumulative bloom
158+
insertBloom(lView, null); // foundation for cumulative bloom
159159
insertBloom(tView.blueprint, null);
160160
}
161161

162-
const parentLoc = getParentInjectorLocation(tNode, hostView);
162+
const parentLoc = getParentInjectorLocation(tNode, lView);
163163
const injectorInde F438 x = tNode.injectorIndex;
164164

165165
// If a parent injector can't be found, its location is set to -1.
166166
// In that case, we don't need to set up a cumulative bloom
167167
if (hasParentInjector(parentLoc)) {
168168
const parentIndex = getParentInjectorIndex(parentLoc);
169-
const parentLView = getParentInjectorView(parentLoc, hostView);
169+
const parentLView = getParentInjectorView(parentLoc, lView);
170170
const parentData = parentLView[TVIEW].data as any;
171171
// Creates a cumulative bloom filter that merges the parent's bloom filter
172172
// and its own cumulative bloom (which contains tokens for all ancestors)
173173
for (let i = 0; i < 8; i++) {
174-
hostView[injectorIndex + i] = parentLView[parentIndex + i] | parentData[parentIndex + i];
174+
lView[injectorIndex + i] = parentLView[parentIndex + i] | parentData[parentIndex + i];
175175
}
176176
}
177177

178-
hostView[injectorIndex + PARENT_INJECTOR] = parentLoc;
178+
lView[injectorIndex + PARENT_INJECTOR] = parentLoc;
179179
return injectorIndex;
180180
}
181181

@@ -202,7 +202,7 @@ export function getInjectorIndex(tNode: TNode, hostView: LView): number {
202202
* Finds the index of the parent injector, with a view offset if applicable. Used to set the
203203
* parent injector initially.
204204
*
205-
* Returns a combination of number of `ViewData` we have to go up and index in that `Viewdata`
205+
* Returns a combination of number of `LView` we have to go up and index in that `LView`
206206
*/
207207
export function getParentInjectorLocation(tNode: TNode, view: LView): RelativeInjectorLocation {
208208
if (tNode.parent && tNode.parent.injectorIndex !== -1) {

‎packages/core/src/render3/i18n/i18n_apply.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -463,7 +463,7 @@ function createDynamicNodeAtIndex(
463463
ngDevMode && assertIndexInRange(lView, index + HEADER_OFFSET);
464464
lView[index + HEADER_OFFSET] = native;
465465
// FIXME(misko): Why does this create A TNode??? I would not expect this to be here.
466-
const tNode = getOrCreateTNode(tView, lView[T_HOST], index, type as any, name, null);
466+
const tNode = getOrCreateTNode(tView, index, type as any, name, null);
467467

468468
// We are creating a dynamic node, the previous tNode might not be pointing at this node.
469469
// We will link ourselves into the tree later with `appendI18nNode`.

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ function elementStartFirstCreatePass(
3333

3434
const tViewConsts = tView.consts;
3535
const attrs = getConstant<TAttributes>(tViewConsts, attrsIndex);
36-
const tNode = getOrCreateTNode(tView, lView[T_HOST], index, TNodeType.Element, name, attrs);
36+
const tNode = getOrCreateTNode(tView, index, TNodeType.Element, name, attrs);
3737

3838
const hasDirectives =
3939
resolveDirectives(tView, lView, tNode, getConstant<string[]>(tViewConsts, localRefsIndex));

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,7 @@ function elementContainerStartFirstCreatePass(
2727

2828
const tViewConsts = tView.consts;
2929
const attrs = getConstant<TAttributes>(tViewConsts, attrsIndex);
30-
const tNode = getOrCreateTNode(
31-
tView, lView[T_HOST], index, TNodeType.ElementContainer, 'ng-container', attrs);
30+
const tNode = getOrCreateTNode(tView, index, TNodeType.ElementContainer, 'ng-container', attrs);
3231

3332
// While ng-container doesn't necessarily support styling, we use the style context to identify
3433
// and execute directives on the ng-container.

‎packages/core/src/render3/instructions/projection.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ export function ɵɵprojection(
125125
const lView = getLView();
126126
const tView = getTView();
127127
const tProjectionNode =
128-
getOrCreateTNode(tView, lView[T_HOST], nodeIndex, TNodeType.Projection, null, attrs || null);
128+
getOrCreateTNode(tView, nodeIndex, TNodeType.Projection, null, attrs || null);
129129

130130
// We can't use viewData[HOST_NODE] because projection nodes can be nested in embedded views.
131131
if (tProjectionNode.projection === null) tProjectionNode.projection = selectorIndex;

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

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -202,9 +202,6 @@ export function createLView<T>(
202202
* Create and stores the TNode, and hooks it up to the tree.
203203
*
204204
* @param tView The current `TView`.
205-
* @param tHostNode This is a hack and we should not have to pass this value in. It is only used to
206-
* determine if the parent belongs to a different tView. Instead we should not have parentTView
207-
* point to TView other the current one.
208205
* @param index The index at which the TNode should be saved (null if view, since they are not
209206
* saved).
210207
* @param type The type of TNode to create
@@ -213,43 +210,44 @@ export function createLView<T>(
213210
* @param attrs Any attrs for the native element, if applicable
214211
*/
215212
export function getOrCreateTNode(
216-
tView: TView, tHostNode: TNode|null, index: number, type: TNodeType.Element, name: string|null,
213+
tView: TView, index: number, type: TNodeType.Element, name: string|null,
217214
attrs: TAttributes|null): TElementNode;
218215
export function getOrCreateTNode(
219-
tView: TView, tHostNode: TNode|null, index: number, type: TNodeType.Container,
220-
name: string|null, attrs: TAttributes|null): TContainerNode;
216+
tView: TView, index: number, type: TNodeType.Container, name: string|null,
217+
attrs: TAttributes|null): TContainerNode;
221218
export function getOrCreateTNode(
222-
tView: TView, tHostNode: TNode|null, index: number, type: TNodeType.Projection, name: null,
219+
tView: TView, index: number, type: TNodeType.Projection, name: null,
223220
attrs: TAttributes|null): TProjectionNode;
224221
export function getOrCreateTNode(
225-
tView: TView, tHostNode: TNode|null, index: number, type: TNodeType.ElementContainer,
226-
name: string|null, attrs: TAttributes|null): TElementContainerNode;
222+
tView: TView, index: number, type: TNodeType.ElementContainer, name: string|null,
223+
attrs: TAttributes|null): TElementContainerNode;
227224
export function getOrCreateTNode(
228-
tView: TView, tHostNode: TNode|null, index: number, type: TNodeType.IcuContainer, name: null,
225+
tView: TView, index: number, type: TNodeType.IcuContainer, name: null,
229226
attrs: TAttributes|null): TElementContainerNode;
230227
export function getOrCreateTNode(
231-
tView: TView, tHostNode: TNode|null, index: number, type: TNodeType, name: string|null,
232-
attrs: TAttributes|null): TElementNode&TContainerNode&TElementContainerNode&TProjectionNode&
233-
TIcuContainerNode {
228+
tView: TView, index: number, type: TNodeType, name: string|null, attrs: TAttributes|null):
229+
TElementNode&TContainerNode&TElementContainerNode&TProjectionNode&TIcuContainerNode {
234230
// Keep this function short, so that the VM will inline it.
235231
const adjustedIndex = index + HEADER_OFFSET;
236232
const tNode = tView.data[adjustedIndex] as TNode ||
237-
createTNodeAtIndex(tView, tHostNode, adjustedIndex, type, name, attrs);
233+
createTNodeAtIndex(tView, adjustedIndex, type, name, attrs);
238234
setPreviousOrParentTNode(tNode, true);
239235
return tNode as TElementNode & TViewNode & TContainerNode & TElementContainerNode &
240236
TProjectionNode & TIcuContainerNode;
241237
}
242238

243239
function createTNodeAtIndex(
244-
tView: TView, tHostNode: TNode|null, adjustedIndex: number, type: TNodeType, name: string|null,
240+
tView: TView, adjustedIndex: number, type: TNodeType, name: string|null,
245241
attrs: TAttributes|null) {
246242
const previousOrParentTNode = getPreviousOrParentTNode();
247243
const isParent = getIsParent();
248244
const parent =
249245
isParent ? previousOrParentTNode : previousOrParentTNode && previousOrParentTNode.parent;
250246
// Parents cannot cross component boundaries because components will be used in multiple places,
251247
// so it's only set if the view is the same.
252-
const parentInSameView = parent && parent !== tHostNode;
248+
// FIXME(misko): This check for `TNodeType.View` should not be needed. But removing it breaks DI,
249+
// so more investigation is needed.
250+
const parentInSameView = parent !== null && parent.type !== TNodeType.View;
253251
const tParentNode = parentInSameView ? parent as TElementNode | TContainerNode : null;
254252
const tNode = tView.data[adjustedIndex] =
255253
createTNode(tView, tParentNode, type, adjustedIndex, name, attrs);

‎packages/core/src/render3/instructions/template.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ function templateFirstCreatePass(
2828
const tViewConsts = tView.consts;
2929
// TODO(pk): refactor getOrCreateTNode to have the "create" only version
3030
const tNode = getOrCreateTNode(
31-
tView, lView[T_HOST], index, TNodeType.Container, tagName || null,
31+
tView, index, TNodeType.Container, tagName || null,
3232
getConstant<TAttributes>(tViewConsts, attrsIndex));
3333

3434
resolveDirectives(tView, lView, tNode, getConstant<string[]>(tViewConsts, localRefsIndex));

‎packages/core/src/render3/instructions/text.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ export function ɵɵtext(index: number, value: string = ''): void {
3535
ngDevMode && assertIndexInRange(lView, adjustedIndex);
3636

3737
const tNode = tView.firstCreatePass ?
38-
getOrCreateTNode(tView, lView[T_HOST], index, TNodeType.Element, null, null) :
38+
getOrCreateTNode(tView, index, TNodeType.Element, null, null) :
3939
tView.data[adjustedIndex] as TElementNode;
4040

4141
const textNative = lView[adjustedIndex] = createTextNode(value, lView[RENDERER]);

‎packages/core/src/render3/node_manipulation.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -520,7 +520,7 @@ function getRenderParent(tView: TView, tNode: TNode, currentView: LView): REleme
520520
// can't be used as a render parent.
521521
let parentTNode = tNode.parent;
522522
while (parentTNode != null &&
523-
(parentTNode.type === TNodeType.ElementContainer ||
523+
(parentTNode.type === TNodeType.ElementContainer || parentTNode.type === TNodeType.View ||
524524
parentTNode.type === TNodeType.IcuContainer)) {
525525
tNode = parentTNode;
526526
parentTNode = tNode.parent;

‎packages/core/test/bundling/router/bundle.golden_symbols.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1355,6 +1355,9 @@
13551355
{
13561356
"name": "getNodeInjectable"
13571357
},
1358+
{
1359+
"name": "getNonViewFirstChild"
1360+
},
13581361
{
13591362
"name": "getNullInjector"
13601363
},

‎packages/core/test/render3/di_spec.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -230,8 +230,7 @@ describe('di', () => {
230230
LViewFlags.CheckAlways, null, null, {} as any, {} as any);
231231
enterView(contentView);
232232
try {
233-
const parentTNode =
234-
getOrCreateTNode(contentView[TVIEW], null, 0, TNodeType.Element, null, null);
233+
const parentTNode = getOrCreateTNode(contentView[TVIEW], 0, TNodeType.Element, null, null);
235234
// Simulate the situation where the previous parent is not initialized.
236235
// This happens on first bootstrap because we don't init existing values
237236
// so that we have smaller HelloWorld.

‎packages/core/test/render3/perf/setup.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ export function setupTestHarness(
5050
directiveRegistry: DirectiveDefList|null = null): TestHarness {
5151
// Create a root view with a container
5252
const hostTView = createTView(TViewType.Root, -1, null, 1, 0, null, null, null, null, consts);
53-
const tContainerNode = getOrCreateTNode(hostTView, null, 0, TNodeType.Container, null, null);
53+
const tContainerNode = getOrCreateTNode(hostTView, 0, TNodeType.Container, null, null);
5454
const hostNode = renderer.createElement('div');
5555
const hostLView = createLView(
5656
null, hostTView, {}, LViewFlags.CheckAlways | LViewFlags.IsRoot, hostNode, null,

‎packages/core/test/render3/render_util.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -278,7 +278,7 @@ export function renderTemplate<T>(
278278
def.pipeDefs = pipes || null;
279279

280280
const componentTView = getOrCreateTComponentView(def);
281-
const hostTNode = getOrCreateTNode(tView, hostLView[T_HOST], 0, TNodeType.Element, null, null);
281+
const hostTNode = getOrCreateTNode(tView, 0, TNodeType.Element, null, null);
282282
hostLView[hostTNode.index] = hostNode;
283283
componentView = createLView(
284284
hostLView, componentTView, context, LViewFlags.CheckAlways, hostNode, hostTNode,

0 commit comments

Comments
 (0)
0