From 3ceb3dea672e184f48b12af9d329ea404dcc15da Mon Sep 17 00:00:00 2001 From: Andrew Kushnir Date: Tue, 13 Apr 2021 17:05:57 -0700 Subject: [PATCH] refactor(core): replace loadLContext with getLContext calls (#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 #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 #41606 --- packages/core/src/debug/debug_node.ts | 19 +++---- .../core/src/render3/util/discovery_utils.ts | 53 +++++++------------ .../test/acceptance/discover_utils_spec.ts | 11 ++-- .../test/acceptance/ngdevmode_debug_spec.ts | 8 +-- 4 files changed, 39 insertions(+), 52 deletions(-) diff --git a/packages/core/src/debug/debug_node.ts b/packages/core/src/debug/debug_node.ts index 1fb53711a0..f5db218fac 100644 --- a/packages/core/src/debug/debug_node.ts +++ b/packages/core/src/debug/debug_node.ts @@ -8,11 +8,12 @@ import {Injector} from '../di/injector'; import {assertTNodeForLView} from '../render3/assert'; +import {getLContext} from '../render3/context_discovery'; import {CONTAINER_HEADER_OFFSET, LContainer, NATIVE} from '../render3/interfaces/container'; import {TElementNode, TNode, TNodeFlags, TNodeType} from '../render3/interfaces/node'; import {isComponentHost, isLContainer} from '../render3/interfaces/type_checks'; import {DECLARATION_COMPONENT_VIEW, LView, PARENT, T_HOST, TData, TVIEW} from '../render3/interfaces/view'; -import {getComponent, getContext, getInjectionTokens, getInjector, getListeners, getLocalRefs, getOwningComponent, loadLContext} from '../render3/util/discovery_utils'; +import {getComponent, getContext, getInjectionTokens, getInjector, getListeners, getLocalRefs, getOwningComponent} from '../render3/util/discovery_utils'; import {INTERPOLATION_DELIMITER} from '../render3/util/misc_utils'; import {renderStringify} from '../render3/util/stringify_utils'; import {getComponentLViewByIndex, getNativeByTNodeOrNull} from '../render3/util/view_utils'; @@ -261,13 +262,13 @@ class DebugElement__POST_R3__ extends DebugNode__POST_R3__ implements DebugEleme } get name(): string { - try { - const context = loadLContext(this.nativeNode)!; + const context = getLContext(this.nativeNode); + if (context !== null) { const lView = context.lView; const tData = lView[TVIEW].data; const tNode = tData[context.nodeIndex] as TNode; return tNode.value!; - } catch (e) { + } else { return this.nativeNode.nodeName; } } @@ -285,8 +286,8 @@ class DebugElement__POST_R3__ extends DebugNode__POST_R3__ implements DebugEleme * - attribute bindings (e.g. `[attr.role]="menu"`) */ get properties(): {[key: string]: any;} { - const context = loadLContext(this.nativeNode, false); - if (context == null) { + const context = getLContext(this.nativeNode); + if (context === null) { return {}; } @@ -311,8 +312,8 @@ class DebugElement__POST_R3__ extends DebugNode__POST_R3__ implements DebugEleme return attributes; } - const context = loadLContext(element, false); - if (context == null) { + const context = getLContext(element); + if (context === null) { return {}; } @@ -501,7 +502,7 @@ function _queryAllR3( function _queryAllR3( parentElement: DebugElement, predicate: Predicate|Predicate, matches: DebugElement[]|DebugNode[], elementsOnly: boolean) { - const context = loadLContext(parentElement.nativeNode, false); + const context = getLContext(parentElement.nativeNode); if (context !== null) { const parentTNode = context.lView[TVIEW].data[context.nodeIndex] as TNode; _queryNodeChildrenR3( diff --git a/packages/core/src/render3/util/discovery_utils.ts b/packages/core/src/render3/util/discovery_utils.ts index 0546b62029..430df1bf2d 100644 --- a/packages/core/src/render3/util/discovery_utils.ts +++ b/packages/core/src/render3/util/discovery_utils.ts @@ -53,7 +53,7 @@ import {getTNode, unwrapRNode} from './view_utils'; */ export function getComponent(element: Element): T|null { assertDomElement(element); - const context = loadLContext(element, false); + const context = getLContext(element); if (context === null) return null; if (context.component === undefined) { @@ -78,7 +78,7 @@ export function getComponent(element: Element): T|null { */ export function getContext(element: Element): T|null { assertDomElement(element); - const context = loadLContext(element, false); + const context = getLContext(element); return context === null ? null : context.lView[CONTEXT] as T; } @@ -98,7 +98,7 @@ export function getContext(element: Element): T|null { * @globalApi ng */ export function getOwningComponent(elementOrDir: Element|{}): T|null { - const context = loadLContext(elementOrDir, false); + const context = getLContext(elementOrDir); if (context === null) return null; let lView = context.lView; @@ -136,7 +136,7 @@ export function getRootComponents(elementOrDir: Element|{}): {}[] { * @globalApi ng */ export function getInjector(elementOrDir: Element|{}): Injector { - const context = loadLContext(elementOrDir, false); + const context = getLContext(elementOrDir); if (context === null) return Injector.NULL; const tNode = context.lView[TVIEW].data[context.nodeIndex] as TElementNode; @@ -149,7 +149,7 @@ export function getInjector(elementOrDir: Element|{}): Injector { * @param element Element for which the injection tokens should be retrieved. */ export function getInjectionTokens(element: Element): any[] { - const context = loadLContext(element, false); + const context = getLContext(element); if (context === null) return []; const lView = context.lView; const tView = lView[TVIEW]; @@ -200,7 +200,7 @@ export function getDirectives(node: Node): {}[] { return []; } - const context = loadLContext(node, false); + const context = getLContext(node); if (context === null) { return []; } @@ -284,22 +284,6 @@ export function getDirectiveMetadata(directiveOrComponentInstance: any): Compone return null; } -/** - * Returns LContext associated with a target passed as an argument. - * Throws if a given target doesn't have associated LContext. - */ -export function loadLContext(target: {}): LContext; -export function loadLContext(target: {}, throwOnNotFound: false): LContext|null; -export function loadLContext(target: {}, throwOnNotFound: boolean = true): LContext|null { - const context = getLContext(target); - if (!context && throwOnNotFound) { - throw new Error( - ngDevMode ? `Unable to find context associated with ${stringifyForError(target)}` : - 'Invalid ng target'); - } - return context; -} - /** * Retrieve map of local references. * @@ -309,7 +293,7 @@ export function loadLContext(target: {}, throwOnNotFound: boolean = true): LCont * the local references. */ export function getLocalRefs(target: {}): {[key: string]: any} { - const context = loadLContext(target, false); + const context = getLContext(target); if (context === null) return {}; if (context.localRefs === undefined) { @@ -349,11 +333,6 @@ export function getRenderedText(component: any): string { return hostElement.textContent || ''; } -export function loadLContextFromNode(node: Node): LContext { - if (!(node instanceof Node)) throw new Error('Expecting instance of DOM Element'); - return loadLContext(node)!; -} - /** * Event listener configuration returned from `getListeners`. * @publicApi @@ -405,7 +384,7 @@ export interface Listener { */ export function getListeners(element: Element): Listener[] { assertDomElement(element); - const lContext = loadLContext(element, false); + const lContext = getLContext(element); if (lContext === null) return []; const lView = lContext.lView; @@ -458,9 +437,15 @@ function isDirectiveDefHack(obj: any): obj is DirectiveDef { * @param element DOM element which is owned by an existing component's view. */ export function getDebugNode(element: Element): DebugNode|null { - let debugNode: DebugNode|null = null; + if (ngDevMode && !(element instanceof Node)) { + throw new Error('Expecting instance of DOM Element'); + } + + const lContext = getLContext(element); + if (lContext === null) { + return null; + } - const lContext = loadLContextFromNode(element); const lView = lContext.lView; const nodeIndex = lContext.nodeIndex; if (nodeIndex !== -1) { @@ -471,10 +456,10 @@ export function getDebugNode(element: Element): DebugNode|null { isLView(valueInLView) ? (valueInLView[T_HOST] as TNode) : getTNode(lView[TVIEW], nodeIndex); ngDevMode && assertEqual(tNode.index, nodeIndex, 'Expecting that TNode at index is same as index'); - debugNode = buildDebugNode(tNode, lView); + return buildDebugNode(tNode, lView); } - return debugNode; + return null; } /** @@ -486,7 +471,7 @@ export function getDebugNode(element: Element): DebugNode|null { * @param target DOM element or component instance for which to retrieve the LView. */ export function getComponentLView(target: any): LView { - const lContext = loadLContext(target); + const lContext = getLContext(target)!; const nodeIndx = lContext.nodeIndex; const lView = lContext.lView; const componentLView = lView[nodeIndx]; diff --git a/packages/core/test/acceptance/discover_utils_spec.ts b/packages/core/test/acceptance/discover_utils_spec.ts index 07ae86691d..a1d589eabe 100644 --- a/packages/core/test/acceptance/discover_utils_spec.ts +++ b/packages/core/test/acceptance/discover_utils_spec.ts @@ -17,8 +17,9 @@ import {getElementStyles} from '@angular/core/testing/src/styling'; import {expect} from '@angular/core/testing/src/testing_internal'; import {onlyInIvy} from '@angular/private/testing'; +import {getLContext} from '../../src/render3/context_discovery'; import {getHostElement, markDirty} from '../../src/render3/index'; -import {ComponentDebugMetadata, getComponent, getComponentLView, getContext, getDebugNode, getDirectiveMetadata, getDirectives, getInjectionTokens, getInjector, getListeners, getLocalRefs, getOwningComponent, getRootComponents, loadLContext} from '../../src/render3/util/discovery_utils'; +import {ComponentDebugMetadata, getComponent, getComponentLView, getContext, getDebugNode, getDirectiveMetadata, getDirectives, getInjectionTokens, getInjector, getListeners, getLocalRefs, getOwningComponent, getRootComponents} from '../../src/render3/util/discovery_utils'; onlyInIvy('Ivy-specific utilities').describe('discovery utils', () => { let fixture: ComponentFixture; @@ -270,9 +271,9 @@ onlyInIvy('Ivy-specific utilities').describe('discovery utils', () => { }); }); - describe('loadLContext', () => { + describe('getLContext', () => { it('should work on components', () => { - const lContext = loadLContext(child[0]); + const lContext = getLContext(child[0])!; expect(lContext).toBeDefined(); expect(lContext.native as any).toBe(child[0]); }); @@ -280,7 +281,7 @@ onlyInIvy('Ivy-specific utilities').describe('discovery utils', () => { it('should work on templates', () => { const templateComment = Array.from((fixture.nativeElement as HTMLElement).childNodes) .find((node: ChildNode) => node.nodeType === Node.COMMENT_NODE)!; - const lContext = loadLContext(templateComment); + const lContext = getLContext(templateComment)!; expect(lContext).toBeDefined(); expect(lContext.native as any).toBe(templateComment); }); @@ -290,7 +291,7 @@ onlyInIvy('Ivy-specific utilities').describe('discovery utils', () => { .find( (node: ChildNode) => node.nodeType === Node.COMMENT_NODE && node.textContent === `ng-container`)!; - const lContext = loadLContext(ngContainerComment); + const lContext = getLContext(ngContainerComment)!; expect(lContext).toBeDefined(); expect(lContext.native as any).toBe(ngContainerComment); }); diff --git a/packages/core/test/acceptance/ngdevmode_debug_spec.ts b/packages/core/test/acceptance/ngdevmode_debug_spec.ts index ef07bb26ca..64763f7650 100644 --- a/packages/core/test/acceptance/ngdevmode_debug_spec.ts +++ b/packages/core/test/acceptance/ngdevmode_debug_spec.ts @@ -8,8 +8,8 @@ import {CommonModule} from '@angular/common'; import {Component} from '@angular/core'; -import {LView} from '@angular/core/src/render3/interfaces/view'; -import {getComponentLView, loadLContext} from '@angular/core/src/render3/util/discovery_utils'; +import {getLContext} from '@angular/core/src/render3/context_discovery'; +import {getComponentLView} from '@angular/core/src/render3/util/discovery_utils'; import {createNamedArrayType} from '@angular/core/src/util/named_array_type'; import {TestBed} from '@angular/core/testing'; import {onlyInIvy} from '@angular/private/testing'; @@ -32,7 +32,7 @@ onlyInIvy('Debug information exist in ivy only').describe('ngDevMode debug', () TestBed.configureTestingModule({declarations: [MyApp], imports: [CommonModule]}); const fixture = TestBed.createComponent(MyApp); - const rootLView = loadLContext(fixture.nativeElement).lView; + const rootLView = getLContext(fixture.nativeElement)!.lView; expect(rootLView.constructor.name).toEqual('LRootView'); const componentLView = getComponentLView(fixture.componentInstance); @@ -41,7 +41,7 @@ onlyInIvy('Debug information exist in ivy only').describe('ngDevMode debug', () const element: HTMLElement = fixture.nativeElement; fixture.detectChanges(); const li = element.querySelector('li')!; - const embeddedLView = loadLContext(li).lView; + const embeddedLView = getLContext(li)!.lView; expect(embeddedLView.constructor.name).toEqual('LEmbeddedView_MyApp_li_1'); }); });