Skip to content

Commit 3ceb3de

Browse files
AndrewKushnirthePunderWoman
AndrewKushnir
authored and
thePunderWoman
committed
refactor(core): replace loadLContext with getLContext calls (angular#41606)
This commit refactors the code to replace `loadLContext` with `getLContext` calls. The only difference between these two functions is that the `loadLContext` supports throwing an error in case `LContext` can not be found. The investigation performed in angular#41525 revealed that throwing while retrieving `LContext` might have undesirable performance implications, so we should avoid that to make sure there are no accidental perf regressions in other parts of code that used `loadLContext`. Moreover, in most of the places the `loadLContext` was already called in a mode that prevented an error from being thrown, so this refactoring should have no effect on the actual behavior. PR Close angular#41606
1 parent 64567d3 commit 3ceb3de

File tree

4 files changed

+39
-52
lines changed

4 files changed

+39
-52
lines changed

‎packages/core/src/debug/debug_node.ts‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,12 @@
88

99
import {Injector} from '../di/injector';
1010
import {assertTNodeForLView} from '../render3/assert';
11+
import {getLContext} from '../render3/context_discovery';
1112
import {CONTAINER_HEADER_OFFSET, LContainer, NATIVE} from '../render3/interfaces/container';
1213
import {TElementNode, TNode, TNodeFlags, TNodeType} from '../render3/interfaces/node';
1314
import {isComponentHost, isLContainer} from '../render3/interfaces/type_checks';
1415
import {DECLARATION_COMPONENT_VIEW, LView, PARENT, T_HOST, TData, TVIEW} from '../render3/interfaces/view';
15-
import {getComponent, getContext, getInjectionTokens, getInjector, getListeners, getLocalRefs, getOwningComponent, loadLContext} from '../render3/util/discovery_utils';
16+
import {getComponent, getContext, getInjectionTokens, getInjector, getListeners, getLocalRefs, getOwningComponent} from '../render3/util/discovery_utils';
1617
import {INTERPOLATION_DELIMITER} from '../render3/util/misc_utils';
1718
import {renderStringify} from '../render3/util/stringify_utils';
1819
import {getComponentLViewByIndex, getNativeByTNodeOrNull} from '../render3/util/view_utils';
@@ -261,13 +262,13 @@ class DebugElement__POST_R3__ extends DebugNode__POST_R3__ implements DebugEleme
261262
}
262263

