From 32dd3c5dd1df1ccb071ba7e6cd1d3c00f524d55b Mon Sep 17 00:00:00 2001 From: Andrew Kushnir Date: Fri, 5 Mar 2021 17:28:45 -0800 Subject: [PATCH] refactor(core): co-locate read/write patched data functions (#41097) This commit refactors Ivy runtime code to move `readPatchedData` and `attachPatchedData` functions to a single location for better maintainability and to make it easier to do further changes if needed. The `readPatchedLView` function was also moved to the same location (since it's a layer on top of the `readPatchedData` function). PR Close #41097 --- packages/core/src/render3/component.ts | 3 +- .../core/src/render3/context_discovery.ts | 29 ++++++- .../core/src/render3/instructions/shared.ts | 4 +- .../core/src/render3/interfaces/context.ts | 4 - .../src/render3/util/view_traversal_utils.ts | 3 +- packages/core/src/render3/util/view_utils.ts | 21 +---- .../render3/instructions/lview_debug_spec.ts | 4 +- .../core/test/render3/integration_spec.ts | 87 +++++++++---------- 8 files changed, 77 insertions(+), 78 deletions(-) diff --git a/packages/core/src/render3/component.ts b/packages/core/src/render3/component.ts index 7ed61bf7a2..f2ba3baee4 100644 --- a/packages/core/src/render3/component.ts +++ b/packages/core/src/render3/component.ts @@ -12,7 +12,9 @@ import {Injector} from '../di/injector'; import {Type} from '../interface/type'; import {Sanitizer} from '../sanitization/sanitizer'; import {assertDefined, assertIndexInRange} from '../util/assert'; + import {assertComponentType} from './assert'; +import {readPatchedLView} from './context_discovery'; import {getComponentDef} from './definition'; import {diPublicInInjector, getOrCreateNodeInjectorForNode} from './di'; import {throwProviderNotFoundError} from './errors_di'; @@ -31,7 +33,6 @@ import {setUpAttributes} from './util/attrs_utils'; import {publishDefaultGlobalUtils} from './util/global_utils'; import {defaultScheduler} from './util/misc_utils'; import {getRootContext} from './util/view_traversal_utils'; -import {readPatchedLView} from './util/view_utils'; diff --git a/packages/core/src/render3/context_discovery.ts b/packages/core/src/render3/context_discovery.ts index 49dc61d169..43f4207eb7 100644 --- a/packages/core/src/render3/context_discovery.ts +++ b/packages/core/src/render3/context_discovery.ts @@ -7,14 +7,14 @@ */ import '../util/ng_dev_mode'; -import {assertDomNode} from '../util/assert'; +import {assertDefined, assertDomNode} from '../util/assert'; import {EMPTY_ARRAY} from '../util/empty'; -import {LContext, MONKEY_PATCH_KEY_NAME} from './interfaces/context'; +import {LContext} from './interfaces/context'; import {TNode, TNodeFlags} from './interfaces/node'; import {RElement, RNode} from './interfaces/renderer_dom'; import {CONTEXT, HEADER_OFFSET, HOST, LView, TVIEW} from './interfaces/view'; -import {getComponentLViewByIndex, readPatchedData, unwrapRNode} from './util/view_utils'; +import {getComponentLViewByIndex, unwrapRNode} from './util/view_utils'; @@ -170,14 +170,37 @@ export function getComponentViewByInstance(componentInstance: {}): LView { return view; } +/** + * This property will be monkey-patched on elements, components and directives. + */ +const MONKEY_PATCH_KEY_NAME = '__ngContext__'; + /** * Assigns the given data to the given target (which could be a component, * directive or DOM node instance) using monkey-patching. */ export function attachPatchData(target: any, data: LView|LContext) { + ngDevMode && assertDefined(target, 'Target expected'); target[MONKEY_PATCH_KEY_NAME] = data; } +/** + * Returns the monkey-patch value data present on the target (which could be + * a component, directive or a DOM node). + */ +export function readPatchedData(target: any): LView|LContext|null { + ngDevMode && assertDefined(target, 'Target expected'); + return target[MONKEY_PATCH_KEY_NAME] || null; +} + +export function readPatchedLView(target: any): LView|null { + const value = readPatchedData(target); + if (value) { + return Array.isArray(value) ? value : (value as LContext).lView; + } + return null; +} + export function isComponentInstance(instance: any): boolean { return instance && instance.constructor && instance.constructor.ɵcmp; } diff --git a/packages/core/src/render3/instructions/shared.ts b/packages/core/src/render3/instructions/shared.ts index 9cb7bfe4ca..6bf02d590a 100644 --- a/packages/core/src/render3/instructions/shared.ts +++ b/packages/core/src/render3/instructions/shared.ts @@ -19,7 +19,7 @@ import {initNgDevMode} from '../../util/ng_dev_mode'; import {normalizeDebugBindingName, normalizeDebugBindingValue} from '../../util/ng_reflect'; import {stringify} from '../../util/stringify'; import {assertFirstCreatePass, assertFirstUpdatePass, assertLContainer, assertLView, assertTNodeForLView, assertTNodeForTView} from '../assert'; -import {attachPatchData} from '../context_discovery'; +import {attachPatchData, readPatchedLView} from '../context_discovery'; import {getFactoryDef} from '../definition_factory'; import {diPublicInInjector, getNodeInjectable, getOrCreateNodeInjectorForNode} from '../di'; import {formatRuntimeError, RuntimeError, RuntimeErrorCode} from '../error_code'; @@ -43,7 +43,7 @@ import {isAnimationProp, mergeHostAttrs} from '../util/attrs_utils'; import {INTERPOLATION_DELIMITER} from '../util/misc_utils'; import {renderStringify, stringifyForError} from '../util/stringify_utils'; import {getFirstLContainer, getLViewParent, getNextLContainer} from '../util/view_traversal_utils'; -import {getComponentLViewByIndex, getNativeByIndex, getNativeByTNode, isCreationMode, readPatchedLView, resetPreOrderHookFlags, unwrapLView, updateTransplantedViewCount, viewAttachedToChangeDetector} from '../util/view_utils'; +import {getComponentLViewByIndex, getNativeByIndex, getNativeByTNode, isCreationMode, resetPreOrderHookFlags, unwrapLView, updateTransplantedViewCount, viewAttachedToChangeDetector} from '../util/view_utils'; import {selectIndexInternal} from './advance'; import {attachLContainerDebug, attachLViewDebug, cloneToLViewFromTViewBlueprint, cloneToTViewData, LCleanup, LViewBlueprint, MatchesArray, TCleanup, TNodeDebug, TNodeInitialInputs, TNodeLocalNames, TViewComponents, TViewConstructor} from './lview_debug'; diff --git a/packages/core/src/render3/interfaces/context.ts b/packages/core/src/render3/interfaces/context.ts index 41943df640..6f78e26891 100644 --- a/packages/core/src/render3/interfaces/context.ts +++ b/packages/core/src/render3/interfaces/context.ts @@ -10,10 +10,6 @@ import {RNode} from './renderer_dom'; import {LView} from './view'; -/** - * This property will be monkey-patched on elements, components and directives - */ -export const MONKEY_PATCH_KEY_NAME = '__ngContext__'; /** * The internal view context which is specific to a given DOM element, directive or diff --git a/packages/core/src/render3/util/view_traversal_utils.ts b/packages/core/src/render3/util/view_traversal_utils.ts index 0dae10ab63..64bf2ba85d 100644 --- a/packages/core/src/render3/util/view_traversal_utils.ts +++ b/packages/core/src/render3/util/view_traversal_utils.ts @@ -8,12 +8,11 @@ import {assertDefined} from '../../util/assert'; import {assertLView} from '../assert'; +import {readPatchedLView} from '../context_discovery'; import {LContainer} from '../interfaces/container'; import {isLContainer, isLView} from '../interfaces/type_checks'; import {CHILD_HEAD, CONTEXT, FLAGS, LView, LViewFlags, NEXT, PARENT, RootContext} from '../interfaces/view'; -import {readPatchedLView} from './view_utils'; - /** * Gets the parent LView of the passed LView, if the PARENT is an LContainer, will get the parent of diff --git a/packages/core/src/render3/util/view_utils.ts b/packages/core/src/render3/util/view_utils.ts index 8683609874..0487d4d256 100644 --- a/packages/core/src/render3/util/view_utils.ts +++ b/packages/core/src/render3/util/view_utils.ts @@ -6,10 +6,9 @@ * found in the LICENSE file at https://angular.io/license */ -import {assertDefined, assertDomNode, assertGreaterThan, assertGreaterThanOrEqual, assertIndexInRange, assertLessThan} from '../../util/assert'; +import {assertDomNode, assertGreaterThan, assertGreaterThanOrEqual, assertIndexInRange, assertLessThan} from '../../util/assert'; import {assertTNode, assertTNodeForLView} from '../assert'; import {LContainer, TYPE} from '../interfaces/container'; -import {LContext, MONKEY_PATCH_KEY_NAME} from '../interfaces/context'; import {TConstants, TNode} from '../interfaces/node'; import {isProceduralRenderer} from '../interfaces/renderer'; import {RNode} from '../interfaces/renderer_dom'; @@ -143,24 +142,6 @@ export function getComponentLViewByIndex(nodeIndex: number, hostView: LView): LV return lView; } - -/** - * Returns the monkey-patch value data present on the target (which could be - * a component, directive or a DOM node). - */ -export function readPatchedData(target: any): LView|LContext|null { - ngDevMode && assertDefined(target, 'Target expected'); - return target[MONKEY_PATCH_KEY_NAME] || null; -} - -export function readPatchedLView(target: any): LView|null { - const value = readPatchedData(target); - if (value) { - return Array.isArray(value) ? value : (value as LContext).lView; - } - return null; -} - /** Checks whether a given view is in creation mode */ export function isCreationMode(view: LView): boolean { return (view[FLAGS] & LViewFlags.CreationMode) === LViewFlags.CreationMode; diff --git a/packages/core/test/render3/instructions/lview_debug_spec.ts b/packages/core/test/render3/instructions/lview_debug_spec.ts index d1d620add6..56219550e3 100644 --- a/packages/core/test/render3/instructions/lview_debug_spec.ts +++ b/packages/core/test/render3/instructions/lview_debug_spec.ts @@ -8,10 +8,10 @@ import {Component, Injectable, ɵɵdefineComponent, ɵɵdefineDirective, ɵɵdirectiveInject, ɵɵProvidersFeature} from '@angular/core/src/core'; import {ComponentDef, DirectiveDef} from '@angular/core/src/render3'; +import {readPatchedData} from '@angular/core/src/render3/context_discovery'; import {ɵɵelement, ɵɵelementEnd, ɵɵelementStart} from '@angular/core/src/render3/instructions/element'; import {TNodeDebug} from '@angular/core/src/render3/instructions/lview_debug'; import {createTNode, createTView} from '@angular/core/src/render3/instructions/shared'; -import {MONKEY_PATCH_KEY_NAME} from '@angular/core/src/render3/interfaces/context'; import {TNodeType} from '@angular/core/src/render3/interfaces/node'; import {LView, LViewDebug, TView, TViewType} from '@angular/core/src/render3/interfaces/view'; import {enterView, leaveView} from '@angular/core/src/render3/state'; @@ -278,7 +278,7 @@ describe('lView_debug', () => { const parentFixture = TestBed.createComponent(ParentComponent); const parentHostElement = parentFixture.nativeElement as HTMLElement; const childElement = parentHostElement.querySelector('child')! as HTMLElement; - if (!(childElement as any)[MONKEY_PATCH_KEY_NAME]) { + if (!readPatchedData(childElement)) { // In these browsers: // - Chrome Mobile 72.0.3626 (Android 0.0.0) // - IE 11.0.0 (Windows 8.1.0.0) diff --git a/packages/core/test/render3/integration_spec.ts b/packages/core/test/render3/integration_spec.ts index afc8334410..fdd830d44b 100644 --- a/packages/core/test/render3/integration_spec.ts +++ b/packages/core/test/render3/integration_spec.ts @@ -8,10 +8,9 @@ import {RElement} from '@angular/core/src/render3/interfaces/renderer_dom'; import {RendererType2} from '../../src/render/api_flags'; -import {getLContext} from '../../src/render3/context_discovery'; +import {getLContext, readPatchedData} from '../../src/render3/context_discovery'; import {AttributeMarker, ɵɵadvance, ɵɵattribute, ɵɵdefineComponent, ɵɵdefineDirective, ɵɵhostProperty, ɵɵproperty} from '../../src/render3/index'; import {ɵɵelement, ɵɵelementEnd, ɵɵelementStart, ɵɵprojection, ɵɵprojectionDef, ɵɵtemplate, ɵɵtext} from '../../src/render3/instructions/all'; -import {MONKEY_PATCH_KEY_NAME} from '../../src/render3/interfaces/context'; import {RenderFlags} from '../../src/render3/interfaces/definition'; import {domRendererFactory3, Renderer3, RendererFactory3} from '../../src/render3/interfaces/renderer'; import {CONTEXT, HEADER_OFFSET} from '../../src/render3/interfaces/view'; @@ -269,8 +268,8 @@ describe('element discovery', () => { const parent = host.querySelector('div') as any; const child = host.querySelector('p') as any; - expect(parent[MONKEY_PATCH_KEY_NAME]).toBeTruthy(); - expect(child[MONKEY_PATCH_KEY_NAME]).toBeFalsy(); + expect(readPatchedData(parent)).toBeTruthy(); + expect(readPatchedData(child)).toBeFalsy(); }); it('should only monkey-patch immediate child nodes in a sub component', () => { @@ -317,12 +316,12 @@ describe('element discovery', () => { const host = fixture.hostElement; const child = host.querySelector('child-comp') as any; - expect(child[MONKEY_PATCH_KEY_NAME]).toBeTruthy(); + expect(readPatchedData(child)).toBeTruthy(); const [kid1, kid2, kid3] = Array.from(host.querySelectorAll('child-comp > *')); - expect(kid1[MONKEY_PATCH_KEY_NAME]).toBeTruthy(); - expect(kid2[MONKEY_PATCH_KEY_NAME]).toBeTruthy(); - expect(kid3[MONKEY_PATCH_KEY_NAME]).toBeTruthy(); + expect(readPatchedData(kid1)).toBeTruthy(); + expect(readPatchedData(kid2)).toBeTruthy(); + expect(readPatchedData(kid3)).toBeTruthy(); }); it('should only monkey-patch immediate child nodes in an embedded template container', () => { @@ -364,16 +363,16 @@ describe('element discovery', () => { const [section, div1, p, div2] = Array.from(host.querySelectorAll('section, div, p')); expect(section.nodeName.toLowerCase()).toBe('section'); - expect(section[MONKEY_PATCH_KEY_NAME]).toBeTruthy(); + expect(readPatchedData(section)).toBeTruthy(); expect(div1.nodeName.toLowerCase()).toBe('div'); - expect(div1[MONKEY_PATCH_KEY_NAME]).toBeTruthy(); + expect(readPatchedData(div1)).toBeTruthy(); expect(p.nodeName.toLowerCase()).toBe('p'); - expect(p[MONKEY_PATCH_KEY_NAME]).toBeFalsy(); + expect(readPatchedData(p)).toBeFalsy(); expect(div2.nodeName.toLowerCase()).toBe('div'); - expect(div2[MONKEY_PATCH_KEY_NAME]).toBeTruthy(); + expect(readPatchedData(div2)).toBeTruthy(); }); it('should return a context object from a given dom node', () => { @@ -436,11 +435,11 @@ describe('element discovery', () => { fixture.update(); const section = fixture.hostElement.querySelector('section')! as any; - const result1 = section[MONKEY_PATCH_KEY_NAME]; + const result1 = readPatchedData(section); expect(Array.isArray(result1)).toBeTruthy(); const context = getLContext(section)!; - const result2 = section[MONKEY_PATCH_KEY_NAME]; + const result2 = readPatchedData(section) as any; expect(Array.isArray(result2)).toBeFalsy(); expect(result2).toBe(context); @@ -471,14 +470,14 @@ describe('element discovery', () => { fixture.update(); const section = fixture.hostElement.querySelector('section')! as any; - expect(section[MONKEY_PATCH_KEY_NAME]).toBeTruthy(); + expect(readPatchedData(section)).toBeTruthy(); const p = fixture.hostElement.querySelector('p')! as any; - expect(p[MONKEY_PATCH_KEY_NAME]).toBeFalsy(); + expect(readPatchedData(p)).toBeFalsy(); const pContext = getLContext(p)!; expect(pContext.native).toBe(p); - expect(p[MONKEY_PATCH_KEY_NAME]).toBe(pContext); + expect(readPatchedData(p)).toBe(pContext); }); it('should be able to pull in element context data even if the element is decorated using styling', @@ -503,14 +502,14 @@ describe('element discovery', () => { fixture.update(); const section = fixture.hostElement.querySelector('section')! as any; - const result1 = section[MONKEY_PATCH_KEY_NAME]; + const result1 = readPatchedData(section) as any; expect(Array.isArray(result1)).toBeTruthy(); const elementResult = result1[HEADER_OFFSET]; // first element expect(elementResult).toBe(section); const context = getLContext(section)!; - const result2 = section[MONKEY_PATCH_KEY_NAME]; + const result2 = readPatchedData(section); expect(Array.isArray(result2)).toBeFalsy(); expect(context.native).toBe(section); @@ -596,14 +595,14 @@ describe('element discovery', () => { expect(projectorComp.children).toContain(header); expect(h1.children).toContain(p); - expect(textNode[MONKEY_PATCH_KEY_NAME]).toBeTruthy(); - expect(section[MONKEY_PATCH_KEY_NAME]).toBeTruthy(); - expect(projectorComp[MONKEY_PATCH_KEY_NAME]).toBeTruthy(); - expect(header[MONKEY_PATCH_KEY_NAME]).toBeTruthy(); - expect(h1[MONKEY_PATCH_KEY_NAME]).toBeFalsy(); - expect(p[MONKEY_PATCH_KEY_NAME]).toBeTruthy(); - expect(pText[MONKEY_PATCH_KEY_NAME]).toBeFalsy(); - expect(projectedTextNode[MONKEY_PATCH_KEY_NAME]).toBeTruthy(); + expect(readPatchedData(textNode)).toBeTruthy(); + expect(readPatchedData(section)).toBeTruthy(); + expect(readPatchedData(projectorComp)).toBeTruthy(); + expect(readPatchedData(header)).toBeTruthy(); + expect(readPatchedData(h1)).toBeFalsy(); + expect(readPatchedData(p)).toBeTruthy(); + expect(readPatchedData(pText)).toBeFalsy(); + expect(readPatchedData(projectedTextNode)).toBeTruthy(); const parentContext = getLContext(section)!; const shadowContext = getLContext(header)!; @@ -676,10 +675,10 @@ describe('element discovery', () => { const hostElm = fixture.hostElement; const component = fixture.component; - const componentLView = (component as any)[MONKEY_PATCH_KEY_NAME]; + const componentLView = readPatchedData(component); expect(Array.isArray(componentLView)).toBeTruthy(); - const hostLView = (hostElm as any)[MONKEY_PATCH_KEY_NAME]; + const hostLView = readPatchedData(hostElm) as any; expect(hostLView).toBe(componentLView); const context1 = getLContext(hostElm)!; @@ -745,9 +744,9 @@ describe('element discovery', () => { expect(componentView).toContain(myDir2Instance); expect(componentView).toContain(myDir3Instance); - expect(Array.isArray((myDir1Instance as any)[MONKEY_PATCH_KEY_NAME])).toBeTruthy(); - expect(Array.isArray((myDir2Instance as any)[MONKEY_PATCH_KEY_NAME])).toBeTruthy(); - expect(Array.isArray((myDir3Instance as any)[MONKEY_PATCH_KEY_NAME])).toBeTruthy(); + expect(Array.isArray(readPatchedData(myDir1Instance))).toBeTruthy(); + expect(Array.isArray(readPatchedData(myDir2Instance))).toBeTruthy(); + expect(Array.isArray(readPatchedData(myDir3Instance))).toBeTruthy(); const d1Context = getLContext(myDir1Instance)!; const d2Context = getLContext(myDir2Instance)!; @@ -757,9 +756,9 @@ describe('element discovery', () => { expect(d2Context.lView).toEqual(componentView); expect(d3Context.lView).toEqual(componentView); - expect((myDir1Instance as any)[MONKEY_PATCH_KEY_NAME]).toBe(d1Context); - expect((myDir2Instance as any)[MONKEY_PATCH_KEY_NAME]).toBe(d2Context); - expect((myDir3Instance as any)[MONKEY_PATCH_KEY_NAME]).toBe(d3Context); + expect(readPatchedData(myDir1Instance)).toBe(d1Context); + expect(readPatchedData(myDir2Instance)).toBe(d2Context); + expect(readPatchedData(myDir3Instance)).toBe(d3Context); expect(d1Context.nodeIndex).toEqual(HEADER_OFFSET); expect(d1Context.native).toBe(div1); @@ -829,11 +828,11 @@ describe('element discovery', () => { const childCompHostElm = fixture.hostElement.querySelector('child-comp')! as any; - const lView = childCompHostElm[MONKEY_PATCH_KEY_NAME]; + const lView = readPatchedData(childCompHostElm); expect(Array.isArray(lView)).toBeTruthy(); - expect((myDir1Instance as any)[MONKEY_PATCH_KEY_NAME]).toBe(lView); - expect((myDir2Instance as any)[MONKEY_PATCH_KEY_NAME]).toBe(lView); - expect((childComponentInstance as any)[MONKEY_PATCH_KEY_NAME]).toBe(lView); + expect(readPatchedData(myDir1Instance)).toBe(lView); + expect(readPatchedData(myDir2Instance)).toBe(lView); + expect(readPatchedData(childComponentInstance)).toBe(lView); const childNodeContext = getLContext(childCompHostElm)!; expect(childNodeContext.component).toBeFalsy(); @@ -864,7 +863,7 @@ describe('element discovery', () => { assertMonkeyPatchValueIsLView(childComponentInstance, false); function assertMonkeyPatchValueIsLView(value: any, yesOrNo = true) { - expect(Array.isArray((value as any)[MONKEY_PATCH_KEY_NAME])).toBe(yesOrNo); + expect(Array.isArray(readPatchedData(value))).toBe(yesOrNo); } }); @@ -913,18 +912,18 @@ describe('element discovery', () => { const host = fixture.hostElement; const child = host.querySelector('child-comp') as any; - expect(child[MONKEY_PATCH_KEY_NAME]).toBeTruthy(); + expect(readPatchedData(child)).toBeTruthy(); const context = getLContext(child)!; - expect(child[MONKEY_PATCH_KEY_NAME]).toBeTruthy(); + expect(readPatchedData(child)).toBeTruthy(); const componentData = context.lView[context.nodeIndex]; const component = componentData[CONTEXT]; expect(component instanceof ChildComp).toBeTruthy(); - expect(component[MONKEY_PATCH_KEY_NAME]).toBe(context.lView); + expect(readPatchedData(component)).toBe(context.lView); const componentContext = getLContext(component)!; - expect(component[MONKEY_PATCH_KEY_NAME]).toBe(componentContext); + expect(readPatchedData(component)).toBe(componentContext); expect(componentContext.nodeIndex).toEqual(context.nodeIndex); expect(componentContext.native).toEqual(context.native); expect(componentContext.lView).toEqual(context.lView);