Skip to content

Commit

Permalink
fix(core): projected views should be dirty checked when the declaring…
Browse files Browse the repository at this point in the history 
… component is dirty checked. (#16592)

Previously a projected view was only dirty checked when the
component in which it was inserted was dirty checked.

This fix changes the behavior so that a view is also dirty checked if
the declaring component is dirty checked.

Note: This does not change the order of change detection,
only the fact whether a projected view is dirty checked or not.

Fixes #14321
  • Loading branch information
@tbosch @jasonaden
tbosch authored and jasonaden committed May 10, 2017
1 parent ec77bf9 commit 9218812
Show file tree
Hide file tree
Showing 5 changed files with 246 additions and 39 deletions.
59 changes: 31 additions & 28 deletions packages/core/src/view/types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,37 +141,38 @@ export const enum NodeFlags {
None = 0,
TypeElement = 1 << 0,
TypeText = 1 << 1,
ProjectedTemplate = 1 << 2,
CatRenderNode = TypeElement | TypeText,
TypeNgContent = 1 << 2,
TypePipe = 1 << 3,
TypePureArray = 1 << 4,
TypePureObject = 1 << 5,
TypePurePipe = 1 << 6,
TypeNgContent = 1 << 3,
TypePipe = 1 << 4,
TypePureArray = 1 << 5,
TypePureObject = 1 << 6,
TypePurePipe = 1 << 7,
CatPureExpression = TypePureArray | TypePureObject | TypePurePipe,
TypeValueProvider = 1 << 7,
TypeClassProvider = 1 << 8,
TypeFactoryProvider = 1 << 9,
TypeUseExistingProvider = 1 << 10,
LazyProvider = 1 << 11,
PrivateProvider = 1 << 12,
TypeDirective = 1 << 13,
Component = 1 << 14,
TypeValueProvider = 1 << 8,
TypeClassProvider = 1 << 9,
TypeFactoryProvider = 1 << 10,
TypeUseExistingProvider = 1 << 11,
LazyProvider = 1 << 12,
PrivateProvider = 1 << 13,
TypeDirective = 1 << 14,
Component = 1 << 15,
CatProvider = TypeValueProvider | TypeClassProvider | TypeFactoryProvider |
TypeUseExistingProvider | TypeDirective,
OnInit = 1 << 15,
OnDestroy = 1 << 16,
DoCheck = 1 << 17,
OnChanges = 1 << 18,
AfterContentInit = 1 << 19,
AfterContentChecked = 1 << 20,
AfterViewInit = 1 << 21,
AfterViewChecked = 1 << 22,
EmbeddedViews = 1 << 23,
ComponentView = 1 << 24,
TypeContentQuery = 1 << 25,
TypeViewQuery = 1 << 26,
StaticQuery = 1 << 27,
DynamicQuery = 1 << 28,
OnInit = 1 << 16,
OnDestroy = 1 << 17,
DoCheck = 1 << 18,
OnChanges = 1 << 19,
AfterContentInit = 1 << 20,
AfterContentChecked = 1 << 21,
AfterViewInit = 1 << 22,
AfterViewChecked = 1 << 23,
EmbeddedViews = 1 << 24,
ComponentView = 1 << 25,
TypeContentQuery = 1 << 26,
TypeViewQuery = 1 << 27,
StaticQuery = 1 << 28,
DynamicQuery = 1 << 29,
CatQuery = TypeContentQuery | TypeViewQuery,

// mutually exclusive values...
Expand Down Expand Up @@ -328,7 +329,9 @@ export const enum ViewState {
FirstCheck = 1 << 1,
Attached = 1 << 2,
ChecksEnabled = 1 << 3,
Destroyed = 1 << 4,
CheckProjectedView = 1 << 4,
CheckProjectedViews = 1 << 5,
Destroyed = 1 << 6,

CatDetectChanges = Attached | ChecksEnabled,
CatInit = BeforeFirstCheck | CatDetectChanges
Expand Down
8 changes: 8 additions & 0 deletions packages/core/src/view/util.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,14 @@ export function markParentViewsForCheck(view: ViewData) {
}
}

export function markParentViewsForCheckProjectedViews(view: ViewData, endView: ViewData) {
let currView: ViewData|null = view;
while (currView && currView !== endView) {
currView.state |= ViewState.CheckProjectedViews;
currView = currView.viewContainerParent || currView.parent;
}
}

export function dispatchEvent(
view: ViewData, nodeIndex: number, eventName: string, event: any): boolean {
const nodeDef = view.def.nodes[nodeIndex];
Expand Down
79 changes: 71 additions & 8 deletions packages/core/src/view/view.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ import {checkAndUpdateQuery, createQuery} from './query';
import {createTemplateData, createViewContainerData} from './refs';
import {checkAndUpdateTextDynamic, checkAndUpdateTextInline, createText} from './text';
import {ArgumentType, CheckType, ElementData, NodeData, NodeDef, NodeFlags, ProviderData, RootData, Services, ViewData, ViewDefinition, ViewFlags, ViewHandleEventFn, ViewState, ViewUpdateFn, asElementData, asQueryList, asTextData} from './types';
import {NOOP, checkBindingNoChanges, isComponentView, resolveViewDefinition} from './util';
import {NOOP, checkBindingNoChanges, isComponentView, markParentViewsForCheckProjectedViews, resolveViewDefinition} from './util';
import {detachProjectedView} from './view_attach';

export function viewDef(
flags: ViewFlags, nodes: NodeDef[], updateDirectives?: ViewUpdateFn,
Expand Down Expand Up @@ -314,12 +315,14 @@ function createViewNodes(view: ViewData) {
}

export function checkNoChangesView(view: ViewData) {
markProjectedViewsForCheck(view);
Services.updateDirectives(view, CheckType.CheckNoChanges);
execEmbeddedViewsAction(view, ViewAction.CheckNoChanges);
Services.updateRenderer(view, CheckType.CheckNoChanges);
execComponentViewsAction(view, ViewAction.CheckNoChanges);
// Note: We don't check queries for changes as we didn't do this in v2.x.
// TODO(tbosch): investigate if we can enable the check again in v5.x with a nicer error message.
view.state &= ~(ViewState.CheckProjectedViews | ViewState.CheckProjectedView);
}

export function checkAndUpdateView(view: ViewData) {
Expand All @@ -329,6 +332,7 @@ export function checkAndUpdateView(view: ViewData) {
} else {
view.state &= ~ViewState.FirstCheck;
}
markProjectedViewsForCheck(view);
Services.updateDirectives(view, CheckType.CheckAndUpdate);
execEmbeddedViewsAction(view, ViewAction.CheckAndUpdate);
execQueriesAction(
Expand All @@ -343,14 +347,14 @@ export function checkAndUpdateView(view: ViewData) {
execComponentViewsAction(view, ViewAction.CheckAndUpdate);
execQueriesAction(
view, NodeFlags.TypeViewQuery, NodeFlags.DynamicQuery, CheckType.CheckAndUpdate);

callLifecycleHooksChildrenFirst(
view, NodeFlags.AfterViewChecked |
(view.state & ViewState.FirstCheck ? NodeFlags.AfterViewInit : 0));

if (view.def.flags & ViewFlags.OnPush) {
view.state &= ~ViewState.ChecksEnabled;
}
view.state &= ~(ViewState.CheckProjectedViews | ViewState.CheckProjectedView);
}

export function checkAndUpdateNode(
Expand All @@ -363,6 +367,31 @@ export function checkAndUpdateNode(
}
}

function markProjectedViewsForCheck(view: ViewData) {
const def = view.def;
if (!(def.nodeFlags & NodeFlags.ProjectedTemplate)) {
return;
}
for (let i = 0; i < def.nodes.length; i++) {
const nodeDef = def.nodes[i];
if (nodeDef.flags & NodeFlags.ProjectedTemplate) {
const projectedViews = asElementData(view, i).template._projectedViews;
if (projectedViews) {
for (let i = 0; i < projectedViews.length; i++) {
const projectedView = projectedViews[i];
projectedView.state |= ViewState.CheckProjectedView;
markParentViewsForCheckProjectedViews(projectedView, view);
}
}
} else if ((nodeDef.childFlags & NodeFlags.ProjectedTemplate) === 0) {
// a parent with leafs
// no child is a component,
// then skip the children
i += nodeDef.childCount;
}
}
}

function checkAndUpdateNodeInline(
view: ViewData, nodeDef: NodeDef, v0?: any, v1?: any, v2?: any, v3?: any, v4?: any, v5?: any,
v6?: any, v7?: any, v8?: any, v9?: any): boolean {
Expand Down Expand Up @@ -474,6 +503,7 @@ export function destroyView(view: ViewData) {
view.disposables[i]();
}
}
detachProjectedView(view);
if (view.renderer.destroyNode) {
destroyViewNodes(view);
}
Expand All @@ -498,7 +528,9 @@ function destroyViewNodes(view: ViewData) {
enum ViewAction {
CreateViewNodes,
CheckNoChanges,
CheckNoChangesProjectedViews,
CheckAndUpdate,
CheckAndUpdateProjectedViews,
Destroy
}

Expand Down Expand Up @@ -547,18 +579,44 @@ function callViewAction(view: ViewData, action: ViewAction) {
const viewState = view.state;
switch (action) {
case ViewAction.CheckNoChanges:
if ((viewState & ViewState.CatDetectChanges) === ViewState.CatDetectChanges &&
(viewState & ViewState.Destroyed) === 0) {
checkNoChangesView(view);
if ((viewState & ViewState.Destroyed) === 0) {
if ((viewState & ViewState.CatDetectChanges) === ViewState.CatDetectChanges) {
checkNoChangesView(view);
} else if (viewState & ViewState.CheckProjectedViews) {
execProjectedViewsAction(view, ViewAction.CheckNoChangesProjectedViews);
}
}
break;
case ViewAction.CheckNoChangesProjectedViews:
if ((viewState & ViewState.Destroyed) === 0) {
if (viewState & ViewState.CheckProjectedView) {
checkNoChangesView(view);
} else if (viewState & ViewState.CheckProjectedViews) {
execProjectedViewsAction(view, action);
}
}
break;
case ViewAction.CheckAndUpdate:
if ((viewState & ViewState.CatDetectChanges) === ViewState.CatDetectChanges &&
(viewState & ViewState.Destroyed) === 0) {
checkAndUpdateView(view);
if ((viewState & ViewState.Destroyed) === 0) {
if ((viewState & ViewState.CatDetectChanges) === ViewState.CatDetectChanges) {
checkAndUpdateView(view);
} else if (viewState & ViewState.CheckProjectedViews) {
execProjectedViewsAction(view, ViewAction.CheckAndUpdateProjectedViews);
}
}
break;
case ViewAction.CheckAndUpdateProjectedViews:
if ((viewState & ViewState.Destroyed) === 0) {
if (viewState & ViewState.CheckProjectedView) {
checkAndUpdateView(view);
} else if (viewState & ViewState.CheckProjectedViews) {
execProjectedViewsAction(view, action);
}
}
break;
case ViewAction.Destroy:
// Note: destroyView recurses over all views,
// so we don't need to special case projected views here.
destroyView(view);
break;
case ViewAction.CreateViewNodes:
Expand All @@ -567,6 +625,11 @@ function callViewAction(view: ViewData, action: ViewAction) {
}
}

function execProjectedViewsAction(view: ViewData, action: ViewAction) {
execEmbeddedViewsAction(view, action);
execComponentViewsAction(view, action);
}

function execQueriesAction(
view: ViewData, queryFlags: NodeFlags, staticDynamicQueryFlag: NodeFlags,
checkType: CheckType) {
Expand Down
22 changes: 20 additions & 2 deletions packages/core/src/view/view_attach.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
* found in the LICENSE file at https://angular.io/license
*/

import {ElementData, Services, ViewData} from './types';
import {RenderNodeAction, declaredViewContainer, renderNode, visitRootRenderNodes} from './util';
import {ElementData, NodeDef, NodeFlags, Services, ViewData, ViewDefinition, ViewState} from './types';
import {RenderNodeAction, declaredViewContainer, isComponentView, renderNode, visitRootRenderNodes} from './util';

export function attachEmbeddedView(
parentView: ViewData, elementData: ElementData, viewIndex: number | undefined | null,
Expand All @@ -25,6 +25,11 @@ export function attachEmbeddedView(
projectedViews = dvcElementData.template._projectedViews = [];
}
projectedViews.push(view);
if (view.parent) {
// Note: we are changing the NodeDef here as we cannot calculate
// the fact whether a template is used for projection during compilation.
markNodeAsProjectedTemplate(view.parent.def, view.parentNodeDef !);
}
}

Services.dirtyParentQueries(view);
Expand All @@ -33,6 +38,19 @@ export function attachEmbeddedView(
renderAttachEmbeddedView(elementData, prevView, view);
}

function markNodeAsProjectedTemplate(viewDef: ViewDefinition, nodeDef: NodeDef) {
if (nodeDef.flags & NodeFlags.ProjectedTemplate) {
return;
}
viewDef.nodeFlags |= NodeFlags.ProjectedTemplate;
nodeDef.flags |= NodeFlags.ProjectedTemplate;
let parentNodeDef = nodeDef.parent;
while (parentNodeDef) {
parentNodeDef.childFlags |= NodeFlags.ProjectedTemplate;
parentNodeDef = parentNodeDef.parent;
}
}

export function detachEmbeddedView(elementData: ElementData, viewIndex?: number): ViewData|null {
const embeddedViews = elementData.viewContainer !._embeddedViews;
if (viewIndex == null || viewIndex >= embeddedViews.length) {
Expand Down
Loading

0 comments on commit 9218812

Please sign in to comment.