263264
get name(): string {
264-
try {
265-
const context = loadLContext(this.nativeNode)!;
265+
const context = getLContext(this.nativeNode);
266+
if (context !== null) {
266267
const lView = context.lView;
267268
const tData = lView[TVIEW].data;
268269
const tNode = tData[context.nodeIndex] as TNode;
269270
return tNode.value!;
270-
} catch (e) {
271+
} else {
271272
return this.nativeNode.nodeName;
272273
}
273274
}
@@ -285,8 +286,8 @@ class DebugElement__POST_R3__ extends DebugNode__POST_R3__ implements DebugEleme
285286
* - attribute bindings (e.g. `[attr.role]="menu"`)
286287
*/
287288
get properties(): {[key: string]: any;} {
288-
const context = loadLContext(this.nativeNode, false);
289-
if (context == null) {
289+
const context = getLContext(this.nativeNode);
290+
if (context === null) {
290291
return {};
291292
}
292293

@@ -311,8 +312,8 @@ class DebugElement__POST_R3__ extends DebugNode__POST_R3__ implements DebugEleme
311312
return attributes;
312313
}
313314

314-
const context = loadLContext(element, false);
315-
if (context == null) {
315+
const context = getLContext(element);
316+
if (context === null) {
316317
return {};
317318
}
318319

@@ -501,7 +502,7 @@ function _queryAllR3(
501502
function _queryAllR3(
502503
parentElement: DebugElement, predicate: Predicate<DebugElement>|Predicate<DebugNode>,
503504
matches: DebugElement[]|DebugNode[], elementsOnly: boolean) {
504-
const context = loadLContext(parentElement.nativeNode, false);
505+
const context = getLContext(parentElement.nativeNode);
505506
if (context !== null) {
506507
const parentTNode = context.lView[TVIEW].data[context.nodeIndex] as TNode;
507508
_queryNodeChildrenR3(

‎packages/core/src/render3/util/discovery_utils.ts‎

Lines changed: 19 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ import {getTNode, unwrapRNode} from './view_utils';
5353
*/
5454
export function getComponent<T>(element: Element): T|null {
5555
assertDomElement(element);
56-
const context = loadLContext(element, false);
56+
const context = getLContext(element);
5757
if (context === null) return null;
5858

5959
if (context.component === undefined) {
@@ -78,7 +78,7 @@ export function getComponent<T>(element: Element): T|null {
7878
*/
7979
export function getContext<T>(element: Element): T|null {
8080
assertDomElement(element);
81-
const context = loadLContext(element, false);
81+
const context = getLContext(element);
8282
return context === null ? null : context.lView[CONTEXT] as T;
8383
}
8484

@@ -98,7 +98,7 @@ export function getContext<T>(element: Element): T|null {
9898
* @globalApi ng
9999
*/
100100
export function getOwningComponent<T>(elementOrDir: Element|{}): T|null {
101-
const context = loadLContext(elementOrDir, false);
101+
const context = getLContext(elementOrDir);
102102
if (context === null) return null;
103103

104104
let lView = context.lView;
@@ -136,7 +136,7 @@ export function getRootComponents(elementOrDir: Element|{}): {}[] {
136136
* @globalApi ng
137137
*/
138138
export function getInjector(elementOrDir: Element|{}): Injector {
139-
const context = loadLContext(elementOrDir, false);
139+
const context = getLContext(elementOrDir);
140140
if (context === null) return Injector.NULL;
141141

142142
const tNode = context.lView[TVIEW].data[context.nodeIndex] as TElementNode;
@@ -149,7 +149,7 @@ export function getInjector(elementOrDir: Element|{}): Injector {
149149
* @param element Element for which the injection tokens should be retrieved.
150150
*/
151151
export function getInjectionTokens(element: Element): any[] {
152-
const context = loadLContext(element, false);
152+
const context = getLContext(element);
153153
if (context === null) return [];
154154
const lView = context.lView;
155155
const tView = lView[TVIEW];
@@ -200,7 +200,7 @@ export function getDirectives(node: Node): {}[] {
200200
return [];
201201
}
202202

203-
const context = loadLContext(node, false);
203+
const context = getLContext(node);
204204
if (context === null) {
205205
return [];
206206
}
@@ -284,22 +284,6 @@ export function getDirectiveMetadata(directiveOrComponentInstance: any): Compone
284284
return null;
285285
}
286286

287-
/**
288-
* Returns LContext associated with a target passed as an argument.
289-
* Throws if a given target doesn't have associated LContext.
290-
*/
291-
export function loadLContext(target: {}): LContext;
292-
export function loadLContext(target: {}, throwOnNotFound: false): LContext|null;
293-
export function loadLContext(target: {}, throwOnNotFound: boolean = true): LContext|null {
294-
const context = getLContext(target);
295-
if (!context && throwOnNotFound) {
296-
throw new Error(
297-
ngDevMode ? `Unable to find context associated with ${stringifyForError(target)}` :
298-
'Invalid ng target');
299-
}
300-
return context;
301-
}
302-
303287
/**
304288
* Retrieve map of local references.
305289
*
@@ -309,7 +293,7 @@ export function loadLContext(target: {}, throwOnNotFound: boolean = true): LCont
309293
* the local references.
310294
*/
311295
export function getLocalRefs(target: {}): {[key: string]: any} {
312-
const context = loadLContext(target, false);
296+
const context = getLContext(target);
313297
if (context === null) return {};
314298

315299
if (context.localRefs === undefined) {
@@ -349,11 +333,6 @@ export function getRenderedText(component: any): string {
349333
return hostElement.textContent || '';
350334
}
351335

352-
export function loadLContextFromNode(node: Node): LContext {
353-
if (!(node instanceof Node)) throw new Error('Expecting instance of DOM Element');
354-
return loadLContext(node)!;
355-
}
356-
357336
/**
358337
* Event listener configuration returned from `getListeners`.
359338
* @publicApi
@@ -405,7 +384,7 @@ export interface Listener {
405384
*/
406385
export function getListeners(element: Element): Listener[] {
407386
assertDomElement(element);
408-
const lContext = loadLContext(element, false);
387+
const lContext = getLContext(element);
409388
if (lContext === null) return [];
410389

411390
const lView = lContext.lView;
@@ -458,9 +437,15 @@ function isDirectiveDefHack(obj: any): obj is DirectiveDef<any> {
458437
* @param element DOM element which is owned by an existing component's view.
459438
*/
460439
export function getDebugNode(element: Element): DebugNode|null {
461-
let debugNode: DebugNode|null = null;
440+
if (ngDevMode && !(element instanceof Node)) {
441+
throw new Error('Expecting instance of DOM Element');
442+
}
443+
444+
const lContext = getLContext(element);
445+
if (lContext === null) {
446+
return null;
447+
}
462448

463-
const lContext = loadLContextFromNode(element);
464449
const lView = lContext.lView;
465450
const nodeIndex = lContext.nodeIndex;
466451
if (nodeIndex !== -1) {
@@ -471,10 +456,10 @@ export function getDebugNode(element: Element): DebugNode|null {
471456
isLView(valueInLView) ? (valueInLView[T_HOST] as TNode) : getTNode(lView[TVIEW], nodeIndex);
472457
ngDevMode &&
473458
assertEqual(tNode.index, nodeIndex, 'Expecting that TNode at index is same as index');
474-
debugNode = buildDebugNode(tNode, lView);
459+
return buildDebugNode(tNode, lView);
475460
}
476461

477-
return debugNode;
462+
return null;
478463
}
479464

480465
/**
@@ -486,7 +471,7 @@ export function getDebugNode(element: Element): DebugNode|null {
486471
* @param target DOM element or component instance for which to retrieve the LView.
487472
*/
488473
export function getComponentLView(target: any): LView {
489-
const lContext = loadLContext(target);
474+
const lContext = getLContext(target)!;
490475
const nodeIndx = lContext.nodeIndex;
491476
const lView = lContext.lView;
492477
const componentLView = lView[nodeIndx];

‎packages/core/test/acceptance/discover_utils_spec.ts‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,9 @@ import {getElementStyles} from '@angular/core/testing/src/styling';
1717
import {expect} from '@angular/core/testing/src/testing_internal';
1818
import {onlyInIvy} from '@angular/private/testing';
1919

20+
import {getLContext} from '../../src/render3/context_discovery';
2021
import {getHostElement, markDirty} from '../../src/render3/index';
21-
import {ComponentDebugMetadata, getComponent, getComponentLView, getContext, getDebugNode, getDirectiveMetadata, getDirectives, getInjectionTokens, getInjector, getListeners, getLocalRefs, getOwningComponent, getRootComponents, loadLContext} from '../../src/render3/util/discovery_utils';
22+
import {ComponentDebugMetadata, getComponent, getComponentLView, getContext, getDebugNode, getDirectiveMetadata, getDirectives, getInjectionTokens, getInjector, getListeners, getLocalRefs, getOwningComponent, getRootComponents} from '../../src/render3/util/discovery_utils';
2223

2324
onlyInIvy('Ivy-specific utilities').describe('discovery utils', () => {
2425
let fixture: ComponentFixture<MyApp>;
@@ -270,17 +271,17 @@ onlyInIvy('Ivy-specific utilities').describe('discovery utils', () => {
270271
});
271272
});
272273

273-
describe('loadLContext', () => {
274+
describe('getLContext', () => {
274275
it('should work on components', () => {
275-
const lContext = loadLContext(child[0]);
276+
const lContext = getLContext(child[0])!;
276277
expect(lContext).toBeDefined();
277278
expect(lContext.native as any).toBe(child[0]);
278279
});
279280

280281
it('should work on templates', () => {
281282
const templateComment = Array.from((fixture.nativeElement as HTMLElement).childNodes)
282283
.find((node: ChildNode) => node.nodeType === Node.COMMENT_NODE)!;
283-
const lContext = loadLContext(templateComment);
284+
const lContext = getLContext(templateComment)!;
284285
expect(lContext).toBeDefined();
285286
expect(lContext.native as any).toBe(templateComment);
286287
});
@@ -290,7 +291,7 @@ onlyInIvy('Ivy-specific utilities').describe('discovery utils', () => {
290291
.find(
291292
(node: ChildNode) => node.nodeType === Node.COMMENT_NODE &&
292293
node.textContent === `ng-container`)!;
293-
const lContext = loadLContext(ngContainerComment);
294+
const lContext = getLContext(ngContainerComment)!;
294295
expect(lContext).toBeDefined();
295296
expect(lContext.native as any).toBe(ngContainerComment);
296297
});

‎packages/core/test/acceptance/ngdevmode_debug_spec.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@
88

99
import {CommonModule} from '@angular/common';
1010
import {Component} from '@angular/core';
11-
import {LView} from '@angular/core/src/render3/interfaces/view';
12-
import {getComponentLView, loadLContext} from '@angular/core/src/render3/util/discovery_utils';
11+
import {getLContext} from '@angular/core/src/render3/context_discovery';
12+
import {getComponentLView} from '@angular/core/src/render3/util/discovery_utils';
1313
import {createNamedArrayType} from '@angular/core/src/util/named_array_type';
1414
import {TestBed} from '@angular/core/testing';
1515
import {onlyInIvy} from '@angular/private/testing';
@@ -32,7 +32,7 @@ onlyInIvy('Debug information exist in ivy only').describe('ngDevMode debug', ()
3232

3333
TestBed.configureTestingModule({declarations: [MyApp], imports: [CommonModule]});
3434
const fixture = TestBed.createComponent(MyApp);
35-
const rootLView = loadLContext(fixture.nativeElement).lView;
35+
const rootLView = getLContext(fixture.nativeElement)!.lView;
3636
expect(rootLView.constructor.name).toEqual('LRootView');
3737

3838
const componentLView = getComponentLView(fixture.componentInstance);
@@ -41,7 +41,7 @@ onlyInIvy('Debug information exist in ivy only').describe('ngDevMode debug', ()
4141
const element: HTMLElement = fixture.nativeElement;
4242
fixture.detectChanges();
4343
const li = element.querySelector('li')!;
44-
const embeddedLView = loadLContext(li).lView;
44+
const embeddedLView = getLContext(li)!.lView;
4545
expect(embeddedLView.constructor.name).toEqual('LEmbeddedView_MyApp_li_1');
4646
});
4747
});

0 commit comments

Comments
 (0)