From e63a7b0532ef618cee56a8ec1737956c49847c79 Mon Sep 17 00:00:00 2001 From: Pawel Kozlowski Date: Thu, 22 Aug 2019 11:43:24 +0200 Subject: [PATCH] refactor(ivy): replace enter / leave view with selectView (#32263) After a series of recent refactorings `enterView` and `leaveView` became identical. This PR merges both into one concept of view selectio (similar to a node selection). This reduces number of concepts and code size. PR Close #32263 --- packages/core/src/render3/component.ts | 6 ++--- packages/core/src/render3/component_ref.ts | 6 ++--- .../src/render3/instructions/embedded_view.ts | 8 +++---- .../core/src/render3/instructions/shared.ts | 10 ++++----- packages/core/src/render3/state.ts | 22 +++++-------------- .../cyclic_import/bundle.golden_symbols.json | 9 +++----- .../hello_world/bundle.golden_symbols.json | 9 +++----- .../bundling/todo/bundle.golden_symbols.json | 9 +++----- packages/core/test/render3/di_spec.ts | 6 ++--- packages/core/test/render3/render_util.ts | 4 ++-- 10 files changed, 35 insertions(+), 54 deletions(-) diff --git a/packages/core/src/render3/component.ts b/packages/core/src/render3/component.ts index 42525c6597..cb5a26d071 100644 --- a/packages/core/src/render3/component.ts +++ b/packages/core/src/render3/component.ts @@ -23,7 +23,7 @@ import {TElementNode, TNode, TNodeType} from './interfaces/node'; import {PlayerHandler} from './interfaces/player'; import {RElement, Renderer3, RendererFactory3, domRendererFactory3} from './interfaces/renderer'; import {CONTEXT, HEADER_OFFSET, LView, LViewFlags, RootContext, RootContextFlags, TVIEW} from './interfaces/view'; -import {enterView, getPreviousOrParentTNode, leaveView, resetComponentState, setActiveHostElement} from './state'; +import {getPreviousOrParentTNode, resetComponentState, selectView, setActiveHostElement} from './state'; import {publishDefaultGlobalUtils} from './util/global_utils'; import {defaultScheduler, stringifyForError} from './util/misc_utils'; import {getRootContext} from './util/view_traversal_utils'; @@ -133,7 +133,7 @@ export function renderComponent( null, rootTView, rootContext, rootFlags, null, null, rendererFactory, renderer, undefined, opts.injector || null); - const oldView = enterView(rootView, null); + const oldView = selectView(rootView, null); let component: T; try { @@ -149,7 +149,7 @@ export function renderComponent( refreshView(rootView, rootTView, null, null); } finally { - leaveView(oldView); + selectView(oldView, null); if (rendererFactory.end) rendererFactory.end(); } diff --git a/packages/core/src/render3/component_ref.ts b/packages/core/src/render3/component_ref.ts index 469b3b1b85..23dba94988 100644 --- a/packages/core/src/render3/component_ref.ts +++ b/packages/core/src/render3/component_ref.ts @@ -29,7 +29,7 @@ import {ComponentDef} from './interfaces/definition'; import {TContainerNode, TElementContainerNode, TElementNode} from './interfaces/node'; import {RNode, RendererFactory3, domRendererFactory3, isProceduralRenderer} from './interfaces/renderer'; import {LView, LViewFlags, TVIEW} from './interfaces/view'; -import {enterView, leaveView, namespaceHTMLInternal} from './state'; +import {namespaceHTMLInternal, selectView} from './state'; import {defaultScheduler} from './util/misc_utils'; import {getTNode} from './util/view_utils'; import {createElementRef} from './view_engine_compatibility'; @@ -167,7 +167,7 @@ export class ComponentFactory extends viewEngine_ComponentFactory { rootViewInjector); // rootView is the parent when bootstrapping - const oldLView = enterView(rootLView, null); + const oldLView = selectView(rootLView, null); let component: T; let tElementNode: TElementNode; @@ -194,7 +194,7 @@ export class ComponentFactory extends viewEngine_ComponentFactory { renderView(rootLView, rootTView, null); } finally { - leaveView(oldLView); + selectView(oldLView, null); } const componentRef = new ComponentRef( diff --git a/packages/core/src/render3/instructions/embedded_view.ts b/packages/core/src/render3/instructions/embedded_view.ts index 1e65eebd88..a366833d9a 100644 --- a/packages/core/src/render3/instructions/embedded_view.ts +++ b/packages/core/src/render3/instructions/embedded_view.ts @@ -14,7 +14,7 @@ import {TContainerNode, TNodeType} from '../interfaces/node'; import {CONTEXT, LView, LViewFlags, PARENT, TVIEW, TView, T_HOST} from '../interfaces/view'; import {assertNodeType} from '../node_assert'; import {insertView, removeView} from '../node_manipulation'; -import {enterView, getIsParent, getLView, getPreviousOrParentTNode, leaveView, setIsParent, setPreviousOrParentTNode} from '../state'; +import {getIsParent, getLView, getPreviousOrParentTNode, selectView, setIsParent, setPreviousOrParentTNode} from '../state'; import {isCreationMode} from '../util/view_utils'; import {assignTViewNodeToLView, createLView, createTView, refreshView, renderView} from './shared'; @@ -43,7 +43,7 @@ export function ɵɵembeddedViewStart( if (viewToRender) { setIsParent(); - enterView(viewToRender, viewToRender[TVIEW].node); + selectView(viewToRender, viewToRender[TVIEW].node); } else { // When we create a new LView, we always reset the state of the instructions. viewToRender = createLView( @@ -54,7 +54,7 @@ export function ɵɵembeddedViewStart( const tParentNode = getIsParent() ? previousOrParentTNode : previousOrParentTNode && previousOrParentTNode.parent; assignTViewNodeToLView(viewToRender[TVIEW], tParentNode, viewBlockId, viewToRender); - enterView(viewToRender, viewToRender[TVIEW].node); + selectView(viewToRender, viewToRender[TVIEW].node); } if (lContainer) { if (isCreationMode(viewToRender)) { @@ -139,6 +139,6 @@ export function ɵɵembeddedViewEnd(): void { const lContainer = lView[PARENT] as LContainer; ngDevMode && assertLContainerOrUndefined(lContainer); - leaveView(lContainer[PARENT] !); + selectView(lContainer[PARENT] !, null); setPreviousOrParentTNode(viewHost !, false); } diff --git a/packages/core/src/render3/instructions/shared.ts b/packages/core/src/render3/instructions/shared.ts index af4287b5a1..b1303b2b94 100644 --- a/packages/core/src/render3/instructions/shared.ts +++ b/packages/core/src/render3/instructions/shared.ts @@ -28,7 +28,7 @@ import {isComponent, isComponentDef, isContentQueryHost, isLContainer, isRootVie import {BINDING_INDEX, CHILD_HEAD, CHILD_TAIL, CLEANUP, CONTEXT, DECLARATION_VIEW, ExpandoInstructions, FLAGS, HEADER_OFFSET, HOST, INJECTOR, InitPhaseState, LView, LViewFlags, NEXT, PARENT, RENDERER, RENDERER_FACTORY, RootContext, RootContextFlags, SANITIZER, TData, TVIEW, TView, T_HOST} from '../interfaces/view'; import {assertNodeOfPossibleTypes} from '../node_assert'; import {isNodeMatchingSelectorList} from '../node_selector_matcher'; -import {enterView, getBindingsEnabled, getCheckNoChangesMode, getIsParent, getLView, getPreviousOrParentTNode, getSelectedIndex, incrementActiveDirectiveId, leaveView, namespaceHTMLInternal, setActiveHostElement, setBindingRoot, setCheckNoChangesMode, setCurrentDirectiveDef, setCurrentQueryIndex, setPreviousOrParentTNode, setSelectedIndex} from '../state'; +import {getBindingsEnabled, getCheckNoChangesMode, getIsParent, getLView, getPreviousOrParentTNode, getSelectedIndex, incrementActiveDirectiveId, namespaceHTMLInternal, selectView, setActiveHostElement, setBindingRoot, setCheckNoChangesMode, setCurrentDirectiveDef, setCurrentQueryIndex, setPreviousOrParentTNode, setSelectedIndex} from '../state'; import {renderStylingMap} from '../styling_next/bindings'; import {NO_CHANGE} from '../tokens'; import {ANIMATION_PROP_PREFIX, isAnimationProp} from '../util/attrs_utils'; @@ -312,7 +312,7 @@ export function allocExpando(view: LView, numSlotsToAlloc: number) { */ export function renderView(lView: LView, tView: TView, context: T): void { ngDevMode && assertEqual(isCreationMode(lView), true, 'Should be run in creation mode'); - const oldView = enterView(lView, lView[T_HOST]); + const oldView = selectView(lView, lView[T_HOST]); try { const viewQuery = tView.viewQuery; if (viewQuery !== null) { @@ -357,7 +357,7 @@ export function renderView(lView: LView, tView: TView, context: T): void { } finally { lView[FLAGS] &= ~LViewFlags.CreationMode; - leaveView(oldView); + selectView(oldView, null); } } @@ -372,7 +372,7 @@ export function renderView(lView: LView, tView: TView, context: T): void { export function refreshView( lView: LView, tView: TView, templateFn: ComponentTemplate<{}>| null, context: T) { ngDevMode && assertEqual(isCreationMode(lView), false, 'Should be run in update mode'); - const oldView = enterView(lView, lView[T_HOST]); + const oldView = selectView(lView, lView[T_HOST]); const flags = lView[FLAGS]; try { resetPreOrderHookFlags(lView); @@ -460,7 +460,7 @@ export function refreshView( } finally { lView[FLAGS] &= ~(LViewFlags.Dirty | LViewFlags.FirstLViewPass); - leaveView(oldView); + selectView(oldView, null); } } diff --git a/packages/core/src/render3/state.ts b/packages/core/src/render3/state.ts index 9304a95eb9..0fbf4234ac 100644 --- a/packages/core/src/render3/state.ts +++ b/packages/core/src/render3/state.ts @@ -400,18 +400,18 @@ export function setCurrentQueryIndex(value: number): void { } /** - * Swap the current state with a new state. + * Swap the current lView with a new lView. * - * For performance reasons we store the state in the top level of the module. + * For performance reasons we store the lView in the top level of the module. * This way we minimize the number of properties to read. Whenever a new view - * is entered we have to store the state for later, and when the view is + * is entered we have to store the lView for later, and when the view is * exited the state has to be restored * - * @param newView New state to become active + * @param newView New lView to become active * @param host Element to which the View is a child of - * @returns the previous state; + * @returns the previously active lView; */ -export function enterView(newView: LView, hostTNode: TElementNode | TViewNode | null): LView { +export function selectView(newView: LView, hostTNode: TElementNode | TViewNode | null): LView { ngDevMode && assertLViewOrUndefined(newView); const oldView = lView; @@ -450,16 +450,6 @@ export function resetComponentState() { resetAllStylingState(); } -/** - * Used in lieu of enterView to make it clear when we are exiting a child view. This makes - * the direction of traversal (up or down the view tree) a bit clearer. - * - * @param newView New LView to become active - */ -export function leaveView(newView: LView): void { - enterView(newView, null); -} - let _selectedIndex = -1; /** diff --git a/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json b/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json index 266c913290..4e6c137884 100644 --- a/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json +++ b/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json @@ -248,9 +248,6 @@ { "name": "elementCreate" }, - { - "name": "enterView" - }, { "name": "executeCheckHooks" }, @@ -488,9 +485,6 @@ { "name": "isStylingValueDefined" }, - { - "name": "leaveView" - }, { "name": "locateHostElement" }, @@ -596,6 +590,9 @@ { "name": "selectInternal" }, + { + "name": "selectView" + }, { "name": "setActiveHostElement" }, diff --git a/packages/core/test/bundling/hello_world/bundle.golden_symbols.json b/packages/core/test/bundling/hello_world/bundle.golden_symbols.json index 6c01d3daba..c1b02adcf8 100644 --- a/packages/core/test/bundling/hello_world/bundle.golden_symbols.json +++ b/packages/core/test/bundling/hello_world/bundle.golden_symbols.json @@ -206,9 +206,6 @@ { "name": "domRendererFactory3" }, - { - "name": "enterView" - }, { "name": "executeCheckHooks" }, @@ -359,9 +356,6 @@ { "name": "isRootView" }, - { - "name": "leaveView" - }, { "name": "locateHostElement" }, @@ -437,6 +431,9 @@ { "name": "selectInternal" }, + { + "name": "selectView" + }, { "name": "setActiveHostElement" }, diff --git a/packages/core/test/bundling/todo/bundle.golden_symbols.json b/packages/core/test/bundling/todo/bundle.golden_symbols.json index 59561d7d39..5b48ecba11 100644 --- a/packages/core/test/bundling/todo/bundle.golden_symbols.json +++ b/packages/core/test/bundling/todo/bundle.golden_symbols.json @@ -629,9 +629,6 @@ { "name": "elementPropertyInternal" }, - { - "name": "enterView" - }, { "name": "executeCheckHooks" }, @@ -1058,9 +1055,6 @@ { "name": "iterateListLike" }, - { - "name": "leaveView" - }, { "name": "listenerInternal" }, @@ -1256,6 +1250,9 @@ { "name": "selectInternal" }, + { + "name": "selectView" + }, { "name": "setActiveHostElement" }, diff --git a/packages/core/test/render3/di_spec.ts b/packages/core/test/render3/di_spec.ts index 4bc84f84f5..7910eab2b5 100644 --- a/packages/core/test/render3/di_spec.ts +++ b/packages/core/test/render3/di_spec.ts @@ -17,7 +17,7 @@ import {TNODE} from '../../src/render3/interfaces/injector'; import {TNodeType} from '../../src/render3/interfaces/node'; import {isProceduralRenderer} from '../../src/render3/interfaces/renderer'; import {LViewFlags, TVIEW} from '../../src/render3/interfaces/view'; -import {enterView, leaveView} from '../../src/render3/state'; +import {selectView} from '../../src/render3/state'; import {ViewRef} from '../../src/render3/view_ref'; import {getRendererFactory2} from './imported_renderer2'; @@ -606,7 +606,7 @@ describe('di', () => { const contentView = createLView( null, createTView(-1, null, 1, 0, null, null, null, null), null, LViewFlags.CheckAlways, null, null, {} as any, {} as any); - const oldView = enterView(contentView, null); + const oldView = selectView(contentView, null); try { const parentTNode = getOrCreateTNode(contentView[TVIEW], null, 0, TNodeType.Element, null, null); @@ -618,7 +618,7 @@ describe('di', () => { const injector = getOrCreateNodeInjectorForNode(parentTNode, contentView); expect(injector).not.toEqual(-1); } finally { - leaveView(oldView); + selectView(oldView, null); } }); }); diff --git a/packages/core/test/render3/render_util.ts b/packages/core/test/render3/render_util.ts index 46e8dea6fa..30af7ef15a 100644 --- a/packages/core/test/render3/render_util.ts +++ b/packages/core/test/render3/render_util.ts @@ -14,7 +14,7 @@ import {ViewContainerRef} from '@angular/core/src/linker/view_container_ref'; import {Renderer2} from '@angular/core/src/render/api'; import {createLView, createTView, getOrCreateTNode, getOrCreateTView, renderComponentOrTemplate} from '@angular/core/src/render3/instructions/shared'; import {TNodeType} from '@angular/core/src/render3/interfaces/node'; -import {enterView, getLView, resetComponentState} from '@angular/core/src/render3/state'; +import {getLView, resetComponentState, selectView} from '@angular/core/src/render3/state'; import {stringifyElement} from '@angular/platform-browser/testing/src/browser_util'; import {SWITCH_CHANGE_DETECTOR_REF_FACTORY__POST_R3__ as R3_CHANGE_DETECTOR_REF_FACTORY} from '../../src/change_detection/change_detector_ref'; @@ -257,7 +257,7 @@ export function renderTemplate( const hostLView = createLView( null, tView, {}, LViewFlags.CheckAlways | LViewFlags.IsRoot, null, null, providedRendererFactory, renderer); - enterView(hostLView, null); // SUSPECT! why do we need to enter the View? + selectView(hostLView, null); // SUSPECT! why do we need to enter the View? const def: ComponentDef = ɵɵdefineComponent({ factory: () => null,