Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(ivy): support lifecycle hooks of ViewContainerRef #23396

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
65 changes: 43 additions & 22 deletions packages/core/src/render3/instructions.ts
Expand Up @@ -224,24 +224,42 @@ export function enterView(newView: LView, host: LElementNode | LViewNode | null)
/**
* Used in lieu of enterView to make it clear when we are exiting a child view. This makes
* the direction of traversal (up or down the view tree) a bit clearer.
*/
export function leaveView(newView: LView): void {
if (!checkNoChangesMode) {
executeHooks(
directives !, currentView.tView.viewHooks, currentView.tView.viewCheckHooks, creationMode);
*
* @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 {
if (!creationOnly) {
if (!checkNoChangesMode) {
executeHooks(
directives !, currentView.tView.viewHooks, currentView.tView.viewCheckHooks,
creationMode);
}
// Views are clean and in update mode after being checked, so these bits are cleared
currentView.flags &= ~(LViewFlags.CreationMode | LViewFlags.Dirty);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like we should be clearing the creation flag whenever we leave a view, even if we are creationOnly. Otherwise, the creation flag would be on for the update mode check that comes next. Can you clarify why it should be in creation mode for both runs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the first update is not done with the CreationMode flag still on, then the onInit, afterContentInit and afterViewInit hooks are never called.

Copy link
Contributor

@kara kara Apr 24, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, thanks for explaining, I get what you're doing now.

My only concern is that the RenderFlags and ViewFlags are sometimes out of sync. The second time through, we are in update mode according to RenderFlags, but we are still in creation mode according to ViewFlags. What do you think about keeping the view flags in sync (by clearing the creation mode in ViewFlags), but using the existing lifecycleStage property to track hooks?

If that doesn't work, okay to keep it as is, as long as we add a comment/TODO explaining the discrepancy.

}
// Views should be clean and in update mode after being checked, so these bits are cleared
currentView.flags &= ~(LViewFlags.CreationMode | LViewFlags.Dirty);
currentView.lifecycleStage = LifecycleStage.Init;
currentView.bindingIndex = -1;
enterView(newView, null);
}

/** Refreshes directives in this view and triggers any init/content hooks. */
function refreshDirectives() {
executeInitAndContentHooks();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're removing this usage, we should probably remove the executeInitAndContentHooks function and inline it. I think it's only used in one other place.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is still used in 2 different places: renderComponent and renderComponentOrTemplate. If this can be refactored, let's do it in a future PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aha, forgot to check component.ts. In that case, sg.


/**
* Refreshes the view, executing the following steps in that order:
* triggers init hooks, refreshes dynamic children, triggers content hooks, sets host bindings,
* refreshes child components.
* Note: view hooks are triggered later when leaving the view.
* */
function refreshView() {
const tView = currentView.tView;
if (!checkNoChangesMode) {
executeInitHooks(currentView, tView, creationMode);
}
refreshDynamicChildren();
if (!checkNoChangesMode) {
executeHooks(directives !, tView.contentHooks, tView.contentCheckHooks, creationMode);
}

// This needs to be set before children are processed to support recursive components
tView.firstTemplatePass = firstTemplatePass = false;

Expand Down Expand Up @@ -456,10 +474,11 @@ export function renderEmbeddedTemplate<T>(
const _isParent = isParent;
const _previousOrParentNode = previousOrParentNode;
let oldView: LView;
let rf: RenderFlags = RenderFlags.Update;
try {
isParent = true;
previousOrParentNode = null !;
let rf: RenderFlags = RenderFlags.Update;

if (viewNode == null) {
const tView = getOrCreateTView(template, directives || null, pipes || null);
const lView = createLView(-1, renderer, tView, template, context, LViewFlags.CheckAlways);
Expand All @@ -468,13 +487,17 @@ export function renderEmbeddedTemplate<T>(
rf = RenderFlags.Create;
}
oldView = enterView(viewNode.data, viewNode);

template(rf, context);
refreshDirectives();
refreshDynamicChildren();

if (rf & RenderFlags.Update) {
refreshView();
} else {
viewNode.data.tView.firstTemplatePass = firstTemplatePass = false;
}
} finally {
leaveView(oldView !);
// renderEmbeddedTemplate() is called twice in fact, 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);
isParent = _isParent;
previousOrParentNode = _previousOrParentNode;
}
Expand All @@ -490,8 +513,7 @@ export function renderComponentOrTemplate<T>(
}
if (template) {
template(getRenderFlags(hostView), componentOrContext !);
refreshDynamicChildren();
refreshDirectives();
refreshView();
} else {
executeInitAndContentHooks();

Expand Down Expand Up @@ -1557,7 +1579,7 @@ function getOrCreateEmbeddedTView(viewIndex: number, parent: LContainerNode): TV

/** Marks the end of an embedded view. */
export function embeddedViewEnd(): void {
refreshDirectives();
refreshView();
isParent = false;
const viewNode = previousOrParentNode = currentView.node as LViewNode;
const containerNode = previousOrParentNode.parent as LContainerNode;
Expand Down Expand Up @@ -1968,8 +1990,7 @@ export function detectChangesInternal<T>(

try {
template(getRenderFlags(hostView), component);
refreshDirectives();
refreshDynamicChildren();
refreshView();
} finally {
leaveView(oldView);
}
Expand Down
Expand Up @@ -132,10 +132,10 @@
"name": "refreshChildComponents"
},
{
"name": "refreshDirectives"
"name": "refreshDynamicChildren"
},
{
"name": "refreshDynamicChildren"
"name": "refreshView"
},
{
"name": "renderComponent"
Expand Down
4 changes: 2 additions & 2 deletions packages/core/test/bundling/todo/bundle.golden_symbols.json
Expand Up @@ -555,10 +555,10 @@
"name": "refreshChildComponents"
},
{
"name": "refreshDirectives"
"name": "refreshDynamicChildren"
},
{
"name": "refreshDynamicChildren"
"name": "refreshView"
},
{
"name": "removeListeners"
Expand Down
88 changes: 88 additions & 0 deletions packages/core/test/render3/lifecycle_spec.ts
Expand Up @@ -2436,6 +2436,94 @@ describe('lifecycles', () => {

});

// Angular 5 reference: https://stackblitz.com/edit/lifecycle-hooks-ng
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great idea to include a stackblitz example in a comment 👍

it('should call all hooks in correct order with view and content', () => {
const Content = createAllHooksComponent('content', (rf: RenderFlags, ctx: any) => {});

const View = createAllHooksComponent('view', (rf: RenderFlags, ctx: any) => {});

/** <ng-content></ng-content><view [val]="val"></view> */
const Parent = createAllHooksComponent('parent', (rf: RenderFlags, ctx: any) => {
if (rf & RenderFlags.Create) {
projectionDef(0);
projection(1, 0);
elementStart(2, 'view');
elementEnd();
}
if (rf & RenderFlags.Update) {
elementProperty(2, 'val', bind(ctx.val));
}
}, [View]);

/**
* <parent [val]="1">
* <content [val]="1"></content>
* </parent>
* <parent [val]="2">
* <content [val]="2"></content>
* </parent>
*/
function Template(rf: RenderFlags, ctx: any) {
if (rf & RenderFlags.Create) {
elementStart(0, 'parent');
{
elementStart(1, 'content');
elementEnd();
}
elementEnd();
elementStart(2, 'parent');
{
elementStart(3, 'content');
elementEnd();
}
elementEnd();
}
if (rf & RenderFlags.Update) {
elementProperty(0, 'val', bind(1));
elementProperty(1, 'val', bind(1));
elementProperty(2, 'val', bind(2));
elementProperty(3, 'val', bind(2));
}
}

const defs = [Parent, Content];
renderToHtml(Template, {}, defs);
expect(events).toEqual([
'changes parent1', 'init parent1',
'check parent1', 'changes content1',
'init content1', 'check content1',
'changes parent2', 'init parent2',
'check parent2', 'changes content2',
'init content2', 'check content2',
'contentInit content1', 'contentCheck content1',
'contentInit parent1', 'contentCheck parent1',
'contentInit content2', 'contentCheck content2',
'contentInit parent2', 'contentCheck parent2',
'changes view1', 'init view1',
'check view1', 'contentInit view1',
'contentCheck view1', 'viewInit view1',
'viewCheck view1', 'changes view2',
'init view2', 'check view2',
'contentInit view2', 'contentCheck view2',
'viewInit view2', 'viewCheck view2',
'viewInit content1', 'viewCheck content1',
'viewInit parent1', 'viewCheck parent1',
'viewInit content2', 'viewCheck content2',
'viewInit parent2', 'viewCheck parent2'
]);

events = [];
renderToHtml(Template, {}, defs);
expect(events).toEqual([
'check parent1', 'check content1', 'check parent2', 'check content2',
'contentCheck content1', 'contentCheck parent1', 'contentCheck content2',
'contentCheck parent2', 'check view1', 'contentCheck view1', 'viewCheck view1',
'check view2', 'contentCheck view2', 'viewCheck view2', 'viewCheck content1',
'viewCheck parent1', 'viewCheck content2', 'viewCheck parent2'
]);

});

});

});
123 changes: 121 additions & 2 deletions packages/core/test/render3/view_container_ref_spec.ts
Expand Up @@ -8,7 +8,7 @@

import {Component, Directive, Pipe, PipeTransform, TemplateRef, ViewContainerRef} from '../../src/core';
import {getOrCreateNodeInjectorForNode, getOrCreateTemplateRef} from '../../src/render3/di';
import {defineComponent, defineDirective, definePipe, injectTemplateRef, injectViewContainerRef} from '../../src/render3/index';
import {NgOnChangesFeature, defineComponent, defineDirective, definePipe, injectTemplateRef, injectViewContainerRef} from '../../src/render3/index';
import {bind, container, containerRefreshEnd, containerRefreshStart, elementEnd, elementProperty, elementStart, embeddedViewEnd, embeddedViewStart, interpolation1, load, loadDirective, projection, projectionDef, text, textBinding} from '../../src/render3/instructions';
import {RenderFlags} from '../../src/render3/interfaces/definition';
import {pipe, pipeBind1} from '../../src/render3/pipe';
Expand Down Expand Up @@ -414,9 +414,11 @@ describe('ViewContainerRef', () => {

const fixture = new ComponentFixture(SomeComponent);
directiveInstance !.vcref.createEmbeddedView(directiveInstance !.tplRef, fixture.component);
directiveInstance !.vcref.createEmbeddedView(directiveInstance !.tplRef, fixture.component);
fixture.update();
expect(fixture.html)
.toEqual('<child vcref="">**A**</child><child>**C**</child><child>**B**</child>');
.toEqual(
'<child vcref="">**A**</child><child>**C**</child><child>**C**</child><child>**B**</child>');
});
});

Expand Down Expand Up @@ -822,4 +824,121 @@ describe('ViewContainerRef', () => {
});
});
});

