From a20b2f72f256a18d54dee7cc27cc07d4ab863e57 Mon Sep 17 00:00:00 2001 From: Kara Erickson Date: Tue, 18 Dec 2018 16:58:51 -0800 Subject: [PATCH] fix(ivy): process creation mode deeply before running update mode (#27744) Prior to this commit, we had two different modes for change detection execution for Ivy, depending on whether you called `bootstrap()` or `renderComponent()`. In the former case, we would complete creation mode for all components in the tree before beginning update mode for any component. In the latter case, we would run creation mode and update mode together for each component individually. Maintaining code to support these two different execution orders was unnecessarily complex, so this commit aligns the two bootstrapping mechanisms to execute in the same order. Now creation mode always runs for all components before update mode begins. This change also simplifies our rendering logic so that we use `LView` flags as the source of truth for rendering mode instead of `rf` function arguments. This fixed some related bugs (e.g. calling `ViewRef.detectChanges` synchronously after the view's creation would create view nodes twice, view queries would execute twice, etc). PR Close #27744 --- packages/common/test/BUILD.bazel | 1 + .../directives/ng_component_outlet_spec.ts | 24 ++- packages/core/src/render3/bindings.ts | 4 +- packages/core/src/render3/component.ts | 6 +- packages/core/src/render3/component_ref.ts | 5 +- packages/core/src/render3/hooks.ts | 19 +- packages/core/src/render3/instructions.ts | 167 ++++++++-------- packages/core/src/render3/interfaces/view.ts | 23 ++- packages/core/src/render3/pure_function.ts | 4 +- packages/core/src/render3/state.ts | 31 ++- .../src/render3/view_engine_compatibility.ts | 2 +- packages/core/src/render3/view_ref.ts | 13 +- .../animation/animation_integration_spec.ts | 6 +- .../animation_query_integration_spec.ts | 7 +- ...s_keyframes_animations_integration_spec.ts | 5 + packages/core/test/application_ref_spec.ts | 7 +- .../hello_world/bundle.golden_symbols.json | 12 +- .../bundling/todo/bundle.golden_symbols.json | 9 +- .../test/linker/query_integration_spec.ts | 119 ++++++------ .../test/render3/change_detection_spec.ts | 178 +++++++++++------- packages/core/test/render3/component_spec.ts | 3 + .../core/test/render3/control_flow_spec.ts | 2 +- .../core/test/render3/integration_spec.ts | 4 +- packages/core/test/render3/properties_spec.ts | 4 +- .../test/render3/renderer_factory_spec.ts | 32 +++- 25 files changed, 384 insertions(+), 303 deletions(-) diff --git a/packages/common/test/BUILD.bazel b/packages/common/test/BUILD.bazel index f7c3831581..3a7c7cf38f 100644 --- a/packages/common/test/BUILD.bazel +++ b/packages/common/test/BUILD.bazel @@ -15,6 +15,7 @@ ts_library( "//packages/platform-browser", "//packages/platform-browser-dynamic", "//packages/platform-browser/testing", + "//packages/private/testing", ], ) diff --git a/packages/common/test/directives/ng_component_outlet_spec.ts b/packages/common/test/directives/ng_component_outlet_spec.ts index d3552c2cca..bc8ae923df 100644 --- a/packages/common/test/directives/ng_component_outlet_spec.ts +++ b/packages/common/test/directives/ng_component_outlet_spec.ts @@ -11,6 +11,7 @@ import {NgComponentOutlet} from '@angular/common/src/directives/ng_component_out import {Compiler, Component, ComponentRef, Inject, InjectionToken, Injector, NO_ERRORS_SCHEMA, NgModule, NgModuleFactory, Optional, QueryList, TemplateRef, Type, ViewChild, ViewChildren, ViewContainerRef} from '@angular/core'; import {TestBed, async} from '@angular/core/testing'; import {expect} from '@angular/platform-browser/testing/src/matchers'; +import {modifiedInIvy} from '@angular/private/testing'; describe('insert/remove', () => { @@ -106,17 +107,20 @@ describe('insert/remove', () => { })); - it('should resolve a with injector', async(() => { - let fixture = TestBed.createComponent(TestComponent); - fixture.componentInstance.cmpRef = null; - fixture.componentInstance.currentComponent = InjectedComponent; - fixture.detectChanges(); - let cmpRef: ComponentRef = fixture.componentInstance.cmpRef !; - expect(cmpRef).toBeAnInstanceOf(ComponentRef); - expect(cmpRef.instance).toBeAnInstanceOf(InjectedComponent); - expect(cmpRef.instance.testToken).toBeNull(); - })); + modifiedInIvy('Static ViewChild and ContentChild queries are resolved in update mode') + .it('should resolve with an injector', async(() => { + let fixture = TestBed.createComponent(TestComponent); + + // We are accessing a ViewChild (ngComponentOutlet) before change detection has run + fixture.componentInstance.cmpRef = null; + fixture.componentInstance.currentComponent = InjectedComponent; + fixture.detectChanges(); + let cmpRef: ComponentRef = fixture.componentInstance.cmpRef !; + expect(cmpRef).toBeAnInstanceOf(ComponentRef); + expect(cmpRef.instance).toBeAnInstanceOf(InjectedComponent); + expect(cmpRef.instance.testToken).toBeNull(); + })); it('should render projectable nodes, if supplied', async(() => { const template = `projected foo${TEST_CMP_TEMPLATE}`; diff --git a/packages/core/src/render3/bindings.ts b/packages/core/src/render3/bindings.ts index 84a70a6eb4..88f63a9bd2 100644 --- a/packages/core/src/render3/bindings.ts +++ b/packages/core/src/render3/bindings.ts @@ -11,7 +11,7 @@ import {devModeEqual} from '../change_detection/change_detection_util'; import {assertDataInRange, assertLessThan, assertNotEqual} from './assert'; import {throwErrorIfNoChangesMode} from './errors'; import {BINDING_INDEX, LView} from './interfaces/view'; -import {getCheckNoChangesMode, getCreationMode} from './state'; +import {getCheckNoChangesMode, isCreationMode} from './state'; import {NO_CHANGE} from './tokens'; import {isDifferent} from './util'; @@ -44,7 +44,7 @@ export function bindingUpdated(lView: LView, bindingIndex: number, value: any): } else if (isDifferent(lView[bindingIndex], value)) { if (ngDevMode && getCheckNoChangesMode()) { if (!devModeEqual(lView[bindingIndex], value)) { - throwErrorIfNoChangesMode(getCreationMode(), lView[bindingIndex], value); + throwErrorIfNoChangesMode(isCreationMode(lView), lView[bindingIndex], value); } } lView[bindingIndex] = value; diff --git a/packages/core/src/render3/component.ts b/packages/core/src/render3/component.ts index 1a50c67d7e..30daba0a46 100644 --- a/packages/core/src/render3/component.ts +++ b/packages/core/src/render3/component.ts @@ -22,7 +22,7 @@ import {ComponentDef, ComponentType, RenderFlags} from './interfaces/definition' import {TElementNode, TNode, TNodeFlags, TNodeType} from './interfaces/node'; import {PlayerHandler} from './interfaces/player'; import {RElement, Renderer3, RendererFactory3, domRendererFactory3} from './interfaces/renderer'; -import {CONTEXT, HEADER_OFFSET, HOST, HOST_NODE, LView, LViewFlags, RootContext, RootContextFlags, TVIEW} from './interfaces/view'; +import {CONTEXT, FLAGS, HEADER_OFFSET, HOST, HOST_NODE, LView, LViewFlags, RootContext, RootContextFlags, TVIEW} from './interfaces/view'; import {enterView, getPreviousOrParentTNode, leaveView, resetComponentState, setCurrentDirectiveDef} from './state'; import {defaultScheduler, getRootView, readPatchedLView, stringify} from './util'; @@ -133,7 +133,9 @@ export function renderComponent( component = createRootComponent( componentView, componentDef, rootView, rootContext, opts.hostFeatures || null); - refreshDescendantViews(rootView, null); + refreshDescendantViews(rootView); // creation mode pass + rootView[FLAGS] &= ~LViewFlags.CreationMode; + refreshDescendantViews(rootView); // update mode pass } finally { leaveView(oldView); if (rendererFactory.end) rendererFactory.end(); diff --git a/packages/core/src/render3/component_ref.ts b/packages/core/src/render3/component_ref.ts index 91e125c411..fc293f5d2e 100644 --- a/packages/core/src/render3/component_ref.ts +++ b/packages/core/src/render3/component_ref.ts @@ -208,10 +208,9 @@ export class ComponentFactory extends viewEngine_ComponentFactory { componentView, this.componentDef, rootLView, rootContext, [LifecycleHooksFeature]); addToViewTree(rootLView, HEADER_OFFSET, componentView); - - refreshDescendantViews(rootLView, RenderFlags.Create); + refreshDescendantViews(rootLView); } finally { - leaveView(oldLView, true); + leaveView(oldLView); if (rendererFactory.end) rendererFactory.end(); } diff --git a/packages/core/src/render3/hooks.ts b/packages/core/src/render3/hooks.ts index 77a10e98c4..c00146b3e9 100644 --- a/packages/core/src/render3/hooks.ts +++ b/packages/core/src/render3/hooks.ts @@ -93,9 +93,10 @@ function queueDestroyHooks(def: DirectiveDef, tView: TView, i: number): voi * * @param currentView The current view */ -export function executeInitHooks(currentView: LView, tView: TView, creationMode: boolean): void { - if (currentView[FLAGS] & LViewFlags.RunInit) { - executeHooks(currentView, tView.initHooks, tView.checkHooks, creationMode); +export function executeInitHooks( + currentView: LView, tView: TView, checkNoChangesMode: boolean): void { + if (!checkNoChangesMode && currentView[FLAGS] & LViewFlags.RunInit) { + executeHooks(currentView, tView.initHooks, tView.checkHooks, checkNoChangesMode); currentView[FLAGS] &= ~LViewFlags.RunInit; } } @@ -106,17 +107,19 @@ export function executeInitHooks(currentView: LView, tView: TView, creationMode: * @param currentView The current view */ export function executeHooks( - data: LView, allHooks: HookData | null, checkHooks: HookData | null, - creationMode: boolean): void { - const hooksToCall = creationMode ? allHooks : checkHooks; + currentView: LView, allHooks: HookData | null, checkHooks: HookData | null, + checkNoChangesMode: boolean): void { + if (checkNoChangesMode) return; + + const hooksToCall = currentView[FLAGS] & LViewFlags.FirstLViewPass ? allHooks : checkHooks; if (hooksToCall) { - callHooks(data, hooksToCall); + callHooks(currentView, hooksToCall); } } /** * Calls lifecycle hooks with their contexts, skipping init hooks if it's not - * creation mode. + * the first LView pass. * * @param currentView The current view * @param arr The array in which the hooks are found diff --git a/packages/core/src/render3/instructions.ts b/packages/core/src/render3/instructions.ts index 80d11dcf31..2f18c17bc4 100644 --- a/packages/core/src/render3/instructions.ts +++ b/packages/core/src/render3/instructions.ts @@ -35,7 +35,7 @@ import {BINDING_INDEX, CLEANUP, CONTAINER_INDEX, CONTENT_QUERIES, CONTEXT, DECLA import {assertNodeOfPossibleTypes, assertNodeType} from './node_assert'; import {appendChild, appendProjectedNode, createTextNode, findComponentView, getLViewChild, getRenderParent, insertView, removeView} from './node_manipulation'; import {isNodeMatchingSelectorList, matchingSelectorIndex} from './node_selector_matcher'; -import {decreaseElementDepthCount, enterView, getBindingsEnabled, getCheckNoChangesMode, getContextLView, getCreationMode, getCurrentDirectiveDef, getElementDepthCount, getFirstTemplatePass, getIsParent, getLView, getPreviousOrParentTNode, increaseElementDepthCount, leaveView, nextContextImpl, resetComponentState, setBindingRoot, setCheckNoChangesMode, setCurrentDirectiveDef, setFirstTemplatePass, setIsParent, setPreviousOrParentTNode} from './state'; +import {decreaseElementDepthCount, enterView, getBindingsEnabled, getCheckNoChangesMode, getContextLView, getCurrentDirectiveDef, getElementDepthCount, getFirstTemplatePass, getIsParent, getLView, getPreviousOrParentTNode, increaseElementDepthCount, isCreationMode, leaveView, nextContextImpl, resetComponentState, setBindingRoot, setCheckNoChangesMode, setCurrentDirectiveDef, setFirstTemplatePass, setIsParent, setPreviousOrParentTNode} from './state'; import {createStylingContextTemplate, renderStyleAndClassBindings, setStyle, updateClassProp as updateElementClassProp, updateStyleProp as updateElementStyleProp, updateStylingMap} from './styling/class_and_style_bindings'; import {BoundPlayerFactory} from './styling/player_factory'; import {getStylingContext, isAnimationProp} from './styling/util'; @@ -60,36 +60,30 @@ const enum BindingDirection { * bindings, refreshes child components. * Note: view hooks are triggered later when leaving the view. */ -export function refreshDescendantViews(lView: LView, rf: RenderFlags | null) { +export function refreshDescendantViews(lView: LView) { const tView = lView[TVIEW]; // This needs to be set before children are processed to support recursive components tView.firstTemplatePass = false; setFirstTemplatePass(false); - // Dynamically created views must run first only in creation mode. If this is a - // creation-only pass, we should not call lifecycle hooks or evaluate bindings. - // This will be done in the update-only pass. - if (rf !== RenderFlags.Create) { - const creationMode = getCreationMode(); + // If this is a creation pass, we should not call lifecycle hooks or evaluate bindings. + // This will be done in the update pass. + if (!isCreationMode(lView)) { const checkNoChangesMode = getCheckNoChangesMode(); - if (!checkNoChangesMode) { - executeInitHooks(lView, tView, creationMode); - } + executeInitHooks(lView, tView, checkNoChangesMode); refreshDynamicEmbeddedViews(lView); // Content query results must be refreshed before content hooks are called. refreshContentQueries(tView); - if (!checkNoChangesMode) { - executeHooks(lView, tView.contentHooks, tView.contentCheckHooks, creationMode); - } + executeHooks(lView, tView.contentHooks, tView.contentCheckHooks, checkNoChangesMode); setHostBindings(tView, lView); } - refreshChildComponents(tView.components, rf); + refreshChildComponents(tView.components); } @@ -147,10 +141,10 @@ function refreshContentQueries(tView: TView): void { } /** Refreshes child components in the current view. */ -function refreshChildComponents(components: number[] | null, rf: RenderFlags | null): void { +function refreshChildComponents(components: number[] | null): void { if (components != null) { for (let i = 0; i < components.length; i++) { - componentRefresh(components[i], rf); + componentRefresh(components[i]); } } } @@ -160,7 +154,8 @@ export function createLView( rendererFactory?: RendererFactory3 | null, renderer?: Renderer3 | null, sanitizer?: Sanitizer | null, injector?: Injector | null): LView { const lView = tView.blueprint.slice() as LView; - lView[FLAGS] = flags | LViewFlags.CreationMode | LViewFlags.Attached | LViewFlags.RunInit; + lView[FLAGS] = flags | LViewFlags.CreationMode | LViewFlags.Attached | LViewFlags.RunInit | + LViewFlags.FirstLViewPass; lView[PARENT] = lView[DECLARATION_VIEW] = parentLView; lView[CONTEXT] = context; lView[RENDERER_FACTORY] = (rendererFactory || parentLView && parentLView[RENDERER_FACTORY]) !; @@ -303,8 +298,7 @@ export function renderTemplate( renderer, sanitizer); hostView[HOST_NODE] = createNodeAtIndex(0, TNodeType.Element, hostNode, null, null); } - renderComponentOrTemplate(hostView, context, null, templateFn); - + renderComponentOrTemplate(hostView, context, templateFn); return hostView; } @@ -348,8 +342,7 @@ export function createEmbeddedViewAndNode( * can't store TViews in the template function itself (as we do for comps). Instead, we store the * TView for dynamically created views on their host TNode, which only has one instance. */ -export function renderEmbeddedTemplate( - viewToRender: LView, tView: TView, context: T, rf: RenderFlags) { +export function renderEmbeddedTemplate(viewToRender: LView, tView: TView, context: T) { const _isParent = getIsParent(); const _previousOrParentTNode = getPreviousOrParentTNode(); setIsParent(true); @@ -365,22 +358,17 @@ export function renderEmbeddedTemplate( oldView = enterView(viewToRender, viewToRender[HOST_NODE]); namespaceHTML(); - tView.template !(rf, context); - if (rf & RenderFlags.Update) { - refreshDescendantViews(viewToRender, null); - } else { - // This must be set to false immediately after the first creation run because in an - // ngFor loop, all the views will be created together before update mode runs and turns - // off firstTemplatePass. If we don't set it here, instances will perform directive - // matching, etc again and again. - viewToRender[TVIEW].firstTemplatePass = false; - setFirstTemplatePass(false); - } + tView.template !(getRenderFlags(viewToRender), context); + // This must be set to false immediately after the first creation run because in an + // ngFor loop, all the views will be created together before update mode runs and turns + // off firstTemplatePass. If we don't set it here, instances will perform directive + // matching, etc again and again. + viewToRender[TVIEW].firstTemplatePass = false; + setFirstTemplatePass(false); + + refreshDescendantViews(viewToRender); } finally { - // renderEmbeddedTemplate() is called twice, once for creation only and then once for - // update. When for creation only, leaveView() must not trigger view hooks, nor clean flags. - const isCreationOnly = (rf & RenderFlags.Create) === RenderFlags.Create; - leaveView(oldView !, isCreationOnly); + leaveView(oldView !); setIsParent(_isParent); setPreviousOrParentTNode(_previousOrParentTNode); } @@ -402,19 +390,28 @@ export function nextContext(level: number = 1): T { } function renderComponentOrTemplate( - hostView: LView, componentOrContext: T, rf: RenderFlags | null, - templateFn?: ComponentTemplate) { + hostView: LView, context: T, templateFn?: ComponentTemplate) { const rendererFactory = hostView[RENDERER_FACTORY]; const oldView = enterView(hostView, hostView[HOST_NODE]); try { if (rendererFactory.begin) { rendererFactory.begin(); } - if (templateFn) { - namespaceHTML(); - templateFn(rf || getRenderFlags(hostView), componentOrContext !); + + if (isCreationMode(hostView)) { + // creation mode pass + if (templateFn) { + namespaceHTML(); + templateFn(RenderFlags.Create, context !); + } + + refreshDescendantViews(hostView); + hostView[FLAGS] &= ~LViewFlags.CreationMode; } - refreshDescendantViews(hostView, rf); + + // update mode pass + templateFn && templateFn(RenderFlags.Update, context !); + refreshDescendantViews(hostView); } finally { if (rendererFactory.end) { rendererFactory.end(); @@ -425,16 +422,11 @@ function renderComponentOrTemplate( /** * This function returns the default configuration of rendering flags depending on when the - * template is in creation mode or update mode. By default, the update block is run with the - * creation block when the view is in creation mode. Otherwise, the update block is run - * alone. - * - * Dynamically created views do NOT use this configuration (update block and create block are - * always run separately). + * template is in creation mode or update mode. Update block and create block are + * always run separately. */ function getRenderFlags(view: LView): RenderFlags { - return view[FLAGS] & LViewFlags.CreationMode ? RenderFlags.Create | RenderFlags.Update : - RenderFlags.Update; + return isCreationMode(view) ? RenderFlags.Create : RenderFlags.Update; } ////////////////////////// @@ -1160,7 +1152,7 @@ export function elementStyling( styleDeclarations?: (string | boolean | InitialStylingFlags)[] | null, styleSanitizer?: StyleSanitizeFn | null, directive?: {}): void { if (directive != undefined) { - getCreationMode() && + isCreationMode() && hackImplementationOfElementStyling( classDeclarations || null, styleDeclarations || null, styleSanitizer || null, directive); // supported in next PR @@ -1214,7 +1206,7 @@ export function elementStylingApply(index: number, directive?: {}): void { return hackImplementationOfElementStylingApply(index, directive); // supported in next PR } const lView = getLView(); - const isFirstRender = (lView[FLAGS] & LViewFlags.CreationMode) !== 0; + const isFirstRender = (lView[FLAGS] & LViewFlags.FirstLViewPass) !== 0; const totalPlayersQueued = renderStyleAndClassBindings( getStylingContext(index + HEADER_OFFSET, lView), lView[RENDERER], lView, isFirstRender); if (totalPlayersQueued > 0) { @@ -1992,11 +1984,9 @@ export function containerRefreshStart(index: number): void { lView[index + HEADER_OFFSET][ACTIVE_INDEX] = 0; - if (!getCheckNoChangesMode()) { - // We need to execute init hooks here so ngOnInit hooks are called in top level views - // before they are called in embedded views (for backwards compatibility). - executeInitHooks(lView, tView, getCreationMode()); - } + // We need to execute init hooks here so ngOnInit hooks are called in top level views + // before they are called in embedded views (for backwards compatibility). + executeInitHooks(lView, tView, getCheckNoChangesMode()); } /** @@ -2041,9 +2031,7 @@ function refreshDynamicEmbeddedViews(lView: LView) { const dynamicViewData = container[VIEWS][i]; // The directives and pipes are not needed here as an existing view is only being refreshed. ngDevMode && assertDefined(dynamicViewData[TVIEW], 'TView must be allocated'); - renderEmbeddedTemplate( - dynamicViewData, dynamicViewData[TVIEW], dynamicViewData[CONTEXT] !, - RenderFlags.Update); + renderEmbeddedTemplate(dynamicViewData, dynamicViewData[TVIEW], dynamicViewData[CONTEXT] !); } } } @@ -2118,13 +2106,14 @@ export function embeddedViewStart(viewBlockId: number, consts: number, vars: num enterView(viewToRender, viewToRender[TVIEW].node); } if (lContainer) { - if (getCreationMode()) { + if (isCreationMode(viewToRender)) { // it is a new view, insert it into collection of views for a given container insertView(viewToRender, lContainer, lView, lContainer[ACTIVE_INDEX] !, -1); } lContainer[ACTIVE_INDEX] !++; } - return getRenderFlags(viewToRender); + return isCreationMode(viewToRender) ? RenderFlags.Create | RenderFlags.Update : + RenderFlags.Update; } /** @@ -2158,7 +2147,12 @@ function getOrCreateEmbeddedTView( export function embeddedViewEnd(): void { const lView = getLView(); const viewHost = lView[HOST_NODE]; - refreshDescendantViews(lView, null); + + if (isCreationMode(lView)) { + refreshDescendantViews(lView); // creation mode pass + lView[FLAGS] &= ~LViewFlags.CreationMode; + } + refreshDescendantViews(lView); // update mode pass leaveView(lView[PARENT] !); setPreviousOrParentTNode(viewHost !); setIsParent(false); @@ -2170,9 +2164,8 @@ export function embeddedViewEnd(): void { * Refreshes components by entering the component view and processing its bindings, queries, etc. * * @param adjustedElementIndex Element index in LView[] (adjusted for HEADER_OFFSET) - * @param rf The render flags that should be used to process this template */ -export function componentRefresh(adjustedElementIndex: number, rf: RenderFlags | null): void { +export function componentRefresh(adjustedElementIndex: number): void { const lView = getLView(); ngDevMode && assertDataInRange(lView, adjustedElementIndex); const hostView = getComponentViewByIndex(adjustedElementIndex, lView); @@ -2181,7 +2174,7 @@ export function componentRefresh(adjustedElementIndex: number, rf: RenderFlag // Only attached CheckAlways components or attached, dirty OnPush components should be checked if (viewAttached(hostView) && hostView[FLAGS] & (LViewFlags.CheckAlways | LViewFlags.Dirty)) { syncViewWithBlueprint(hostView); - detectChangesInternal(hostView, hostView[CONTEXT], rf); + checkView(hostView, hostView[CONTEXT]); } } @@ -2461,7 +2454,7 @@ export function tick(component: T): void { function tickRootContext(rootContext: RootContext) { for (let i = 0; i < rootContext.components.length; i++) { const rootComponent = rootContext.components[i]; - renderComponentOrTemplate(readPatchedLView(rootComponent) !, rootComponent, RenderFlags.Update); + renderComponentOrTemplate(readPatchedLView(rootComponent) !, rootComponent); } } @@ -2479,7 +2472,21 @@ function tickRootContext(rootContext: RootContext) { * @param component The component which the change detection should be performed on. */ export function detectChanges(component: T): void { - detectChangesInternal(getComponentViewByInstance(component) !, component, null); + const view = getComponentViewByInstance(component) !; + detectChangesInternal(view, component); +} + +export function detectChangesInternal(view: LView, context: T) { + const rendererFactory = view[RENDERER_FACTORY]; + + if (rendererFactory.begin) rendererFactory.begin(); + + if (isCreationMode(view)) { + checkView(view, context); // creation mode pass + } + checkView(view, context); // update mode pass + + if (rendererFactory.end) rendererFactory.end(); } /** @@ -2526,7 +2533,7 @@ export function checkNoChangesInRootView(lView: LView): void { } /** Checks the view of the component provided. Does not gate on dirty checks or execute doCheck. */ -export function detectChangesInternal(hostView: LView, component: T, rf: RenderFlags | null) { +export function checkView(hostView: LView, component: T) { const hostTView = hostView[TVIEW]; const oldView = enterView(hostView, hostView[HOST_NODE]); const templateFn = hostTView.template !; @@ -2534,27 +2541,23 @@ export function detectChangesInternal(hostView: LView, component: T, rf: Rend try { namespaceHTML(); - createViewQuery(viewQuery, rf, hostView[FLAGS], component); - templateFn(rf || getRenderFlags(hostView), component); - refreshDescendantViews(hostView, rf); - updateViewQuery(viewQuery, hostView[FLAGS], component); + createViewQuery(viewQuery, hostView, component); + templateFn(getRenderFlags(hostView), component); + refreshDescendantViews(hostView); + updateViewQuery(viewQuery, hostView, component); } finally { - leaveView(oldView, rf === RenderFlags.Create); + leaveView(oldView); } } -function createViewQuery( - viewQuery: ComponentQuery<{}>| null, renderFlags: RenderFlags | null, viewFlags: LViewFlags, - component: T): void { - if (viewQuery && (renderFlags === RenderFlags.Create || - (renderFlags === null && (viewFlags & LViewFlags.CreationMode)))) { +function createViewQuery(viewQuery: ComponentQuery<{}>| null, view: LView, component: T): void { + if (viewQuery && isCreationMode(view)) { viewQuery(RenderFlags.Create, component); } } -function updateViewQuery( - viewQuery: ComponentQuery<{}>| null, flags: LViewFlags, component: T): void { - if (viewQuery && flags & RenderFlags.Update) { +function updateViewQuery(viewQuery: ComponentQuery<{}>| null, view: LView, component: T): void { + if (viewQuery && !isCreationMode(view)) { viewQuery(RenderFlags.Update, component); } } diff --git a/packages/core/src/render3/interfaces/view.ts b/packages/core/src/render3/interfaces/view.ts index 1b28ee54df..996415a284 100644 --- a/packages/core/src/render3/interfaces/view.ts +++ b/packages/core/src/render3/interfaces/view.ts @@ -221,16 +221,25 @@ export const enum LViewFlags { * back into the parent view, `data` will be defined and `creationMode` will be * improperly reported as false. */ - CreationMode = 0b0000001, + CreationMode = 0b000000001, + + /** + * Whether or not this LView instance is on its first processing pass. + * + * An LView instance is considered to be on its "first pass" until it + * has completed one creation mode run and one update mode run. At this + * time, the flag is turned off. + */ + FirstLViewPass = 0b000000010, /** Whether this view has default change detection strategy (checks always) or onPush */ - CheckAlways = 0b0000010, + CheckAlways = 0b000000100, /** Whether or not this view is currently dirty (needing check) */ - Dirty = 0b0000100, + Dirty = 0b000001000, /** Whether or not this view is currently attached to change detection tree. */ - Attached = 0b0001000, + Attached = 0b000010000, /** * Whether or not the init hooks have run. @@ -239,13 +248,13 @@ export const enum LViewFlags { * runs OR the first cR() instruction that runs (so inits are run for the top level view before * any embedded views). */ - RunInit = 0b0010000, + RunInit = 0b000100000, /** Whether or not this view is destroyed. */ - Destroyed = 0b0100000, + Destroyed = 0b001000000, /** Whether or not this view is the root view */ - IsRoot = 0b1000000, + IsRoot = 0b010000000, } /** diff --git a/packages/core/src/render3/pure_function.ts b/packages/core/src/render3/pure_function.ts index 562b6b0d1e..6f78bf7ae7 100644 --- a/packages/core/src/render3/pure_function.ts +++ b/packages/core/src/render3/pure_function.ts @@ -7,7 +7,7 @@ */ import {bindingUpdated, bindingUpdated2, bindingUpdated3, bindingUpdated4, getBinding, updateBinding} from './bindings'; -import {getBindingRoot, getCreationMode, getLView} from './state'; +import {getBindingRoot, getLView, isCreationMode} from './state'; @@ -42,7 +42,7 @@ export function pureFunction0(slotOffset: number, pureFn: () => T, thisArg?: // TODO(kara): use bindingRoot instead of bindingStartIndex when implementing host bindings const bindingIndex = getBindingRoot() + slotOffset; const lView = getLView(); - return getCreationMode() ? + return isCreationMode() ? updateBinding(lView, bindingIndex, thisArg ? pureFn.call(thisArg) : pureFn()) : getBinding(lView, bindingIndex); } diff --git a/packages/core/src/render3/state.ts b/packages/core/src/render3/state.ts index 76b5ab90e3..69185a2340 100644 --- a/packages/core/src/render3/state.ts +++ b/packages/core/src/render3/state.ts @@ -186,14 +186,9 @@ export function getOrCreateCurrentQueries( return currentQueries || (lView[QUERIES] = new QueryType(null, null, null)); } -/** - * This property gets set before entering a template. - */ -let creationMode: boolean; - -export function getCreationMode(): boolean { - // top level variables should not be exported for performance reasons (PERF_NOTES.md) - return creationMode; +/** Checks whether a given view is in creation mode */ +export function isCreationMode(view: LView = lView): boolean { + return (view[FLAGS] & LViewFlags.CreationMode) === LViewFlags.CreationMode; } /** @@ -276,8 +271,6 @@ export function enterView(newView: LView, hostTNode: TElementNode | TViewNode | const oldView = lView; if (newView) { const tView = newView[TVIEW]; - - creationMode = (newView[FLAGS] & LViewFlags.CreationMode) === LViewFlags.CreationMode; firstTemplatePass = tView.firstTemplatePass; bindingRootIndex = tView.bindingStartIndex; } @@ -320,19 +313,17 @@ export function resetComponentState() { * the direction of traversal (up or down the view tree) a bit clearer. * * @param newView New state to become active - * @param creationOnly An optional boolean to indicate that the view was processed in creation mode - * only, i.e. the first update will be done later. Only possible for dynamically created views. */ -export function leaveView(newView: LView, creationOnly?: boolean): void { +export function leaveView(newView: LView): void { const tView = lView[TVIEW]; - if (!creationOnly) { - if (!checkNoChangesMode) { - executeHooks(lView, tView.viewHooks, tView.viewCheckHooks, creationMode); - } + if (isCreationMode(lView)) { + lView[FLAGS] &= ~LViewFlags.CreationMode; + } else { + executeHooks(lView, tView.viewHooks, tView.viewCheckHooks, checkNoChangesMode); // Views are clean and in update mode after being checked, so these bits are cleared - lView[FLAGS] &= ~(LViewFlags.CreationMode | LViewFlags.Dirty); + lView[FLAGS] &= ~(LViewFlags.Dirty | LViewFlags.FirstLViewPass); + lView[FLAGS] |= LViewFlags.RunInit; + lView[BINDING_INDEX] = tView.bindingStartIndex; } - lView[FLAGS] |= LViewFlags.RunInit; - lView[BINDING_INDEX] = tView.bindingStartIndex; enterView(newView, null); } diff --git a/packages/core/src/render3/view_engine_compatibility.ts b/packages/core/src/render3/view_engine_compatibility.ts index 437abd47f3..d38319b354 100644 --- a/packages/core/src/render3/view_engine_compatibility.ts +++ b/packages/core/src/render3/view_engine_compatibility.ts @@ -112,7 +112,7 @@ export function createTemplateRef( if (container) { insertView(lView, container, hostView !, index !, hostTNode !.index); } - renderEmbeddedTemplate(lView, this._tView, context, RenderFlags.Create); + renderEmbeddedTemplate(lView, this._tView, context); const viewRef = new ViewRef(lView, context, -1); viewRef._tViewNode = lView[HOST_NODE] as TViewNode; return viewRef; diff --git a/packages/core/src/render3/view_ref.ts b/packages/core/src/render3/view_ref.ts index b42a754297..b88ec8cb94 100644 --- a/packages/core/src/render3/view_ref.ts +++ b/packages/core/src/render3/view_ref.ts @@ -11,7 +11,7 @@ import {ChangeDetectorRef as viewEngine_ChangeDetectorRef} from '../change_detec import {ViewContainerRef as viewEngine_ViewContainerRef} from '../linker/view_container_ref'; import {EmbeddedViewRef as viewEngine_EmbeddedViewRef, InternalViewRef as viewEngine_InternalViewRef} from '../linker/view_ref'; -import {checkNoChanges, checkNoChangesInRootView, detectChangesInRootView, detectChangesInternal, markViewDirty, storeCleanupFn, viewAttached} from './instructions'; +import {checkNoChanges, checkNoChangesInRootView, checkView, detectChangesInRootView, detectChangesInternal, markViewDirty, storeCleanupFn, viewAttached} from './instructions'; import {TNode, TNodeType, TViewNode} from './interfaces/node'; import {FLAGS, HOST, HOST_NODE, LView, LViewFlags, PARENT, RENDERER_FACTORY} from './interfaces/view'; import {destroyLView} from './node_manipulation'; @@ -244,16 +244,7 @@ export class ViewRef implements viewEngine_EmbeddedViewRef, viewEngine_Int * * See {@link ChangeDetectorRef#detach detach} for more information. */ - detectChanges(): void { - const rendererFactory = this._lView[RENDERER_FACTORY]; - if (rendererFactory.begin) { - rendererFactory.begin(); - } - detectChangesInternal(this._lView, this.context, null); - if (rendererFactory.end) { - rendererFactory.end(); - } - } + detectChanges(): void { detectChangesInternal(this._lView, this.context); } /** * Checks the change detector and its children, and throws if any changes are detected. diff --git a/packages/core/test/animation/animation_integration_spec.ts b/packages/core/test/animation/animation_integration_spec.ts index fd1a961a46..7c0734fb82 100644 --- a/packages/core/test/animation/animation_integration_spec.ts +++ b/packages/core/test/animation/animation_integration_spec.ts @@ -13,7 +13,7 @@ import {TestBed, fakeAsync, flushMicrotasks} from '@angular/core/testing'; import {ɵDomRendererFactory2} from '@angular/platform-browser'; import {ANIMATION_MODULE_TYPE, BrowserAnimationsModule, NoopAnimationsModule} from '@angular/platform-browser/animations'; import {getDOM} from '@angular/platform-browser/src/dom/dom_adapter'; -import {fixmeIvy} from '@angular/private/testing'; +import {fixmeIvy, ivyEnabled} from '@angular/private/testing'; const DEFAULT_NAMESPACE_ID = 'id'; const DEFAULT_COMPONENT_ID = '1'; @@ -3110,6 +3110,10 @@ const DEFAULT_COMPONENT_ID = '1'; expect(element.style['height']).toEqual(height); } + // In Ivy, change detection needs to run before the ViewQuery for cmp.element will + // resolve. Keeping this test enabled since we still want to test the animation logic. + if (ivyEnabled) fixture.detectChanges(); + const cmp = fixture.componentInstance; const element = cmp.element.nativeElement; fixture.detectChanges(); diff --git a/packages/core/test/animation/animation_query_integration_spec.ts b/packages/core/test/animation/animation_query_integration_spec.ts index a350739d13..f30cf1d597 100644 --- a/packages/core/test/animation/animation_query_integration_spec.ts +++ b/packages/core/test/animation/animation_query_integration_spec.ts @@ -14,7 +14,7 @@ import {CommonModule} from '@angular/common'; import {Component, HostBinding, ViewChild} from '@angular/core'; import {TestBed, fakeAsync, flushMicrotasks} from '@angular/core/testing'; import {BrowserAnimationsModule} from '@angular/platform-browser/animations'; -import {fixmeIvy} from '@angular/private/testing'; +import {fixmeIvy, ivyEnabled} from '@angular/private/testing'; import {HostListener} from '../../src/metadata/directives'; @@ -1706,6 +1706,11 @@ import {HostListener} from '../../src/metadata/directives'; TestBed.configureTestingModule({declarations: [ParentCmp, ChildCmp]}); const fixture = TestBed.createComponent(ParentCmp); const cmp = fixture.componentInstance; + + // In Ivy, change detection needs to run before the ViewQuery for cmp.child will resolve. + // Keeping this test enabled since we still want to test the animation logic in Ivy. + if (ivyEnabled) fixture.detectChanges(); + cmp.child.items = [4, 5, 6]; fixture.detectChanges(); diff --git a/packages/core/test/animation/animations_with_css_keyframes_animations_integration_spec.ts b/packages/core/test/animation/animations_with_css_keyframes_animations_integration_spec.ts index 22c1c498c5..b502ae5af2 100644 --- a/packages/core/test/animation/animations_with_css_keyframes_animations_integration_spec.ts +++ b/packages/core/test/animation/animations_with_css_keyframes_animations_integration_spec.ts @@ -11,6 +11,7 @@ import {AnimationGroupPlayer} from '@angular/animations/src/players/animation_gr import {Component, ViewChild} from '@angular/core'; import {BrowserAnimationsModule} from '@angular/platform-browser/animations'; import {browserDetection} from '@angular/platform-browser/testing/src/browser_util'; +import {ivyEnabled} from '@angular/private/testing'; import {TestBed} from '../../testing'; @@ -290,6 +291,10 @@ import {TestBed} from '../../testing'; const fixture = TestBed.createComponent(Cmp); const cmp = fixture.componentInstance; + // In Ivy, change detection needs to run before the ViewQuery for cmp.element will resolve. + // Keeping this test enabled since we still want to test the animation logic in Ivy. + if (ivyEnabled) fixture.detectChanges(); + const elm = cmp.element.nativeElement; const foo = elm.querySelector('.foo') as HTMLElement; diff --git a/packages/core/test/application_ref_spec.ts b/packages/core/test/application_ref_spec.ts index 0b194e7fea..5a5f9e38c8 100644 --- a/packages/core/test/application_ref_spec.ts +++ b/packages/core/test/application_ref_spec.ts @@ -15,7 +15,7 @@ import {getDOM} from '@angular/platform-browser/src/dom/dom_adapter'; import {DOCUMENT} from '@angular/platform-browser/src/dom/dom_tokens'; import {dispatchEvent} from '@angular/platform-browser/testing/src/browser_util'; import {expect} from '@angular/platform-browser/testing/src/matchers'; -import {fixmeIvy} from '@angular/private/testing'; +import {fixmeIvy, ivyEnabled, modifiedInIvy} from '@angular/private/testing'; import {NoopNgZone} from '../src/zone/ng_zone'; import {ComponentFixtureNoNgZone, TestBed, async, inject, withModule} from '../testing'; @@ -421,6 +421,11 @@ class SomeComponent { it('should detach attached embedded views if they are destroyed', () => { const comp = TestBed.createComponent(EmbeddedViewComp); const appRef: ApplicationRef = TestBed.get(ApplicationRef); + + // In Ivy, change detection needs to run before the ViewQuery for tplRef will resolve. + // Keeping this test enabled since we still want to test this destroy logic in Ivy. + if (ivyEnabled) comp.detectChanges(); + const embeddedViewRef = comp.componentInstance.tplRef.createEmbeddedView({}); appRef.attachView(embeddedViewRef); 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 2cb59a4e45..c63e3016e5 100644 --- a/packages/core/test/bundling/hello_world/bundle.golden_symbols.json +++ b/packages/core/test/bundling/hello_world/bundle.golden_symbols.json @@ -170,6 +170,9 @@ { "name": "checkNoChangesMode" }, + { + "name": "checkView" + }, { "name": "componentRefresh" }, @@ -209,9 +212,6 @@ { "name": "defineComponent" }, - { - "name": "detectChangesInternal" - }, { "name": "diPublicInInjector" }, @@ -254,9 +254,6 @@ { "name": "getContainerRenderParent" }, - { - "name": "getCreationMode" - }, { "name": "getDirectiveDef" }, @@ -353,6 +350,9 @@ { "name": "isComponentDef" }, + { + "name": "isCreationMode" + }, { "name": "isFactory" }, diff --git a/packages/core/test/bundling/todo/bundle.golden_symbols.json b/packages/core/test/bundling/todo/bundle.golden_symbols.json index edefdd0c71..4a0a88740c 100644 --- a/packages/core/test/bundling/todo/bundle.golden_symbols.json +++ b/packages/core/test/bundling/todo/bundle.golden_symbols.json @@ -437,6 +437,9 @@ { "name": "checkNoChangesMode" }, + { + "name": "checkView" + }, { "name": "cleanUpView" }, @@ -659,9 +662,6 @@ { "name": "getContextLView" }, - { - "name": "getCreationMode" - }, { "name": "getCurrentView" }, @@ -896,6 +896,9 @@ { "name": "isContextDirty" }, + { + "name": "isCreationMode" + }, { "name": "isCssClassMatching" }, diff --git a/packages/core/test/linker/query_integration_spec.ts b/packages/core/test/linker/query_integration_spec.ts index 6e16295415..41aa635ccb 100644 --- a/packages/core/test/linker/query_integration_spec.ts +++ b/packages/core/test/linker/query_integration_spec.ts @@ -112,23 +112,21 @@ describe('Query API', () => { expect(directive.child.text).toEqual('foo'); }); - fixmeIvy('FW-782 - View queries are executed twice in some cases') - .it('should contain the first view child', () => { - const template = ''; - const view = createTestCmpAndDetectChanges(MyComp0, template); + it('should contain the first view child', () => { + const template = ''; + const view = createTestCmpAndDetectChanges(MyComp0, template); - const q: NeedsViewChild = view.debugElement.children[0].references !['q']; - expect(q.logs).toEqual([['setter', 'foo'], ['init', 'foo'], ['check', 'foo']]); + const q: NeedsViewChild = view.debugElement.children[0].references !['q']; + expect(q.logs).toEqual([['setter', 'foo'], ['init', 'foo'], ['check', 'foo']]); - q.shouldShow = false; - view.detectChanges(); - expect(q.logs).toEqual([ - ['setter', 'foo'], ['init', 'foo'], ['check', 'foo'], ['setter', null], - ['check', null] - ]); - }); + q.shouldShow = false; + view.detectChanges(); + expect(q.logs).toEqual([ + ['setter', 'foo'], ['init', 'foo'], ['check', 'foo'], ['setter', null], ['check', null] + ]); + }); - fixmeIvy('FW-782 - View queries are executed twice in some cases') + modifiedInIvy('Static ViewChild and ContentChild queries are resolved in update mode') .it('should set static view and content children already after the constructor call', () => { const template = '
'; @@ -142,34 +140,33 @@ describe('Query API', () => { expect(q.viewChild.text).toEqual('viewFoo'); }); - fixmeIvy('FW-782 - View queries are executed twice in some cases') - .it('should contain the first view child across embedded views', () => { - TestBed.overrideComponent( - MyComp0, {set: {template: ''}}); - TestBed.overrideComponent(NeedsViewChild, { - set: { - template: - '
' - } - }); - const view = TestBed.createComponent(MyComp0); + it('should contain the first view child across embedded views', () => { + TestBed.overrideComponent( + MyComp0, {set: {template: ''}}); + TestBed.overrideComponent(NeedsViewChild, { + set: { + template: + '
' + } + }); + const view = TestBed.createComponent(MyComp0); - view.detectChanges(); - const q: NeedsViewChild = view.debugElement.children[0].references !['q']; - expect(q.logs).toEqual([['setter', 'foo'], ['init', 'foo'], ['check', 'foo']]); + view.detectChanges(); + const q: NeedsViewChild = view.debugElement.children[0].references !['q']; + expect(q.logs).toEqual([['setter', 'foo'], ['init', 'foo'], ['check', 'foo']]); - q.shouldShow = false; - q.shouldShow2 = true; - q.logs = []; - view.detectChanges(); - expect(q.logs).toEqual([['setter', 'bar'], ['check', 'bar']]); + q.shouldShow = false; + q.shouldShow2 = true; + q.logs = []; + view.detectChanges(); + expect(q.logs).toEqual([['setter', 'bar'], ['check', 'bar']]); - q.shouldShow = false; - q.shouldShow2 = false; - q.logs = []; - view.detectChanges(); - expect(q.logs).toEqual([['setter', null], ['check', null]]); - }); + q.shouldShow = false; + q.shouldShow2 = false; + q.logs = []; + view.detectChanges(); + expect(q.logs).toEqual([['setter', null], ['check', null]]); + }); fixmeIvy( 'FW-781 - Directives invocation sequence on root and nested elements is different in Ivy') @@ -589,31 +586,35 @@ describe('Query API', () => { }); // Note: this test is just document our current behavior, which we do for performance reasons. - fixmeIvy('FW-782 - View queries are executed twice in some cases') - .it('should not affected queries for projected templates if views are detached or moved', () => { - const template = - '
'; - const view = createTestCmpAndDetectChanges(MyComp0, template); - const q = view.debugElement.children[0].references !['q'] as ManualProjecting; - expect(q.query.length).toBe(0); + fixmeIvy('FW-853: Query results are cleared if embedded views are detached / moved') + .it('should not affect queries for projected templates if views are detached or moved', + () => { + const template = ` + +
+ + `; + const view = createTestCmpAndDetectChanges(MyComp0, template); + const q = view.debugElement.children[0].references !['q'] as ManualProjecting; + expect(q.query.length).toBe(0); - const view1 = q.vc.createEmbeddedView(q.template, {'x': '1'}); - const view2 = q.vc.createEmbeddedView(q.template, {'x': '2'}); - view.detectChanges(); - expect(q.query.map((d: TextDirective) => d.text)).toEqual(['1', '2']); + const view1 = q.vc.createEmbeddedView(q.template, {'x': '1'}); + const view2 = q.vc.createEmbeddedView(q.template, {'x': '2'}); + view.detectChanges(); + expect(q.query.map((d: TextDirective) => d.text)).toEqual(['1', '2']); - q.vc.detach(1); - q.vc.detach(0); + q.vc.detach(1); + q.vc.detach(0); - view.detectChanges(); - expect(q.query.map((d: TextDirective) => d.text)).toEqual(['1', '2']); + view.detectChanges(); + expect(q.query.map((d: TextDirective) => d.text)).toEqual(['1', '2']); - q.vc.insert(view2); - q.vc.insert(view1); + q.vc.insert(view2); + q.vc.insert(view1); - view.detectChanges(); - expect(q.query.map((d: TextDirective) => d.text)).toEqual(['1', '2']); - }); + view.detectChanges(); + expect(q.query.map((d: TextDirective) => d.text)).toEqual(['1', '2']); + }); fixmeIvy('unknown').it( 'should remove manually projected templates if their parent view is destroyed', () => { diff --git a/packages/core/test/render3/change_detection_spec.ts b/packages/core/test/render3/change_detection_spec.ts index 0d924733b3..c5fb5dd039 100644 --- a/packages/core/test/render3/change_detection_spec.ts +++ b/packages/core/test/render3/change_detection_spec.ts @@ -87,73 +87,6 @@ describe('change detection', () => { await whenRendered(myComp); expect(getRenderedText(myComp)).toEqual('updated'); })); - - it('should support detectChanges on components that have LContainers', () => { - let structuralComp !: StructuralComp; - - function FooTemplate(rf: RenderFlags, ctx: any) { - if (rf & RenderFlags.Create) { - text(0); - } - if (rf & RenderFlags.Update) { - textBinding(0, bind(ctx.value)); - } - } - - class StructuralComp { - tmp !: TemplateRef; - value = 'one'; - - constructor(public vcr: ViewContainerRef) {} - - create() { return this.vcr.createEmbeddedView(this.tmp, this); } - - static ngComponentDef = defineComponent({ - type: StructuralComp, - selectors: [['structural-comp']], - factory: () => structuralComp = - new StructuralComp(directiveInject(ViewContainerRef as any)), - inputs: {tmp: 'tmp'}, - consts: 1, - vars: 1, - template: FooTemplate - }); - } - - /** - * {{ value }} - * - */ - const App = createComponent('app', function(rf: RenderFlags, ctx: any) { - if (rf & RenderFlags.Create) { - template(0, FooTemplate, 2, 1, 'ng-template', null, ['foo', ''], templateRefExtractor); - element(2, 'structural-comp'); - } - if (rf & RenderFlags.Update) { - const foo = reference(1) as any; - elementProperty(2, 'tmp', bind(foo)); - } - }, 3, 1, [StructuralComp]); - - const fixture = new ComponentFixture(App); - fixture.update(); - expect(fixture.html).toEqual('one'); - - const viewRef: EmbeddedViewRef = structuralComp.create(); - fixture.update(); - expect(fixture.html).toEqual('oneone'); - - // check embedded view update - structuralComp.value = 'two'; - viewRef.detectChanges(); - expect(fixture.html).toEqual('onetwo'); - - // check root view update - structuralComp.value = 'three'; - fixture.update(); - expect(fixture.html).toEqual('threethree'); - }); - }); describe('onPush', () => { @@ -662,6 +595,117 @@ describe('change detection', () => { expect(getRenderedText(comp)).toEqual('1'); }); + describe('dynamic views', () => { + let structuralComp: StructuralComp|null = null; + + beforeEach(() => structuralComp = null); + + class StructuralComp { + tmp !: TemplateRef; + value = 'one'; + + constructor(public vcr: ViewContainerRef) {} + + create() { return this.vcr.createEmbeddedView(this.tmp, this); } + + static ngComponentDef = defineComponent({ + type: StructuralComp, + selectors: [['structural-comp']], + factory: () => structuralComp = + new StructuralComp(directiveInject(ViewContainerRef as any)), + inputs: {tmp: 'tmp'}, + consts: 1, + vars: 1, + template: function(rf: RenderFlags, ctx: any) { + if (rf & RenderFlags.Create) { + text(0); + } + if (rf & RenderFlags.Update) { + textBinding(0, bind(ctx.value)); + } + } + }); + } + + it('should support ViewRef.detectChanges()', () => { + function FooTemplate(rf: RenderFlags, ctx: any) { + if (rf & RenderFlags.Create) { + text(0); + } + if (rf & RenderFlags.Update) { + textBinding(0, bind(ctx.value)); + } + } + + /** + * {{ value }} + * + */ + const App = createComponent('app', function(rf: RenderFlags, ctx: any) { + if (rf & RenderFlags.Create) { + template( + 0, FooTemplate, 1, 1, 'ng-template', null, ['foo', ''], templateRefExtractor); + element(2, 'structural-comp'); + } + if (rf & RenderFlags.Update) { + const foo = reference(1) as any; + elementProperty(2, 'tmp', bind(foo)); + } + }, 3, 1, [StructuralComp]); + + const fixture = new ComponentFixture(App); + fixture.update(); + expect(fixture.html).toEqual('one'); + + const viewRef: EmbeddedViewRef = structuralComp !.create(); + fixture.update(); + expect(fixture.html).toEqual('oneone'); + + // check embedded view update + structuralComp !.value = 'two'; + viewRef.detectChanges(); + expect(fixture.html).toEqual('onetwo'); + + // check root view update + structuralComp !.value = 'three'; + fixture.update(); + expect(fixture.html).toEqual('threethree'); + }); + + it('should support ViewRef.detectChanges() directly after creation', () => { + function FooTemplate(rf: RenderFlags, ctx: any) { + if (rf & RenderFlags.Create) { + text(0, 'Template text'); + } + } + + /** + * Template text + * + */ + const App = createComponent('app', function(rf: RenderFlags, ctx: any) { + if (rf & RenderFlags.Create) { + template( + 0, FooTemplate, 1, 0, 'ng-template', null, ['foo', ''], templateRefExtractor); + element(2, 'structural-comp'); + } + if (rf & RenderFlags.Update) { + const foo = reference(1) as any; + elementProperty(2, 'tmp', bind(foo)); + } + }, 3, 1, [StructuralComp]); + + const fixture = new ComponentFixture(App); + fixture.update(); + expect(fixture.html).toEqual('one'); + + const viewRef: EmbeddedViewRef = structuralComp !.create(); + viewRef.detectChanges(); + expect(fixture.html).toEqual('oneTemplate text'); + }); + + }); + }); describe('attach/detach', () => { diff --git a/packages/core/test/render3/component_spec.ts b/packages/core/test/render3/component_spec.ts index 817e2594ef..743590a276 100644 --- a/packages/core/test/render3/component_spec.ts +++ b/packages/core/test/render3/component_spec.ts @@ -498,6 +498,8 @@ describe('recursive components', () => { class NgIfTree { data: TreeNode = _buildTree(0); + ngDoCheck() { events.push('check' + this.data.value); } + ngOnDestroy() { events.push('destroy' + this.data.value); } static ngComponentDef = defineComponent({ @@ -628,6 +630,7 @@ describe('recursive components', () => { const fixture = new ComponentFixture(App); expect(getRenderedText(fixture.component)).toEqual('6201534'); + expect(events).toEqual(['check6', 'check2', 'check0', 'check1', 'check5', 'check3', 'check4']); events = []; fixture.component.skipContent = true; diff --git a/packages/core/test/render3/control_flow_spec.ts b/packages/core/test/render3/control_flow_spec.ts index e333402a20..e7129336da 100644 --- a/packages/core/test/render3/control_flow_spec.ts +++ b/packages/core/test/render3/control_flow_spec.ts @@ -38,8 +38,8 @@ describe('JS control flow', () => { embeddedViewEnd(); } } + containerRefreshEnd(); } - containerRefreshEnd(); }, 2); const fixture = new ComponentFixture(App); diff --git a/packages/core/test/render3/integration_spec.ts b/packages/core/test/render3/integration_spec.ts index 89468d775f..3c981a96c4 100644 --- a/packages/core/test/render3/integration_spec.ts +++ b/packages/core/test/render3/integration_spec.ts @@ -115,9 +115,7 @@ describe('render3 integration test', () => { function Template(rf: RenderFlags, value: string) { if (rf & RenderFlags.Create) { text(0); - } - if (rf & RenderFlags.Update) { - textBinding(0, rf & RenderFlags.Create ? value : NO_CHANGE); + textBinding(0, value); } } expect(renderToHtml(Template, 'once', 1, 1)).toEqual('once'); diff --git a/packages/core/test/render3/properties_spec.ts b/packages/core/test/render3/properties_spec.ts index 40851f0046..8a13ec371a 100644 --- a/packages/core/test/render3/properties_spec.ts +++ b/packages/core/test/render3/properties_spec.ts @@ -49,9 +49,7 @@ describe('elementProperty', () => { function Template(rf: RenderFlags, ctx: string) { if (rf & RenderFlags.Create) { element(0, 'span'); - } - if (rf & RenderFlags.Update) { - elementProperty(0, 'id', rf & RenderFlags.Create ? expensive(ctx) : NO_CHANGE); + elementProperty(0, 'id', expensive(ctx)); } } diff --git a/packages/core/test/render3/renderer_factory_spec.ts b/packages/core/test/render3/renderer_factory_spec.ts index 3e1459c1a5..3fdd05170e 100644 --- a/packages/core/test/render3/renderer_factory_spec.ts +++ b/packages/core/test/render3/renderer_factory_spec.ts @@ -37,10 +37,13 @@ describe('renderer factory lifecycle', () => { consts: 1, vars: 0, template: function(rf: RenderFlags, ctx: SomeComponent) { - logs.push('component'); if (rf & RenderFlags.Create) { + logs.push('component create'); text(0, 'foo'); } + if (rf & RenderFlags.Update) { + logs.push('component update'); + } }, factory: () => new SomeComponent }); @@ -61,31 +64,38 @@ describe('renderer factory lifecycle', () => { } function Template(rf: RenderFlags, ctx: any) { - logs.push('function'); if (rf & RenderFlags.Create) { + logs.push('function create'); text(0, 'bar'); } + if (rf & RenderFlags.Update) { + logs.push('function update'); + } } const directives = [SomeComponent, SomeComponentWhichThrows]; function TemplateWithComponent(rf: RenderFlags, ctx: any) { - logs.push('function_with_component'); if (rf & RenderFlags.Create) { + logs.push('function_with_component create'); text(0, 'bar'); element(1, 'some-component'); } + if (rf & RenderFlags.Update) { + logs.push('function_with_component update'); + } } beforeEach(() => { logs = []; }); it('should work with a component', () => { const component = renderComponent(SomeComponent, {rendererFactory}); - expect(logs).toEqual(['create', 'create', 'begin', 'component', 'end']); + expect(logs).toEqual( + ['create', 'create', 'begin', 'component create', 'component update', 'end']); logs = []; tick(component); - expect(logs).toEqual(['begin', 'component', 'end']); + expect(logs).toEqual(['begin', 'component update', 'end']); }); it('should work with a component which throws', () => { @@ -95,21 +105,23 @@ describe('renderer factory lifecycle', () => { it('should work with a template', () => { renderToHtml(Template, {}, 1, 0, null, null, rendererFactory); - expect(logs).toEqual(['create', 'begin', 'function', 'end']); + expect(logs).toEqual(['create', 'begin', 'function create', 'function update', 'end']); logs = []; renderToHtml(Template, {}); - expect(logs).toEqual(['begin', 'function', 'end']); + expect(logs).toEqual(['begin', 'function update', 'end']); }); it('should work with a template which contains a component', () => { renderToHtml(TemplateWithComponent, {}, 2, 0, directives, null, rendererFactory); - expect(logs).toEqual( - ['create', 'begin', 'function_with_component', 'create', 'component', 'end']); + expect(logs).toEqual([ + 'create', 'begin', 'function_with_component create', 'create', 'component create', + 'function_with_component update', 'component update', 'end' + ]); logs = []; renderToHtml(TemplateWithComponent, {}, 2, 0, directives); - expect(logs).toEqual(['begin', 'function_with_component', 'component', 'end']); + expect(logs).toEqual(['begin', 'function_with_component update', 'component update', 'end']); }); });