From d485346d3c33ee80c252cc934deabd12c3819d50 Mon Sep 17 00:00:00 2001 From: Kara Erickson Date: Tue, 6 Mar 2018 10:13:49 -0800 Subject: [PATCH] fix(ivy): lifecycle hooks should be queued for root component (#22614) PR Close #22614 --- packages/core/src/render3/component.ts | 37 ++++++++++-- packages/core/src/render3/index.ts | 3 +- packages/core/test/render3/component_spec.ts | 6 +- packages/core/test/render3/lifecycle_spec.ts | 60 ++++++++++++++++++- packages/core/test/render3/listeners_spec.ts | 3 +- packages/core/test/render3/render_util.ts | 6 +- .../test/render3/renderer_factory_spec.ts | 12 ++-- 7 files changed, 105 insertions(+), 22 deletions(-) diff --git a/packages/core/src/render3/component.ts b/packages/core/src/render3/component.ts index 5c53ef6a34..b878ab0d3f 100644 --- a/packages/core/src/render3/component.ts +++ b/packages/core/src/render3/component.ts @@ -12,6 +12,7 @@ import {Injector} from '../di/injector'; import {ComponentRef as viewEngine_ComponentRef} from '../linker/component_factory'; import {assertNotNull} from './assert'; +import {queueLifecycleHooks} from './hooks'; import {CLEAN_PROMISE, _getComponentHostLElementNode, createLView, createTView, detectChanges, directiveCreate, enterView, getDirectiveInstance, hostElement, initChangeDetectorIfExisting, leaveView, locateHostElement, scheduleChangeDetection} from './instructions'; import {ComponentDef, ComponentType} from './interfaces/definition'; import {RElement, RendererFactory3, domRendererFactory3} from './interfaces/renderer'; @@ -39,7 +40,13 @@ export interface CreateComponentOptions { * List of features to be applied to the created component. Features are simply * functions that decorate a component with a certain behavior. * - * Example: PublicFeature is a function that makes the component public to the DI system. + * Typically, the features in this list are features that cannot be added to the + * other features list in the component definition because they rely on other factors. + * + * Example: RootLifecycleHooks is a function that adds lifecycle hook capabilities + * to root components in a tree-shakable way. It cannot be added to the component + * features list because there's no way of knowing when the component will be used as + * a root component. */ features?: ((component: T, componentDef: ComponentDef) => void)[]; @@ -124,12 +131,14 @@ export function renderComponent( // Create element node at index 0 in data array const elementNode = hostElement(hostNode, componentDef); // Create directive instance with n() and store at index 1 in data array (el is 0) - const instance = componentDef.n(); component = rootContext.component = - getDirectiveInstance(directiveCreate(1, instance, componentDef)); - initChangeDetectorIfExisting(elementNode.nodeInjector, instance); + getDirectiveInstance(directiveCreate(1, componentDef.n(), componentDef)); + initChangeDetectorIfExisting(elementNode.nodeInjector, component); } finally { - leaveView(oldView); + // We must not use leaveView here because it will set creationMode to false too early, + // causing init-only hooks not to run. The detectChanges call below will execute + // leaveView at the appropriate time in the lifecycle. + enterView(oldView, null); } opts.features && opts.features.forEach((feature) => feature(component, componentDef)); @@ -139,7 +148,23 @@ export function renderComponent( /** - * Retrieve the root component of any component by walking the parent `LView` until + * Used to enable lifecycle hooks on the root component. + * + * Include this feature when calling `renderComponent` if the root component + * you are rendering has lifecycle hooks defined. Otherwise, the hooks won't + * be called properly. + * + * Example: + * + * renderComponent(AppComponent, {features: [RootLifecycleHooks]}); + */ +export function RootLifecycleHooks(component: any, def: ComponentDef): void { + const elementNode = _getComponentHostLElementNode(component); + queueLifecycleHooks(elementNode.flags, elementNode.view); +} + +/** + * Retrieve the root context for any component by walking the parent `LView` until * reaching the root `LView`. * * @param component any component diff --git a/packages/core/src/render3/index.ts b/packages/core/src/render3/index.ts index 7d764f6f34..9151b460c8 100644 --- a/packages/core/src/render3/index.ts +++ b/packages/core/src/render3/index.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {createComponentRef, getHostElement, getRenderedText, renderComponent, whenRendered} from './component'; +import {RootLifecycleHooks, createComponentRef, getHostElement, getRenderedText, renderComponent, whenRendered} from './component'; import {NgOnChangesFeature, PublicFeature, defineComponent, defineDirective, definePipe} from './definition'; import {InjectFlags} from './di'; import {ComponentDef, ComponentTemplate, ComponentType, DirectiveDef, DirectiveDefFlags, DirectiveType} from './interfaces/definition'; @@ -109,6 +109,7 @@ export { DirectiveType, NgOnChangesFeature, PublicFeature, + RootLifecycleHooks, defineComponent, defineDirective, definePipe, diff --git a/packages/core/test/render3/component_spec.ts b/packages/core/test/render3/component_spec.ts index 83989080ea..011c7357fc 100644 --- a/packages/core/test/render3/component_spec.ts +++ b/packages/core/test/render3/component_spec.ts @@ -181,14 +181,14 @@ describe('encapsulation', () => { } it('should encapsulate children, but not host nor grand children', () => { - renderComponent(WrapperComponent, getRendererFactory2(document)); + renderComponent(WrapperComponent, {rendererFactory: getRendererFactory2(document)}); expect(containerEl.outerHTML) .toMatch( /
foobar<\/span><\/leaf><\/encapsulated><\/div>/); }); it('should encapsulate host', () => { - renderComponent(EncapsulatedComponent, getRendererFactory2(document)); + renderComponent(EncapsulatedComponent, {rendererFactory: getRendererFactory2(document)}); expect(containerEl.outerHTML) .toMatch( /
foobar<\/span><\/leaf><\/div>/); @@ -230,7 +230,7 @@ describe('encapsulation', () => { }); } - renderComponent(WrapperComponentWith, getRendererFactory2(document)); + renderComponent(WrapperComponentWith, {rendererFactory: getRendererFactory2(document)}); expect(containerEl.outerHTML) .toMatch( /
bar<\/span><\/leaf><\/div>/); diff --git a/packages/core/test/render3/lifecycle_spec.ts b/packages/core/test/render3/lifecycle_spec.ts index 3a1fb02793..8c1c0cd5e8 100644 --- a/packages/core/test/render3/lifecycle_spec.ts +++ b/packages/core/test/render3/lifecycle_spec.ts @@ -7,10 +7,10 @@ */ import {SimpleChanges} from '../../src/core'; -import {ComponentTemplate, NgOnChangesFeature, defineComponent, defineDirective} from '../../src/render3/index'; -import {bind, container, containerRefreshEnd, containerRefreshStart, directiveRefresh, elementEnd, elementProperty, elementStart, embeddedViewEnd, embeddedViewStart, listener, projection, projectionDef, store, text} from '../../src/render3/instructions'; +import {ComponentTemplate, NgOnChangesFeature, RootLifecycleHooks, defineComponent, defineDirective} from '../../src/render3/index'; +import {bind, container, containerRefreshEnd, containerRefreshStart, directiveRefresh, elementEnd, elementProperty, elementStart, embeddedViewEnd, embeddedViewStart, listener, markDirty, projection, projectionDef, store, text} from '../../src/render3/instructions'; -import {containerEl, renderToHtml} from './render_util'; +import {containerEl, renderComponent, renderToHtml, requestAnimationFrame} from './render_util'; describe('lifecycles', () => { @@ -86,6 +86,15 @@ describe('lifecycles', () => { expect(events).toEqual(['comp1']); }); + it('should be called on root component in creation mode', () => { + const comp = renderComponent(Comp, {features: [RootLifecycleHooks]}); + expect(events).toEqual(['comp']); + + markDirty(comp); + requestAnimationFrame.flush(); + expect(events).toEqual(['comp']); + }); + it('should call parent onInit before child onInit', () => { /** * @@ -413,6 +422,15 @@ describe('lifecycles', () => { expect(events).toEqual(['comp', 'comp']); }); + it('should be called on root component', () => { + const comp = renderComponent(Comp, {features: [RootLifecycleHooks]}); + expect(events).toEqual(['comp']); + + markDirty(comp); + requestAnimationFrame.flush(); + expect(events).toEqual(['comp', 'comp']); + }); + it('should call parent doCheck before child doCheck', () => { /** * @@ -564,6 +582,15 @@ describe('lifecycles', () => { expect(events).toEqual(['comp']); }); + it('should be called on root component in creation mode', () => { + const comp = renderComponent(Comp, {features: [RootLifecycleHooks]}); + expect(events).toEqual(['comp']); + + markDirty(comp); + requestAnimationFrame.flush(); + expect(events).toEqual(['comp']); + }); + it('should be called on every init (if blocks)', () => { /** * % if (condition) { @@ -842,6 +869,15 @@ describe('lifecycles', () => { }); + it('should be called on root component', () => { + const comp = renderComponent(Comp, {features: [RootLifecycleHooks]}); + expect(allEvents).toEqual(['comp init', 'comp check']); + + markDirty(comp); + requestAnimationFrame.flush(); + expect(allEvents).toEqual(['comp init', 'comp check', 'comp check']); + }); + }); describe('directives', () => { @@ -948,6 +984,15 @@ describe('lifecycles', () => { expect(events).toEqual(['comp']); }); + it('should be called on root component in creation mode', () => { + const comp = renderComponent(Comp, {features: [RootLifecycleHooks]}); + expect(events).toEqual(['comp']); + + markDirty(comp); + requestAnimationFrame.flush(); + expect(events).toEqual(['comp']); + }); + it('should be called every time a view is initialized (if block)', () => { /* * % if (condition) { @@ -1250,6 +1295,15 @@ describe('lifecycles', () => { expect(allEvents).toEqual(['comp init', 'comp check', 'comp check']); }); + it('should be called on root component', () => { + const comp = renderComponent(Comp, {features: [RootLifecycleHooks]}); + expect(allEvents).toEqual(['comp init', 'comp check']); + + markDirty(comp); + requestAnimationFrame.flush(); + expect(allEvents).toEqual(['comp init', 'comp check', 'comp check']); + }); + it('should call ngAfterViewChecked with bindings', () => { /** */ function Template(ctx: any, cm: boolean) { diff --git a/packages/core/test/render3/listeners_spec.ts b/packages/core/test/render3/listeners_spec.ts index 406a1eb040..64b0cd0d4a 100644 --- a/packages/core/test/render3/listeners_spec.ts +++ b/packages/core/test/render3/listeners_spec.ts @@ -106,7 +106,8 @@ describe('event listeners', () => { }); it('should retain event handler return values with renderer2', () => { - const preventDefaultComp = renderComponent(PreventDefaultComp, getRendererFactory2(document)); + const preventDefaultComp = + renderComponent(PreventDefaultComp, {rendererFactory: getRendererFactory2(document)}); const button = containerEl.querySelector('button') !; button.click(); diff --git a/packages/core/test/render3/render_util.ts b/packages/core/test/render3/render_util.ts index 2ae002f628..75dd37ffbf 100644 --- a/packages/core/test/render3/render_util.ts +++ b/packages/core/test/render3/render_util.ts @@ -10,6 +10,7 @@ import {stringifyElement} from '@angular/platform-browser/testing/src/browser_ut import {ComponentTemplate, ComponentType, DirectiveType, PublicFeature, defineComponent, defineDirective, renderComponent as _renderComponent} from '../../src/render3/index'; import {NG_HOST_SYMBOL, createLNode, createLView, renderTemplate} from '../../src/render3/instructions'; +import {CreateComponentOptions} from '../../src/render3/component'; import {DirectiveDefArgs} from '../../src/render3/interfaces/definition'; import {LElementNode, LNodeFlags} from '../../src/render3/interfaces/node'; import {RElement, RText, Renderer3, RendererFactory3, domRendererFactory3} from '../../src/render3/interfaces/renderer'; @@ -112,11 +113,12 @@ export function renderToHtml( beforeEach(resetDOM); -export function renderComponent(type: ComponentType, rendererFactory?: RendererFactory3): T { +export function renderComponent(type: ComponentType, opts?: CreateComponentOptions): T { return _renderComponent(type, { - rendererFactory: rendererFactory || testRendererFactory, + rendererFactory: opts && opts.rendererFactory || testRendererFactory, host: containerEl, scheduler: requestAnimationFrame, + features: opts && opts.features }); } diff --git a/packages/core/test/render3/renderer_factory_spec.ts b/packages/core/test/render3/renderer_factory_spec.ts index 0cbd10734d..4b6ca626c0 100644 --- a/packages/core/test/render3/renderer_factory_spec.ts +++ b/packages/core/test/render3/renderer_factory_spec.ts @@ -74,7 +74,7 @@ describe('renderer factory lifecycle', () => { beforeEach(() => { logs = []; }); it('should work with a component', () => { - const component = renderComponent(SomeComponent, rendererFactory); + const component = renderComponent(SomeComponent, {rendererFactory}); expect(logs).toEqual(['create', 'create', 'begin', 'component', 'end']); logs = []; @@ -83,7 +83,7 @@ describe('renderer factory lifecycle', () => { }); it('should work with a component which throws', () => { - expect(() => renderComponent(SomeComponentWhichThrows, rendererFactory)).toThrow(); + expect(() => renderComponent(SomeComponentWhichThrows, {rendererFactory})).toThrow(); expect(logs).toEqual(['create', 'create', 'begin', 'end']); }); @@ -177,13 +177,13 @@ describe('animation renderer factory', () => { } it('should work with components without animations', () => { - renderComponent(SomeComponent, getAnimationRendererFactory2(document)); + renderComponent(SomeComponent, {rendererFactory: getAnimationRendererFactory2(document)}); expect(toHtml(containerEl)).toEqual('foo'); }); isBrowser && it('should work with animated components', (done) => { - const factory = getAnimationRendererFactory2(document); - const component = renderComponent(SomeComponentWithAnimation, factory); + const rendererFactory = getAnimationRendererFactory2(document); + const component = renderComponent(SomeComponentWithAnimation, {rendererFactory}); expect(toHtml(containerEl)) .toMatch(/
foo<\/div>/); @@ -197,7 +197,7 @@ describe('animation renderer factory', () => { ]); player.finish(); - factory.whenRenderingDone !().then(() => { + rendererFactory.whenRenderingDone !().then(() => { expect(eventLogs).toEqual(['void - start', 'void - done', 'on - start', 'on - done']); done(); });