diff --git a/goldens/size-tracking/aio-payloads.json b/goldens/size-tracking/aio-payloads.json index e10ce7bf43..68ba71ad60 100755 --- a/goldens/size-tracking/aio-payloads.json +++ b/goldens/size-tracking/aio-payloads.json @@ -12,7 +12,7 @@ "master": { "uncompressed": { "runtime-es2015": 2987, - "main-es2015": 452060, + "main-es2015": 452876, "polyfills-es2015": 52195 } } diff --git a/goldens/size-tracking/integration-payloads.json b/goldens/size-tracking/integration-payloads.json index 18be2ed2aa..e62b88baee 100644 --- a/goldens/size-tracking/integration-payloads.json +++ b/goldens/size-tracking/integration-payloads.json @@ -3,7 +3,7 @@ "master": { "uncompressed": { "runtime-es2015": 1485, - "main-es2015": 142073, + "main-es2015": 142794, "polyfills-es2015": 36657 } } @@ -12,7 +12,7 @@ "master": { "uncompressed": { "runtime-es2015": 1485, - "main-es2015": 16514, + "main-es2015": 16959, "polyfills-es2015": 36657 } } @@ -21,7 +21,7 @@ "master": { "uncompressed": { "runtime-es2015": 1485, - "main-es2015": 148196, + "main-es2015": 148932, "polyfills-es2015": 36657 } } @@ -30,7 +30,7 @@ "master": { "uncompressed": { "runtime-es2015": 1485, - "main-es2015": 137897, + "main-es2015": 138343, "polyfills-es2015": 37334 } } @@ -39,7 +39,7 @@ "master": { "uncompressed": { "runtime-es2015": 2289, - "main-es2015": 247727, + "main-es2015": 248671, "polyfills-es2015": 36657, "5-es2015": 751 } @@ -49,7 +49,7 @@ "master": { "uncompressed": { "runtime-es2015": 2289, - "main-es2015": 227258, + "main-es2015": 227677, "polyfills-es2015": 36657, "5-es2015": 779 } diff --git a/packages/core/src/render3/instructions/shared.ts b/packages/core/src/render3/instructions/shared.ts index d5b18c87c8..3796cf1fa6 100644 --- a/packages/core/src/render3/instructions/shared.ts +++ b/packages/core/src/render3/instructions/shared.ts @@ -28,15 +28,15 @@ import {AttributeMarker, InitialInputData, InitialInputs, LocalRefExtractor, Pro import {isProceduralRenderer, RComment, RElement, Renderer3, RendererFactory3, RNode, RText} from '../interfaces/renderer'; import {SanitizerFn} from '../interfaces/sanitization'; import {isComponentDef, isComponentHost, isContentQueryHost, isLContainer, isRootView} from '../interfaces/type_checks'; -import {CHILD_HEAD, CHILD_TAIL, CLEANUP, CONTEXT, DECLARATION_COMPONENT_VIEW, DECLARATION_VIEW, FLAGS, HEADER_OFFSET, HOST, InitPhaseState, INJECTOR, LView, LViewFlags, NEXT, PARENT, RENDERER, RENDERER_FACTORY, RootContext, RootContextFlags, SANITIZER, T_HOST, TData, TVIEW, TView, TViewType} from '../interfaces/view'; +import {CHILD_HEAD, CHILD_TAIL, CLEANUP, CONTEXT, DECLARATION_COMPONENT_VIEW, DECLARATION_VIEW, FLAGS, HEADER_OFFSET, HOST, InitPhaseState, INJECTOR, LView, LViewFlags, NEXT, PARENT, RENDERER, RENDERER_FACTORY, RootContext, RootContextFlags, SANITIZER, T_HOST, TData, TRANSPLANTED_VIEWS_TO_REFRESH, TVIEW, TView, TViewType} from '../interfaces/view'; import {assertNodeOfPossibleTypes} from '../node_assert'; import {isInlineTemplate, isNodeMatchingSelectorList} from '../node_selector_matcher'; -import {enterView, getBindingsEnabled, getCheckNoChangesMode, getCurrentDirectiveIndex, getIsParent, getPreviousOrParentTNode, getSelectedIndex, getTView, leaveView, setBindingIndex, setBindingRootForHostBindings, setCheckNoChangesMode, setCurrentDirectiveIndex, setCurrentQueryIndex, setPreviousOrParentTNode, setSelectedIndex} from '../state'; +import {enterView, getBindingsEnabled, getCheckNoChangesMode, getCurrentDirectiveIndex, getIsParent, getPreviousOrParentTNode, getSelectedIndex, leaveView, setBindingIndex, setBindingRootForHostBindings, setCheckNoChangesMode, setCurrentDirectiveIndex, setCurrentQueryIndex, setPreviousOrParentTNode, setSelectedIndex} from '../state'; import {NO_CHANGE} from '../tokens'; import {isAnimationProp, mergeHostAttrs} from '../util/attrs_utils'; import {INTERPOLATION_DELIMITER, renderStringify, stringifyForError} from '../util/misc_utils'; import {getLViewParent} from '../util/view_traversal_utils'; -import {getComponentLViewByIndex, getNativeByIndex, getNativeByTNode, getTNode, isCreationMode, readPatchedLView, resetPreOrderHookFlags, unwrapLView, viewAttachedToChangeDetector} from '../util/view_utils'; +import {getComponentLViewByIndex, getNativeByIndex, getNativeByTNode, isCreationMode, readPatchedLView, 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'; @@ -431,6 +431,10 @@ export function refreshView( } } + // First mark transplanted views that are declared in this lView as needing a refresh at their + // insertion points. This is needed to avoid the situation where the template is defined in this + // `LView` but its declaration appears after the insertion component. + markTransplantedViewsForRefresh(lView); refreshDynamicEmbeddedViews(lView); // Content query results must be refreshed before content hooks are called. @@ -507,6 +511,10 @@ export function refreshView( if (!checkNoChangesMode) { lView[FLAGS] &= ~(LViewFlags.Dirty | LViewFlags.FirstLViewPass); } + if (lView[FLAGS] & LViewFlags.RefreshTransplantedView) { + lView[FLAGS] &= ~LViewFlags.RefreshTransplantedView; + updateTransplantedViewCount(lView[PARENT] as LContainer, -1); + } } finally { leaveView(); } @@ -1600,85 +1608,94 @@ export function createLContainer( ActiveIndexFlag.DYNAMIC_EMBEDDED_VIEWS_ONLY << ActiveIndexFlag.SHIFT, // active index currentView, // parent null, // next - null, // queries - tNode, // t_host - native, // native, - null, // view refs + 0, // transplanted views to refresh count + tNode, // t_host + native, // native, + null, // view refs + null, // moved views ); + ngDevMode && + assertEqual( + lContainer.length, CONTAINER_HEADER_OFFSET, + 'Should allocate correct number of slots for LContainer header.'); ngDevMode && attachLContainerDebug(lContainer); return lContainer; } - /** * Goes over dynamic embedded views (ones created through ViewContainerRef APIs) and refreshes * them by executing an associated template function. */ function refreshDynamicEmbeddedViews(lView: LView) { - let viewOrContainer = lView[CHILD_HEAD]; - while (viewOrContainer !== null) { - // Note: viewOrContainer can be an LView or an LContainer instance, but here we are only - // interested in LContainer - let activeIndexFlag: ActiveIndexFlag; - if (isLContainer(viewOrContainer) && - (activeIndexFlag = viewOrContainer[ACTIVE_INDEX]) >> ActiveIndexFlag.SHIFT === - ActiveIndexFlag.DYNAMIC_EMBEDDED_VIEWS_ONLY) { - for (let i = CONTAINER_HEADER_OFFSET; i < viewOrContainer.length; i++) { - const embeddedLView = viewOrContainer[i] as LView; - const embeddedTView = embeddedLView[TVIEW]; - ngDevMode && assertDefined(embeddedTView, 'TView must be allocated'); - if (viewAttachedToChangeDetector(embeddedLView)) { - refreshView( - embeddedTView, embeddedLView, embeddedTView.template, embeddedLView[CONTEXT]!); - } - } - if ((activeIndexFlag & ActiveIndexFlag.HAS_TRANSPLANTED_VIEWS) !== 0) { - // We should only CD moved views if the component where they were inserted does not match - // the component where they were declared and insertion is on-push. Moved views also - // contains intra component moves, or check-always which need to be skipped. - refreshTransplantedViews(viewOrContainer, lView[DECLARATION_COMPONENT_VIEW]!); + for (let lContainer = getFirstLContainer(lView); lContainer !== null; + lContainer = getNextLContainer(lContainer)) { + for (let i = CONTAINER_HEADER_OFFSET; i < lContainer.length; i++) { + const embeddedLView = lContainer[i]; + const embeddedTView = embeddedLView[TVIEW]; + ngDevMode && assertDefined(embeddedTView, 'TView must be allocated'); + if (viewAttachedToChangeDetector(embeddedLView)) { + refreshView(embeddedTView, embeddedLView, embeddedTView.template, embeddedLView[CONTEXT]!); } } - viewOrContainer = viewOrContainer[NEXT]; } } +/** + * Gets the first `LContainer` in the LView or `null` if none exists. + */ +function getFirstLContainer(lView: LView): LContainer|null { + let viewOrContainer = lView[CHILD_HEAD]; + while (viewOrContainer !== null && + !(isLContainer(viewOrContainer) && + viewOrContainer[ACTIVE_INDEX] >> ActiveIndexFlag.SHIFT === + ActiveIndexFlag.DYNAMIC_EMBEDDED_VIEWS_ONLY)) { + viewOrContainer = viewOrContainer[NEXT]; + } + return viewOrContainer; +} /** - * Refresh transplanted LViews. + * Gets the next `LContainer` that is a sibling of the given container. + */ +function getNextLContainer(container: LContainer): LContainer|null { + let viewOrContainer = container[NEXT]; + while (viewOrContainer !== null && + !(isLContainer(viewOrContainer) && + viewOrContainer[ACTIVE_INDEX] >> ActiveIndexFlag.SHIFT === + ActiveIndexFlag.DYNAMIC_EMBEDDED_VIEWS_ONLY)) { + viewOrContainer = viewOrContainer[NEXT]; + } + return viewOrContainer; +} + +/** + * Mark transplanted views as needing to be refreshed at their insertion points. * * See: `ActiveIndexFlag.HAS_TRANSPLANTED_VIEWS` and `LView[DECLARATION_COMPONENT_VIEW]` for * explanation of transplanted views. * - * @param lContainer The `LContainer` which has transplanted views. - * @param declaredComponentLView The `lContainer` parent component `LView`. + * @param lView The `LView` that may have transplanted views. */ -function refreshTransplantedViews(lContainer: LContainer, declaredComponentLView: LView) { - const movedViews = lContainer[MOVED_VIEWS]!; - ngDevMode && assertDefined(movedViews, 'Transplanted View flags set but missing MOVED_VIEWS'); - for (let i = 0; i < movedViews.length; i++) { - const movedLView = movedViews[i]!; - const insertionLContainer = movedLView[PARENT] as LContainer; - ngDevMode && assertLContainer(insertionLContainer); - const insertedComponentLView = insertionLContainer[PARENT][DECLARATION_COMPONENT_VIEW]!; - ngDevMode && assertDefined(insertedComponentLView, 'Missing LView'); - // Check if we have a transplanted view by compering declaration and insertion location. - if (insertedComponentLView !== declaredComponentLView) { - // Yes the `LView` is transplanted. - // Here we would like to know if the component is `OnPush`. We don't have - // explicit `OnPush` flag instead we set `CheckAlways` to false (which is `OnPush`) - // Not to be confused with `ManualOnPush` which is used with wether a DOM event - // should automatically mark a view as dirty. - const insertionComponentIsOnPush = - (insertedComponentLView[FLAGS] & LViewFlags.CheckAlways) === 0; - if (insertionComponentIsOnPush) { - // Here we know that the template has been transplanted across components and is - // on-push (not just moved within a component). If the insertion is marked dirty, then - // there is no need to CD here as we will do it again later when we get to insertion - // point. - const movedTView = movedLView[TVIEW]; - ngDevMode && assertDefined(movedTView, 'TView must be allocated'); - refreshView(movedTView, movedLView, movedTView.template, movedLView[CONTEXT]!); +function markTransplantedViewsForRefresh(lView: LView) { + for (let lContainer = getFirstLContainer(lView); lContainer !== null; + lContainer = getNextLContainer(lContainer)) { + if ((lContainer[ACTIVE_INDEX] & ActiveIndexFlag.HAS_TRANSPLANTED_VIEWS) !== 0) { + const movedViews = lContainer[MOVED_VIEWS]!; + ngDevMode && assertDefined(movedViews, 'Transplanted View flags set but missing MOVED_VIEWS'); + for (let i = 0; i < movedViews.length; i++) { + const movedLView = movedViews[i]!; + const insertionLContainer = movedLView[PARENT] as LContainer; + ngDevMode && assertLContainer(insertionLContainer); + // We don't want to increment the counter if the moved LView was already marked for + // refresh. + if ((movedLView[FLAGS] & LViewFlags.RefreshTransplantedView) === 0) { + updateTransplantedViewCount(insertionLContainer, 1); + } + // Note, it is possible that the `movedViews` is tracking views that are transplanted *and* + // those that aren't (declaration component === insertion component). In the latter case, + // it's fine to add the flag, as we will clear it immediately in + // `refreshDynamicEmbeddedViews` for the view currently being refreshed. + movedLView[FLAGS] |= LViewFlags.RefreshTransplantedView; } } } @@ -1695,10 +1712,50 @@ function refreshComponent(hostLView: LView, componentHostIdx: number): void { ngDevMode && assertEqual(isCreationMode(hostLView), false, 'Should be run in update mode'); const componentView = getComponentLViewByIndex(componentHostIdx, hostLView); // Only attached components that are CheckAlways or OnPush and dirty should be refreshed - if (viewAttachedToChangeDetector(componentView) && - componentView[FLAGS] & (LViewFlags.CheckAlways | LViewFlags.Dirty)) { - const componentTView = componentView[TVIEW]; - refreshView(componentTView, componentView, componentTView.template, componentView[CONTEXT]); + if (viewAttachedToChangeDetector(componentView)) { + const tView = componentView[TVIEW]; + if (componentView[FLAGS] & (LViewFlags.CheckAlways | LViewFlags.Dirty)) { + refreshView(tView, componentView, tView.template, componentView[CONTEXT]); + } else if (componentView[TRANSPLANTED_VIEWS_TO_REFRESH] > 0) { + // Only attached components that are CheckAlways or OnPush and dirty should be refreshed + refreshContainsDirtyView(componentView); + } + } +} + +/** + * Refreshes all transplanted views marked with `LViewFlags.RefreshTransplantedView` that are + * children or descendants of the given lView. + * + * @param lView The lView which contains descendant transplanted views that need to be refreshed. + */ +function refreshContainsDirtyView(lView: LView) { + for (let lContainer = getFirstLContainer(lView); lContainer !== null; + lContainer = getNextLContainer(lContainer)) { + for (let i = CONTAINER_HEADER_OFFSET; i < lContainer.length; i++) { + const embeddedLView = lContainer[i]; + if (embeddedLView[FLAGS] & LViewFlags.RefreshTransplantedView) { + const embeddedTView = embeddedLView[TVIEW]; + ngDevMode && assertDefined(embeddedTView, 'TView must be allocated'); + refreshView(embeddedTView, embeddedLView, embeddedTView.template, embeddedLView[CONTEXT]!); + } else if (embeddedLView[TRANSPLANTED_VIEWS_TO_REFRESH] > 0) { + refreshContainsDirtyView(embeddedLView); + } + } + } + + const tView = lView[TVIEW]; + // Refresh child component views. + const components = tView.components; + if (components !== null) { + for (let i = 0; i < components.length; i++) { + const componentView = getComponentLViewByIndex(components[i], lView); + // Only attached components that are CheckAlways or OnPush and dirty should be refreshed + if (viewAttachedToChangeDetector(componentView) && + componentView[TRANSPLANTED_VIEWS_TO_REFRESH] > 0) { + refreshContainsDirtyView(componentView); + } + } } } diff --git a/packages/core/src/render3/interfaces/container.ts b/packages/core/src/render3/interfaces/container.ts index 44e292daf2..b4d1e07369 100644 --- a/packages/core/src/render3/interfaces/container.ts +++ b/packages/core/src/render3/interfaces/container.ts @@ -10,8 +10,8 @@ import {ViewRef} from '../../linker/view_ref'; import {TNode} from './node'; import {RComment, RElement} from './renderer'; +import {HOST, LView, NEXT, PARENT, T_HOST, TRANSPLANTED_VIEWS_TO_REFRESH} from './view'; -import {HOST, LView, NEXT, PARENT, T_HOST} from './view'; /** @@ -27,16 +27,16 @@ export const TYPE = 1; */ export const ACTIVE_INDEX = 2; -// PARENT and NEXT are indices 3 and 4 +// PARENT, NEXT, TRANSPLANTED_VIEWS_TO_REFRESH are indices 3, 4, and 5 // As we already have these constants in LView, we don't need to re-create them. -export const MOVED_VIEWS = 5; - // T_HOST is index 6 // We already have this constants in LView, we don't need to re-create it. export const NATIVE = 7; export const VIEW_REFS = 8; +export const MOVED_VIEWS = 9; + /** * Size of LContainer's header. Represents the index after which all views in the @@ -44,7 +44,7 @@ export const VIEW_REFS = 8; * which views are already in the DOM (and don't need to be re-added) and so we can * remove views from the DOM when they are no longer required. */ -export const CONTAINER_HEADER_OFFSET = 9; +export const CONTAINER_HEADER_OFFSET = 10; /** @@ -132,6 +132,14 @@ export interface LContainer extends Array { */ [NEXT]: LView|LContainer|null; + /** + * The number of direct transplanted views which need a refresh or have descendants themselves + * that need a refresh but have not marked their ancestors as Dirty. This tells us that during + * change detection we should still descend to find those children to refresh, even if the parents + * are not `Dirty`/`CheckAlways`. + */ + [TRANSPLANTED_VIEWS_TO_REFRESH]: number; + /** * A collection of views created based on the underlying `` element but inserted into * a different `LContainer`. We need to track views created from a given declaration point since diff --git a/packages/core/src/render3/interfaces/view.ts b/packages/core/src/render3/interfaces/view.ts index 4fc01b3f2b..0d02144c84 100644 --- a/packages/core/src/render3/interfaces/view.ts +++ b/packages/core/src/render3/interfaces/view.ts @@ -31,7 +31,7 @@ export const TVIEW = 1; export const FLAGS = 2; export const PARENT = 3; export const NEXT = 4; -export const QUERIES = 5; +export const TRANSPLANTED_VIEWS_TO_REFRESH = 5; export const T_HOST = 6; export const CLEANUP = 7; export const CONTEXT = 8; @@ -45,8 +45,9 @@ export const DECLARATION_VIEW = 15; export const DECLARATION_COMPONENT_VIEW = 16; export const DECLARATION_LCONTAINER = 17; export const PREORDER_HOOK_FLAGS = 18; +export const QUERIES = 19; /** Size of LView's header. Necessary to adjust for it when setting slots. */ -export const HEADER_OFFSET = 19; +export const HEADER_OFFSET = 20; // This interface replaces the real LView interface if it is an arg or a @@ -282,6 +283,14 @@ export interface LView extends Array { * More flags for this view. See PreOrderHookFlags for more info. */ [PREORDER_HOOK_FLAGS]: PreOrderHookFlags; + + /** + * The number of direct transplanted views which need a refresh or have descendants themselves + * that need a refresh but have not marked their ancestors as Dirty. This tells us that during + * change detection we should still descend to find those children to refresh, even if the parents + * are not `Dirty`/`CheckAlways`. + */ + [TRANSPLANTED_VIEWS_TO_REFRESH]: number; } /** Flags associated with an LView (saved in LView[FLAGS]) */ @@ -342,11 +351,17 @@ export const enum LViewFlags { IsRoot = 0b001000000000, /** - * Index of the current init phase on last 22 bits + * Whether this moved LView was needs to be refreshed at the insertion location because the + * declaration was dirty. */ - IndexWithinInitPhaseIncrementer = 0b010000000000, - IndexWithinInitPhaseShift = 10, - IndexWithinInitPhaseReset = 0b001111111111, + RefreshTransplantedView = 0b0010000000000, + + /** + * Index of the current init phase on last 21 bits + */ + IndexWithinInitPhaseIncrementer = 0b0100000000000, + IndexWithinInitPhaseShift = 11, + IndexWithinInitPhaseReset = 0b0011111111111, } /** diff --git a/packages/core/src/render3/node_manipulation.ts b/packages/core/src/render3/node_manipulation.ts index ce5315440c..373c20a97c 100644 --- a/packages/core/src/render3/node_manipulation.ts +++ b/packages/core/src/render3/node_manipulation.ts @@ -23,7 +23,7 @@ import {isLContainer, isLView} from './interfaces/type_checks'; import {CHILD_HEAD, CLEANUP, DECLARATION_COMPONENT_VIEW, DECLARATION_LCONTAINER, DestroyHookData, FLAGS, HookData, HookFn, HOST, LView, LViewFlags, NEXT, PARENT, QUERIES, RENDERER, T_HOST, TVIEW, TView, unusedValueExportToPlacateAjd as unused5} from './interfaces/view'; import {assertNodeOfPossibleTypes, assertNodeType} from './node_assert'; import {getLViewParent} from './util/view_traversal_utils'; -import {getNativeByTNode, unwrapRNode} from './util/view_utils'; +import {getNativeByTNode, unwrapRNode, updateTransplantedViewCount} from './util/view_utils'; const unusedValueToPlacateAjd = unused1 + unused2 + unused3 + unused4 + unused5; @@ -270,18 +270,13 @@ function trackMovedView(declarationContainer: LContainer, lView: LView) { ngDevMode && assertLContainer(insertedLContainer); const insertedComponentLView = insertedLContainer[PARENT]![DECLARATION_COMPONENT_VIEW]; ngDevMode && assertDefined(insertedComponentLView, 'Missing insertedComponentLView'); - const insertedComponentIsOnPush = - (insertedComponentLView[FLAGS] & LViewFlags.CheckAlways) !== LViewFlags.CheckAlways; - if (insertedComponentIsOnPush) { - const declaredComponentLView = lView[DECLARATION_COMPONENT_VIEW]; - ngDevMode && assertDefined(declaredComponentLView, 'Missing declaredComponentLView'); - if (declaredComponentLView !== insertedComponentLView) { - // At this point the declaration-component is not same as insertion-component and we are in - // on-push mode, this means that this is a transplanted view. Mark the declared lView as - // having - // transplanted views so that those views can participate in CD. - declarationContainer[ACTIVE_INDEX] |= ActiveIndexFlag.HAS_TRANSPLANTED_VIEWS; - } + const declaredComponentLView = lView[DECLARATION_COMPONENT_VIEW]; + ngDevMode && assertDefined(declaredComponentLView, 'Missing declaredComponentLView'); + if (declaredComponentLView !== insertedComponentLView) { + // At this point the declaration-component is not same as insertion-component; this means that + // this is a transplanted view. Mark the declared lView as having transplanted views so that + // those views can participate in CD. + declarationContainer[ACTIVE_INDEX] |= ActiveIndexFlag.HAS_TRANSPLANTED_VIEWS; } if (movedViews === null) { declarationContainer[MOVED_VIEWS] = [lView]; @@ -297,8 +292,18 @@ function detachMovedView(declarationContainer: LContainer, lView: LView) { declarationContainer[MOVED_VIEWS], 'A projected view should belong to a non-empty projected views collection'); const movedViews = declarationContainer[MOVED_VIEWS]!; - const declaredViewIndex = movedViews.indexOf(lView); - movedViews.splice(declaredViewIndex, 1); + const declarationViewIndex = movedViews.indexOf(lView); + const insertionLContainer = lView[PARENT] as LContainer; + ngDevMode && assertLContainer(insertionLContainer); + + // If the view was marked for refresh but then detached before it was checked (where the flag + // would be cleared and the counter decremented), we need to decrement the view counter here + // instead. + if (lView[FLAGS] & LViewFlags.RefreshTransplantedView) { + updateTransplantedViewCount(insertionLContainer, -1); + } + + movedViews.splice(declarationViewIndex, 1); } /** diff --git a/packages/core/src/render3/util/view_utils.ts b/packages/core/src/render3/util/view_utils.ts index 54fe62be01..59a3d4edaa 100644 --- a/packages/core/src/render3/util/view_utils.ts +++ b/packages/core/src/render3/util/view_utils.ts @@ -13,7 +13,7 @@ import {LContext, MONKEY_PATCH_KEY_NAME} from '../interfaces/context'; import {TConstants, TNode} from '../interfaces/node'; import {isProceduralRenderer, RNode} from '../interfaces/renderer'; import {isLContainer, isLView} from '../interfaces/type_checks'; -import {FLAGS, HEADER_OFFSET, HOST, LView, LViewFlags, PARENT, PREORDER_HOOK_FLAGS, RENDERER, TData, TView} from '../interfaces/view'; +import {FLAGS, HEADER_OFFSET, HOST, LView, LViewFlags, PARENT, PREORDER_HOOK_FLAGS, RENDERER, TData, TRANSPLANTED_VIEWS_TO_REFRESH, TView} from '../interfaces/view'; @@ -194,4 +194,24 @@ export function getLContainerActiveIndex(lContainer: LContainer) { export function setLContainerActiveIndex(lContainer: LContainer, index: number) { lContainer[ACTIVE_INDEX] = index << ActiveIndexFlag.SHIFT; -} \ No newline at end of file +} + +/** + * Updates the `TRANSPLANTED_VIEWS_TO_REFRESH` counter on the `LContainer` as well as the parents + * whose + * 1. counter goes from 0 to 1, indicating that there is a new child that has a view to refresh + * or + * 2. counter goes from 1 to 0, indicating there are no more descendant views to refresh + */ +export function updateTransplantedViewCount(lContainer: LContainer, amount: 1|- 1) { + lContainer[TRANSPLANTED_VIEWS_TO_REFRESH] += amount; + let viewOrContainer: LView|LContainer = lContainer; + let parent: LView|LContainer|null = lContainer[PARENT]; + while (parent !== null && + ((amount === 1 && viewOrContainer[TRANSPLANTED_VIEWS_TO_REFRESH] === 1) || + (amount === -1 && viewOrContainer[TRANSPLANTED_VIEWS_TO_REFRESH] === 0))) { + parent[TRANSPLANTED_VIEWS_TO_REFRESH] += amount; + viewOrContainer = parent; + parent = parent[PARENT]; + } +} diff --git a/packages/core/test/acceptance/change_detection_spec.ts b/packages/core/test/acceptance/change_detection_spec.ts index 9e9643ddbc..df6d67f9c8 100644 --- a/packages/core/test/acceptance/change_detection_spec.ts +++ b/packages/core/test/acceptance/change_detection_spec.ts @@ -9,7 +9,6 @@ import {CommonModule} from '@angular/common'; import {ApplicationRef, ChangeDetectionStrategy, ChangeDetectorRef, Component, ComponentFactoryResolver, ComponentRef, Directive, DoCheck, EmbeddedViewRef, ErrorHandler, Input, NgModule, OnInit, QueryList, TemplateRef, Type, ViewChild, ViewChildren, ViewContainerRef} from '@angular/core'; -import {AfterViewChecked} from '@angular/core/src/core'; import {ComponentFixture, TestBed} from '@angular/core/testing'; import {expect} from '@angular/platform-browser/testing/src/matchers'; import {ivyEnabled} from '@angular/private/testing'; @@ -1227,167 +1226,6 @@ describe('change detection', () => { }); }); - describe('transplanted views', () => { - @Component({ - selector: 'insert-comp', - changeDetection: ChangeDetectionStrategy.OnPush, - template: ` - InsertComp({{greeting}}) -
- - - -
- ` - }) - class InsertComp implements DoCheck, AfterViewChecked { - get template(): TemplateRef { - return declareComp.myTmpl; - } - greeting: string = 'Hello'; - constructor(public changeDetectorRef: ChangeDetectorRef) { - insertComp = this; - } - ngDoCheck(): void { - logValue = 'Insert'; - } - ngAfterViewChecked(): void { - logValue = null; - } - } - - @Component({ - selector: `declare-comp`, - template: ` - DeclareComp({{name}}) - - {{greeting}} {{logName()}}! - - ` - }) - class DeclareComp implements DoCheck, AfterViewChecked { - @ViewChild('myTmpl') myTmpl!: TemplateRef; - name: string = 'world'; - constructor() { - declareComp = this; - } - ngDoCheck(): void { - logValue = 'Declare'; - } - logName() { - // This will log when the embedded view gets CD. The `logValue` will show if the CD was from - // `Insert` or from `Declare` component. - log.push(logValue!); - return this.name; - } - ngAfterViewChecked(): void { - logValue = null; - } - } - - @Component({ - template: ` - - - ` - }) - class AppComp { - showDeclare: boolean = true; - showInsert: boolean = true; - constructor() { - appComp = this; - } - } - - let log!: string[]; - let logValue!: string|null; - let fixture!: ComponentFixture; - let appComp!: AppComp; - let insertComp!: InsertComp; - let declareComp!: DeclareComp; - - beforeEach(() => { - TestBed.configureTestingModule({ - declarations: [InsertComp, DeclareComp, AppComp], - imports: [CommonModule], - }); - log = []; - fixture = TestBed.createComponent(AppComp); - }); - - it('should CD with declaration', () => { - // NOTE: The CD of VE and Ivy is different and is captured in the assertions: - // `expect(log).toEqual(ivyEnabled ? [...] : [...])` - // - // The reason for this difference is in the algorithm which VE and Ivy use to deal with - // transplanted views: - // - VE: always runs CD at insertion point. If the insertion component is `OnPush` and the - // transplanted view is `CheckAlways` then the insertion component will be changed to - // `CheckAlways` (defeating the benefit of `OnPush`) - // - Ivy: Runs the CD at both the declaration as well as insertion point. The benefit of this - // approach is that each side (declaration/insertion) gets to keep its own semantics (either - // `OnPush` or `CheckAlways`). The implication is that: - // 1. The two semantics are slightly different. - // 2. Ivy will CD the transplanted view twice under some circumstances. (When both insertion - // and declaration are both dirty.) - - fixture.detectChanges(false); - expect(log).toEqual(['Insert']); - log.length = 0; - expect(trim(fixture.nativeElement.textContent)) - .toEqual('DeclareComp(world) InsertComp(Hello) Hello world!'); - - declareComp.name = 'Angular'; - fixture.detectChanges(false); - expect(log).toEqual(ivyEnabled ? ['Declare'] : ['Insert']); - log.length = 0; - // Expect transplanted LView to be CD because the declaration is CD. - expect(trim(fixture.nativeElement.textContent)) - .toEqual('DeclareComp(Angular) InsertComp(Hello) Hello Angular!'); - - insertComp.greeting = 'Hi'; - fixture.detectChanges(false); - expect(log).toEqual(ivyEnabled ? ['Declare'] : ['Insert']); - log.length = 0; - // expect no change because it is on push. - expect(trim(fixture.nativeElement.textContent)) - .toEqual('DeclareComp(Angular) InsertComp(Hello) Hello Angular!'); - - insertComp.changeDetectorRef.markForCheck(); - fixture.detectChanges(false); - expect(log).toEqual(ivyEnabled ? ['Declare', 'Insert'] : ['Insert']); - log.length = 0; - expect(trim(fixture.nativeElement.textContent)) - .toEqual('DeclareComp(Angular) InsertComp(Hi) Hi Angular!'); - - // Destroy insertion should also destroy declaration - appComp.showInsert = false; - insertComp.changeDetectorRef.markForCheck(); - fixture.detectChanges(false); - expect(log).toEqual([]); - log.length = 0; - expect(trim(fixture.nativeElement.textContent)).toEqual('DeclareComp(Angular)'); - - // Restore both - appComp.showInsert = true; - fixture.detectChanges(false); - expect(log).toEqual(['Insert']); - log.length = 0; - expect(trim(fixture.nativeElement.textContent)) - .toEqual('DeclareComp(Angular) InsertComp(Hello) Hello Angular!'); - - // Destroy declaration, But we should still be able to see updates in insertion - appComp.showDeclare = false; - insertComp.greeting = 'Hello'; - insertComp.changeDetectorRef.markForCheck(); - fixture.detectChanges(false); - expect(log).toEqual(['Insert']); - log.length = 0; - expect(trim(fixture.nativeElement.textContent)).toEqual('InsertComp(Hello) Hello Angular!'); - }); - }); describe('OnPush markForCheck in lifecycle hooks', () => { describe('with check no changes enabled', () => createOnPushMarkForCheckTests(true)); @@ -1652,7 +1490,3 @@ describe('change detection', () => { // }); }); }); - -function trim(text: string|null): string { - return text ? text.replace(/[\s\n]+/gm, ' ').trim() : ''; -} diff --git a/packages/core/test/acceptance/change_detection_transplanted_view_spec.ts b/packages/core/test/acceptance/change_detection_transplanted_view_spec.ts new file mode 100644 index 0000000000..0f01867817 --- /dev/null +++ b/packages/core/test/acceptance/change_detection_transplanted_view_spec.ts @@ -0,0 +1,601 @@ +/** + * @license + * Copyright Google Inc. All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import {CommonModule} from '@angular/common'; +import {ChangeDetectionStrategy, ChangeDetectorRef, Component, DoCheck, Input, TemplateRef, Type, ViewChild} from '@angular/core'; +import {AfterViewChecked} from '@angular/core/src/core'; +import {ComponentFixture, TestBed} from '@angular/core/testing'; +import {expect} from '@angular/platform-browser/testing/src/matchers'; +import {onlyInIvy} from '@angular/private/testing'; + +describe('change detection for transplanted views', () => { + describe('when declaration appears before insertion', () => { + const insertCompTemplate = ` + InsertComp({{greeting}}) +
+ + + +
+ `; + @Component({ + selector: 'insert-comp', + changeDetection: ChangeDetectionStrategy.OnPush, + template: insertCompTemplate, + }) + class InsertComp implements DoCheck, AfterViewChecked { + get template(): TemplateRef { + return declareComp.myTmpl; + } + greeting: string = 'Hello'; + constructor(public changeDetectorRef: ChangeDetectorRef) { + if (!(this instanceof InsertForOnPushDeclareComp)) { + insertComp = this; + } + } + ngDoCheck(): void { + logValue = 'Insert'; + } + ngAfterViewChecked(): void { + logValue = null; + } + } + + @Component({ + selector: 'insert-for-onpush-declare-comp', + changeDetection: ChangeDetectionStrategy.OnPush, + template: insertCompTemplate, + }) + class InsertForOnPushDeclareComp extends InsertComp { + constructor(public changeDetectorRef: ChangeDetectorRef) { + super(changeDetectorRef); + insertForOnPushDeclareComp = this; + } + get template(): TemplateRef { + return onPushDeclareComp.myTmpl; + } + } + + @Component({ + selector: `declare-comp`, + template: ` + DeclareComp({{name}}) + + {{greeting}} {{logName()}}! + + ` + }) + class DeclareComp implements DoCheck, AfterViewChecked { + @ViewChild('myTmpl') myTmpl!: TemplateRef; + name: string = 'world'; + constructor(readonly changeDetector: ChangeDetectorRef) { + if (!(this instanceof OnPushDeclareComp)) { + declareComp = this; + } + } + ngDoCheck(): void { + logValue = 'Declare'; + } + logName() { + // This will log when the embedded view gets CD. The `logValue` will show if the CD was + // from `Insert` or from `Declare` component. + log.push(logValue!); + return this.name; + } + ngAfterViewChecked(): void { + logValue = null; + } + } + + @Component({ + selector: `onpush-declare-comp`, + template: ` + OnPushDeclareComp({{name}}) + + {{greeting}} {{logName()}}! + + `, + changeDetection: ChangeDetectionStrategy.OnPush + }) + class OnPushDeclareComp extends DeclareComp { + constructor(readonly changeDetector: ChangeDetectorRef) { + super(changeDetector); + onPushDeclareComp = this; + } + } + + + @Component({ + template: ` + + + + + ` + }) + class AppComp { + showDeclare: boolean = false; + showOnPushDeclare: boolean = false; + showInsert: boolean = false; + showInsertForOnPushDeclare: boolean = false; + constructor() { + appComp = this; + } + } + + let log!: Array; + let logValue!: string|null; + let fixture!: ComponentFixture; + let appComp!: AppComp; + let insertComp!: InsertComp; + let insertForOnPushDeclareComp!: InsertForOnPushDeclareComp; + let declareComp!: DeclareComp; + let onPushDeclareComp!: OnPushDeclareComp; + + beforeEach(() => { + TestBed.configureTestingModule({ + declarations: + [InsertComp, DeclareComp, OnPushDeclareComp, InsertForOnPushDeclareComp, AppComp], + imports: [CommonModule], + }); + log = []; + fixture = TestBed.createComponent(AppComp); + }); + + describe('and declaration component is CheckAlways', () => { + beforeEach(() => { + fixture.componentInstance.showDeclare = true; + fixture.componentInstance.showInsert = true; + fixture.detectChanges(false); + log.length = 0; + }); + + it('should set up the component under test correctly', () => { + expect(log.length).toEqual(0); + expect(trim(fixture.nativeElement.textContent)) + .toEqual('DeclareComp(world) InsertComp(Hello) Hello world!'); + }); + + it('should CD at insertion point only', () => { + declareComp.name = 'Angular'; + fixture.detectChanges(false); + expect(log).toEqual(['Insert']); + log.length = 0; + expect(trim(fixture.nativeElement.textContent)) + .toEqual( + 'DeclareComp(Angular) InsertComp(Hello) Hello Angular!', + 'Expect transplanted LView to be CD because the declaration is CD.'); + + insertComp.greeting = 'Hi'; + fixture.detectChanges(false); + expect(log).toEqual(['Insert']); + log.length = 0; + expect(trim(fixture.nativeElement.textContent)) + .toEqual( + 'DeclareComp(Angular) InsertComp(Hello) Hello Angular!', + 'expect no change because it is on push.'); + + insertComp.changeDetectorRef.markForCheck(); + fixture.detectChanges(false); + expect(log).toEqual(['Insert']); + log.length = 0; + expect(trim(fixture.nativeElement.textContent)) + .toEqual('DeclareComp(Angular) InsertComp(Hi) Hi Angular!'); + + // Destroy insertion should also destroy declaration + appComp.showInsert = false; + fixture.detectChanges(false); + expect(log).toEqual([]); + log.length = 0; + expect(trim(fixture.nativeElement.textContent)).toEqual('DeclareComp(Angular)'); + + // Restore both + appComp.showInsert = true; + fixture.detectChanges(false); + expect(log).toEqual(['Insert']); + log.length = 0; + expect(trim(fixture.nativeElement.textContent)) + .toEqual('DeclareComp(Angular) InsertComp(Hello) Hello Angular!'); + + // Destroy declaration, But we should still be able to see updates in insertion + appComp.showDeclare = false; + insertComp.greeting = 'Hello'; + insertComp.changeDetectorRef.markForCheck(); + fixture.detectChanges(false); + expect(log).toEqual(['Insert']); + log.length = 0; + expect(trim(fixture.nativeElement.textContent)).toEqual('InsertComp(Hello) Hello Angular!'); + }); + + it('is not checked if detectChanges is called in declaration component', () => { + declareComp.name = 'Angular'; + declareComp.changeDetector.detectChanges(); + expect(log).toEqual([]); + log.length = 0; + expect(trim(fixture.nativeElement.textContent)) + .toEqual('DeclareComp(Angular) InsertComp(Hello) Hello world!'); + }); + + it('is checked as part of CheckNoChanges pass', () => { + fixture.detectChanges(true); + expect(log).toEqual(['Insert', null /* logName set to null afterViewChecked */]); + log.length = 0; + expect(trim(fixture.nativeElement.textContent)) + .toEqual('DeclareComp(world) InsertComp(Hello) Hello world!'); + }); + }); + + describe('and declaration component is OnPush', () => { + beforeEach(() => { + fixture.componentInstance.showOnPushDeclare = true; + fixture.componentInstance.showInsertForOnPushDeclare = true; + fixture.detectChanges(false); + log.length = 0; + }); + + it('should set up component under test correctly', () => { + expect(log.length).toEqual(0); + expect(trim(fixture.nativeElement.textContent)) + .toEqual('OnPushDeclareComp(world) InsertComp(Hello) Hello world!'); + }); + + it('should not check anything no views are dirty', () => { + fixture.detectChanges(false); + expect(log).toEqual([]); + }); + + it('should CD at insertion point only', () => { + onPushDeclareComp.name = 'Angular'; + insertForOnPushDeclareComp.greeting = 'Hi'; + // mark declaration point dirty + onPushDeclareComp.changeDetector.markForCheck(); + fixture.detectChanges(false); + expect(log).toEqual(['Insert']); + log.length = 0; + expect(trim(fixture.nativeElement.textContent)) + .toEqual('OnPushDeclareComp(Angular) InsertComp(Hello) Hello Angular!'); + + // mark insertion point dirty + insertForOnPushDeclareComp.changeDetectorRef.markForCheck(); + fixture.detectChanges(false); + expect(log).toEqual(['Insert']); + log.length = 0; + expect(trim(fixture.nativeElement.textContent)) + .toEqual('OnPushDeclareComp(Angular) InsertComp(Hi) Hi Angular!'); + + // mark both insertion and declaration point dirty + insertForOnPushDeclareComp.changeDetectorRef.markForCheck(); + onPushDeclareComp.changeDetector.markForCheck(); + fixture.detectChanges(false); + expect(log).toEqual(['Insert']); + log.length = 0; + }); + + it('is not checked if detectChanges is called in declaration component', () => { + onPushDeclareComp.name = 'Angular'; + onPushDeclareComp.changeDetector.detectChanges(); + expect(log).toEqual([]); + log.length = 0; + expect(trim(fixture.nativeElement.textContent)) + .toEqual('OnPushDeclareComp(Angular) InsertComp(Hello) Hello world!'); + }); + + // TODO(FW-1774): blocked by https://github.com/angular/angular/pull/34443 + xit('is checked as part of CheckNoChanges pass', () => { + // mark declaration point dirty + onPushDeclareComp.changeDetector.markForCheck(); + fixture.detectChanges(false); + expect(log).toEqual(['Insert', null /* logName set to null in afterViewChecked */]); + log.length = 0; + + // mark insertion point dirty + insertForOnPushDeclareComp.changeDetectorRef.markForCheck(); + fixture.detectChanges(false); + expect(log).toEqual(['Insert', null]); + log.length = 0; + + // mark both insertion and declaration point dirty + insertForOnPushDeclareComp.changeDetectorRef.markForCheck(); + onPushDeclareComp.changeDetector.markForCheck(); + fixture.detectChanges(false); + expect(log).toEqual(['Insert', null]); + log.length = 0; + }); + }); + }); + + // Note that backwards references are not handled well in VE or Ivy at the moment. + // These tests assert the current behavior. Some of these would need to be updated + // if future work refreshes backwards referenced transplanted views. + describe('backwards references', () => { + @Component({ + selector: 'insertion', + template: ` +
Insertion({{name}})
+ + `, + changeDetection: ChangeDetectionStrategy.OnPush + }) + class Insertion { + @Input() template !: TemplateRef<{}>; + name = 'initial'; + constructor(readonly changeDetectorRef: ChangeDetectorRef) {} + } + + @Component({ + selector: 'declaration', + template: ` +
Declaration({{name}})
+ +
{{incrementChecks()}}
+
TemplateDeclaration({{name}})
+
TemplateContext({{contextName}})
+ + `, + changeDetection: ChangeDetectionStrategy.OnPush + }) + class Declaration { + @ViewChild('template') template?: TemplateRef<{}>; + name = 'initial'; + transplantedViewRefreshCount = 0; + constructor(readonly changeDetectorRef: ChangeDetectorRef) {} + incrementChecks() { + this.transplantedViewRefreshCount++; + } + } + let fixture: ComponentFixture; + let appComponent: App; + + @Component({ + template: ` + + + + ` + }) + class App { + @ViewChild(Declaration) declaration!: Declaration; + @ViewChild(Insertion) insertion!: Insertion; + template?: TemplateRef<{}>; + showInsertion = false; + } + + beforeEach(() => { + fixture = TestBed.configureTestingModule({declarations: [App, Declaration, Insertion]}) + .createComponent(App); + appComponent = fixture.componentInstance; + fixture.detectChanges(false); + appComponent.showInsertion = true; + fixture.detectChanges(false); + appComponent.declaration.transplantedViewRefreshCount = 0; + }); + + it('should set up component under test correctly', () => { + expect(fixture.nativeElement.textContent) + .toEqual( + 'Insertion(initial)TemplateDeclaration(initial)TemplateContext(initial)Declaration(initial)'); + expect(appComponent.declaration.transplantedViewRefreshCount).toEqual(0); + }); + + + it('should not update declaration view when there is a change in the declaration and insertion is marked dirty', + () => { + appComponent.declaration.name = 'new name'; + appComponent.insertion.changeDetectorRef.markForCheck(); + fixture.detectChanges(false); + expect(fixture.nativeElement.textContent) + .toEqual( + 'Insertion(initial)TemplateDeclaration(new name)TemplateContext(initial)Declaration(initial)', + 'Name should not update in declaration view because only insertion was marked dirty'); + expect(appComponent.declaration.transplantedViewRefreshCount).toEqual(1); + }); + + it('updates only the declaration view when there is a change to declaration and declaration is marked dirty', + () => { + appComponent.declaration.name = 'new name'; + appComponent.declaration.changeDetectorRef.markForCheck(); + fixture.detectChanges(false); + + const expectedContent = + 'Insertion(initial)TemplateDeclaration(initial)TemplateContext(initial)Declaration(new name)'; + expect(fixture.nativeElement.textContent).toEqual(expectedContent); + expect(appComponent.declaration.transplantedViewRefreshCount).toEqual(0); + + // Note here that this second change detection should not be necessary, but is because of + // the backwards reference not being fully supported. The assertions below should be true + // after the first CD. + fixture.detectChanges(false); + expect(fixture.nativeElement.textContent) + .toEqual( + 'Insertion(initial)TemplateDeclaration(new name)TemplateContext(initial)Declaration(new name)'); + expect(appComponent.declaration.transplantedViewRefreshCount).toEqual(1); + }); + + it('should not update anything when there is a change to insertion and declaration is marked dirty', + () => { + appComponent.insertion.name = 'new name'; + appComponent.declaration.changeDetectorRef.markForCheck(); + fixture.detectChanges(false); + // Name should not update in insertion view because only declaration was marked dirty + // Context name also does not update in the template because the insertion view needs to be + // checked to update the `ngTemplateOutletContext` input. + expect(fixture.nativeElement.textContent) + .toEqual( + 'Insertion(initial)TemplateDeclaration(initial)TemplateContext(initial)Declaration(initial)', + 'Name should not update in insertion view because only declaration was marked dirty\n' + + 'Context name also does not update in the template because the insertion view needs to be' + + 'checked to update the `ngTemplateOutletContext` input.'); + // Note here that if backwards references were better supported, we would be able to + // refresh the transplanted view in the first `detectChanges` call but because the + // insertion point was already checked, we need to call detectChanges again to refresh it. + expect(appComponent.declaration.transplantedViewRefreshCount).toEqual(0); + + fixture.detectChanges(false); + expect(fixture.nativeElement.textContent) + .toEqual( + 'Insertion(initial)TemplateDeclaration(initial)TemplateContext(initial)Declaration(initial)', + 'Expected bindings to still be initial values. Again, TemplateContext can only update if ' + + 'insertion point is dirty and updates the ngTemplateOutletContext input.'); + expect(appComponent.declaration.transplantedViewRefreshCount).toEqual(1); + }); + + it('should update insertion view and template when there is a change to insertion and insertion marked dirty', + () => { + appComponent.insertion.name = 'new name'; + appComponent.insertion.changeDetectorRef.markForCheck(); + fixture.detectChanges(false); + expect(fixture.nativeElement.textContent) + .toEqual( + 'Insertion(new name)TemplateDeclaration(initial)TemplateContext(new name)Declaration(initial)'); + expect(appComponent.declaration.transplantedViewRefreshCount).toEqual(1); + }); + + it('should not refresh the template if nothing is marked dirty', () => { + fixture.detectChanges(false); + expect(appComponent.declaration.transplantedViewRefreshCount).toEqual(0); + }); + + it('should only refresh template once when declaration and insertion are marked dirty', () => { + appComponent.declaration.changeDetectorRef.markForCheck(); + appComponent.insertion.changeDetectorRef.markForCheck(); + fixture.detectChanges(false); + expect(appComponent.declaration.transplantedViewRefreshCount) + .toEqual( + 1, + 'Expected transplanted view to only be refreshed when insertion component is refreshed'); + }); + }); + + describe('transplanted views shielded by OnPush', () => { + @Component({ + selector: 'check-always-insertion', + template: `` + }) + class CheckAlwaysInsertion { + @Input() template !: TemplateRef<{}>; + } + + @Component({ + selector: 'on-push-insertion-host', + template: ``, + changeDetection: ChangeDetectionStrategy.OnPush + }) + class OnPushInsertionHost { + @Input() template !: TemplateRef<{}>; + constructor(readonly cdr: ChangeDetectorRef) {} + } + @Component({ + template: ` + {{value}} + + `, + changeDetection: ChangeDetectionStrategy.OnPush + }) + class OnPushDeclaration { + @ViewChild(OnPushInsertionHost) onPushInsertionHost?: OnPushInsertionHost; + value = 'initial'; + + constructor(readonly cdr: ChangeDetectorRef) {} + } + @Component({ + template: ` + {{value}} + + ` + }) + class CheckAlwaysDeclaration { + @ViewChild(OnPushInsertionHost) onPushInsertionHost?: OnPushInsertionHost; + value = 'initial'; + } + + function getFixture(componentUnderTest: Type): ComponentFixture { + return TestBed + .configureTestingModule({ + declarations: [ + CheckAlwaysDeclaration, OnPushDeclaration, CheckAlwaysInsertion, OnPushInsertionHost + ] + }) + .createComponent(componentUnderTest); + } + + it('refresh when transplanted view is declared in CheckAlways component', () => { + const fixture = getFixture(CheckAlwaysDeclaration); + fixture.detectChanges(); + fixture.componentInstance.value = 'new'; + fixture.detectChanges(); + expect(fixture.nativeElement.textContent).toEqual('new'); + }); + + it('refresh when transplanted view is declared in OnPush component', () => { + const fixture = getFixture(OnPushDeclaration); + fixture.detectChanges(); + fixture.componentInstance.value = 'new'; + fixture.componentInstance.cdr.markForCheck(); + fixture.detectChanges(); + expect(fixture.nativeElement.textContent).toEqual('new'); + }); + + onlyInIvy('behavior is inconsistent in VE').describe('when insertion is detached', () => { + it('does not refresh CheckAlways transplants', () => { + const fixture = getFixture(CheckAlwaysDeclaration); + fixture.detectChanges(); + fixture.componentInstance.onPushInsertionHost!.cdr.detach(); + fixture.componentInstance.value = 'new'; + fixture.detectChanges(); + expect(fixture.nativeElement.textContent).toEqual('initial'); + }); + + it('does not refresh OnPush transplants', () => { + const fixture = getFixture(OnPushDeclaration); + fixture.detectChanges(); + fixture.componentInstance.onPushInsertionHost!.cdr.detach(); + fixture.componentInstance.value = 'new'; + fixture.detectChanges(); + expect(fixture.nativeElement.textContent).toEqual('initial'); + }); + }); + }); + + it('refreshes transplanted views used as template in ngForTemplate', () => { + @Component({ + selector: 'triple', + template: '
', + changeDetection: ChangeDetectionStrategy.OnPush + }) + class TripleTemplate { + @Input() template !: TemplateRef<{}>; + } + + @Component({ + template: ` + {{name}} + + ` + }) + class App { + name = 'Penny'; + } + + const fixture = + TestBed.configureTestingModule({declarations: [App, TripleTemplate]}).createComponent(App); + fixture.detectChanges(); + expect(fixture.nativeElement.textContent).toEqual('PennyPennyPenny'); + fixture.componentInstance.name = 'Sheldon'; + fixture.detectChanges(); + expect(fixture.nativeElement.textContent) + .toEqual( + 'SheldonSheldonSheldon', + 'Expected transplanted view to be refreshed even when insertion is not dirty'); + }); +}); + +function trim(text: string|null): string { + return text ? text.replace(/[\s\n]+/gm, ' ').trim() : ''; +} 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 e9ab239847..9e1adb20ce 100644 --- a/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json +++ b/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json @@ -116,6 +116,9 @@ { "name": "SANITIZER" }, + { + "name": "TRANSPLANTED_VIEWS_TO_REFRESH" + }, { "name": "TVIEW" }, @@ -326,6 +329,9 @@ { "name": "getFactoryDef" }, + { + "name": "getFirstLContainer" + }, { "name": "getFirstNativeNode" }, @@ -356,6 +362,9 @@ { "name": "getNativeByTNode" }, + { + "name": "getNextLContainer" + }, { "name": "getNodeInjectable" }, @@ -503,6 +512,9 @@ { "name": "markAsComponentHost" }, + { + "name": "markTransplantedViewsForRefresh" + }, { "name": "matchTemplateAttribute" }, @@ -536,15 +548,15 @@ { "name": "refreshComponent" }, + { + "name": "refreshContainsDirtyView" + }, { "name": "refreshContentQueries" }, { "name": "refreshDynamicEmbeddedViews" }, - { - "name": "refreshTransplantedViews" - }, { "name": "refreshView" }, @@ -638,6 +650,9 @@ { "name": "unwrapRNode" }, + { + "name": "updateTransplantedViewCount" + }, { "name": "viewAttachedToChangeDetector" }, 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 67bf1add68..6c2d598829 100644 --- a/packages/core/test/bundling/hello_world/bundle.golden_symbols.json +++ b/packages/core/test/bundling/hello_world/bundle.golden_symbols.json @@ -107,6 +107,9 @@ { "name": "SANITIZER" }, + { + "name": "TRANSPLANTED_VIEWS_TO_REFRESH" + }, { "name": "TVIEW" }, @@ -263,6 +266,9 @@ { "name": "getFactoryDef" }, + { + "name": "getFirstLContainer" + }, { "name": "getFirstNativeNode" }, @@ -287,6 +293,9 @@ { "name": "getNativeByTNode" }, + { + "name": "getNextLContainer" + }, { "name": "getNodeInjectable" }, @@ -389,6 +398,9 @@ { "name": "markAsComponentHost" }, + { + "name": "markTransplantedViewsForRefresh" + }, { "name": "nativeAppendChild" }, @@ -413,15 +425,15 @@ { "name": "refreshComponent" }, + { + "name": "refreshContainsDirtyView" + }, { "name": "refreshContentQueries" }, { "name": "refreshDynamicEmbeddedViews" }, - { - "name": "refreshTransplantedViews" - }, { "name": "refreshView" }, @@ -488,6 +500,9 @@ { "name": "unwrapRNode" }, + { + "name": "updateTransplantedViewCount" + }, { "name": "viewAttachedToChangeDetector" }, diff --git a/packages/core/test/bundling/todo/bundle.golden_symbols.json b/packages/core/test/bundling/todo/bundle.golden_symbols.json index dddc6cf473..be4029b89e 100644 --- a/packages/core/test/bundling/todo/bundle.golden_symbols.json +++ b/packages/core/test/bundling/todo/bundle.golden_symbols.json @@ -206,6 +206,9 @@ { "name": "TNODE" }, + { + "name": "TRANSPLANTED_VIEWS_TO_REFRESH" + }, { "name": "TVIEW" }, @@ -614,6 +617,9 @@ { "name": "getFactoryDef" }, + { + "name": "getFirstLContainer" + }, { "name": "getFirstNativeNode" }, @@ -653,6 +659,9 @@ { "name": "getNativeByTNode" }, + { + "name": "getNextLContainer" + }, { "name": "getNodeInjectable" }, @@ -959,6 +968,9 @@ { "name": "markDuplicates" }, + { + "name": "markTransplantedViewsForRefresh" + }, { "name": "markViewDirty" }, @@ -1019,15 +1031,15 @@ { "name": "refreshComponent" }, + { + "name": "refreshContainsDirtyView" + }, { "name": "refreshContentQueries" }, { "name": "refreshDynamicEmbeddedViews" }, - { - "name": "refreshTransplantedViews" - }, { "name": "refreshView" }, @@ -1202,6 +1214,9 @@ { "name": "updateStyling" }, + { + "name": "updateTransplantedViewCount" + }, { "name": "viewAttachedToChangeDetector" },