describe('life cycle hooks', () => {

// Angular 5 reference: https://stackblitz.com/edit/lifecycle-hooks-vcref
const log: string[] = [];
it('should call all hooks in correct order', () => {
@Component({selector: 'hooks', template: `{{name}}`})
class ComponentWithHooks {
name: string;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is missing an @Input decorator. Although it won't have any effect, it will make the ngComponentDef in line with the decorators.


private log(msg: string) { log.push(msg); }

ngOnChanges() { this.log('onChanges-' + this.name); }
ngOnInit() { this.log('onInit-' + this.name); }
ngDoCheck() { this.log('doCheck-' + this.name); }

ngAfterContentInit() { this.log('afterContentInit-' + this.name); }
ngAfterContentChecked() { this.log('afterContentChecked-' + this.name); }

ngAfterViewInit() { this.log('afterViewInit-' + this.name); }
ngAfterViewChecked() { this.log('afterViewChecked-' + this.name); }

static ngComponentDef = defineComponent({
type: ComponentWithHooks,
selectors: [['hooks']],
factory: () => new ComponentWithHooks(),
template: (rf: RenderFlags, cmp: ComponentWithHooks) => {
if (rf & RenderFlags.Create) {
text(0);
}
if (rf & RenderFlags.Update) {
textBinding(0, interpolation1('', cmp.name, ''));
}
},
features: [NgOnChangesFeature()],
inputs: {name: 'name'}
});
}

@Component({
template: `
<ng-template #foo>
<hooks [name]="'C'"></hooks>
</ng-template>
<hooks vcref [tplRef]="foo" [name]="'A'"></hooks>
<hooks [name]="'B'"></hooks>
`
})
class SomeComponent {
static ngComponentDef = defineComponent({
type: SomeComponent,
selectors: [['some-comp']],
factory: () => new SomeComponent(),
template: (rf: RenderFlags, cmp: SomeComponent) => {
if (rf & RenderFlags.Create) {
container(0, (rf: RenderFlags, ctx: any) => {
if (rf & RenderFlags.Create) {
elementStart(0, 'hooks');
elementEnd();
}
if (rf & RenderFlags.Update) {
elementProperty(0, 'name', bind('C'));
}
});
elementStart(1, 'hooks', ['vcref', '']);
elementEnd();
elementStart(2, 'hooks');
elementEnd();
}
if (rf & RenderFlags.Update) {
const tplRef = getOrCreateTemplateRef(getOrCreateNodeInjectorForNode(load(0)));
elementProperty(1, 'tplRef', bind(tplRef));
elementProperty(1, 'name', bind('A'));
elementProperty(2, 'name', bind('B'));
}
},
directives: [ComponentWithHooks, DirectiveWithVCRef]
});
}

const fixture = new ComponentFixture(SomeComponent);
expect(log).toEqual([
'onChanges-A', 'onInit-A', 'doCheck-A', 'onChanges-B', 'onInit-B', 'doCheck-B',
'afterContentInit-A', 'afterContentChecked-A', 'afterContentInit-B',
'afterContentChecked-B', 'afterViewInit-A', 'afterViewChecked-A', 'afterViewInit-B',
'afterViewChecked-B'
]);

log.length = 0;
fixture.update();
expect(log).toEqual([
'doCheck-A', 'doCheck-B', 'afterContentChecked-A', 'afterContentChecked-B',
'afterViewChecked-A', 'afterViewChecked-B'
]);

log.length = 0;
directiveInstance !.vcref.createEmbeddedView(directiveInstance !.tplRef, fixture.component);
expect(fixture.html).toEqual('<hooks vcref="">A</hooks><hooks></hooks><hooks>B</hooks>');
expect(log).toEqual([]);

log.length = 0;
fixture.update();
expect(fixture.html).toEqual('<hooks vcref="">A</hooks><hooks>C</hooks><hooks>B</hooks>');
expect(log).toEqual([
'doCheck-A', 'doCheck-B', 'onChanges-C', 'onInit-C', 'doCheck-C', 'afterContentInit-C',
'afterContentChecked-C', 'afterViewInit-C', 'afterViewChecked-C', 'afterContentChecked-A',
'afterContentChecked-B', 'afterViewChecked-A', 'afterViewChecked-B'
]);

log.length = 0;
fixture.update();
expect(log).toEqual([
'doCheck-A', 'doCheck-B', 'doCheck-C', 'afterContentChecked-C', 'afterViewChecked-C',
'afterContentChecked-A', 'afterContentChecked-B', 'afterViewChecked-A', 'afterViewChecked-B'
]);
});
});
});