Skip to content

Commit

Permalink
refactor(ivy): split up directiveCreate for tree shaking
Browse files Browse the repository at this point in the history
  • Loading branch information
kara committed Mar 17, 2018
1 parent b4b2ad3 commit 8353b5c
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 43 deletions.
10 changes: 7 additions & 3 deletions packages/core/src/render3/component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ 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, directiveCreate, enterView, getDirectiveInstance, getRootView, hostElement, initChangeDetectorIfExisting, locateHostElement, renderComponentOrTemplate} from './instructions';
import {queueInitHooks, queueLifecycleHooks} from './hooks';
import {CLEAN_PROMISE, _getComponentHostLElementNode, baseDirectiveCreate, createLView, createTView, enterView, getRootView, hostElement, initChangeDetectorIfExisting, locateHostElement, renderComponentOrTemplate} from './instructions';
import {ComponentDef, ComponentType} from './interfaces/definition';
import {LElementNode} from './interfaces/node';
import {RElement, RendererFactory3, domRendererFactory3} from './interfaces/renderer';
Expand All @@ -22,6 +22,7 @@ import {stringify} from './util';
import {createViewRef} from './view_ref';



/** Options that control how the component should be bootstrapped. */
export interface CreateComponentOptions {
/** Which renderer factory to use. */
Expand Down Expand Up @@ -135,7 +136,7 @@ export function renderComponent<T>(
// Create element node at index 0 in data array
elementNode = hostElement(hostNode, componentDef);
// Create directive instance with n() and store at index 1 in data array (el is 0)
component = rootContext.component = directiveCreate(1, componentDef.n(), componentDef) as T;
component = rootContext.component = baseDirectiveCreate(1, componentDef.n(), componentDef) as T;
initChangeDetectorIfExisting(elementNode.nodeInjector, component);
} finally {
// We must not use leaveView here because it will set creationMode to false too early,
Expand Down Expand Up @@ -164,6 +165,9 @@ export function renderComponent<T>(
*/
export function LifecycleHooksFeature(component: any, def: ComponentDef<any>): void {
const elementNode = _getComponentHostLElementNode(component);

// Root component is always created at dir index 1, after host element at 0
queueInitHooks(1, def.onInit, def.doCheck, elementNode.view.tView);
queueLifecycleHooks(elementNode.flags, elementNode.view);
}

Expand Down
42 changes: 28 additions & 14 deletions packages/core/src/render3/instructions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1052,6 +1052,34 @@ export function textBinding<T>(index: number, value: T | NO_CHANGE): void {
export function directiveCreate<T>(
index: number, directive: T, directiveDef: DirectiveDef<T>,
localNames?: (string | number)[] | null): T {
const instance = baseDirectiveCreate(index, directive, directiveDef);

ngDevMode && assertNotNull(previousOrParentNode.tNode, 'previousOrParentNode.tNode');
const tNode: TNode|null = previousOrParentNode.tNode !;

if (currentView.tView.firstTemplatePass && localNames) {
tNode.localNames = tNode.localNames ? tNode.localNames.concat(localNames) : localNames;
}

if (tNode && tNode.attrs) {
setInputsFromAttrs<T>(instance, directiveDef !.inputs, tNode);
}

// Init hooks are queued now so ngOnInit is called in host components before
// any projected components.
queueInitHooks(index, directiveDef.onInit, directiveDef.doCheck, currentView.tView);

return instance;
}

/**
* A lighter version of directiveCreate() that is used for the root component
*
* This version does not contain features that we don't already support at root in
* current Angular. Example: local refs and inputs on root component.
*/
export function baseDirectiveCreate<T>(
index: number, directive: T, directiveDef: DirectiveDef<T>): T {
let instance;
ngDevMode &&
assertNull(currentView.bindingStartIndex, 'directives should be created before any bindings');
Expand All @@ -1073,11 +1101,6 @@ export function directiveCreate<T>(

if (index >= tData.length) {
tData[index] = directiveDef !;
if (localNames) {
ngDevMode && assertNotNull(previousOrParentNode.tNode, 'previousOrParentNode.tNode');
const tNode = previousOrParentNode !.tNode !;
tNode.localNames = tNode.localNames ? tNode.localNames.concat(localNames) : localNames;
}
}

const diPublic = directiveDef !.diPublic;
Expand All @@ -1091,15 +1114,6 @@ export function directiveCreate<T>(
(previousOrParentNode as LElementNode).native, directiveDef !.attributes as string[]);
}

const tNode: TNode|null = previousOrParentNode.tNode !;
if (tNode && tNode.attrs) {
setInputsFromAttrs<T>(instance, directiveDef !.inputs, tNode);
}

// Init hooks are queued now so ngOnInit is called in host components before
// any projected components.
queueInitHooks(index, directiveDef.onInit, directiveDef.doCheck, currentView.tView);

return instance;
}

Expand Down
15 changes: 3 additions & 12 deletions packages/core/test/bundling/hello_world/bundle.golden_symbols.json
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@
{
"name": "appendChild"
},
{
"name": "baseDirectiveCreate"
},
{
"name": "bindingUpdated"
},
Expand Down Expand Up @@ -74,9 +77,6 @@
{
"name": "detectChangesInternal"
},
{
"name": "directiveCreate"
},
{
"name": "domRendererFactory3"
},
Expand All @@ -98,9 +98,6 @@
{
"name": "findNextRNodeSibling"
},
{
"name": "generateInitialInputs"
},
{
"name": "getDirectiveInstance"
},
Expand Down Expand Up @@ -149,9 +146,6 @@
{
"name": "noop$2"
},
{
"name": "queueInitHooks"
},
{
"name": "refreshChildComponents"
},
Expand All @@ -173,9 +167,6 @@
{
"name": "resolveRendererType2"
},
{
"name": "setInputsFromAttrs"
},
{
"name": "setUpAttributes"
},
Expand Down
26 changes: 13 additions & 13 deletions packages/core/test/render3/change_detection_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {withBody} from '@angular/core/testing';

import {ChangeDetectionStrategy, ChangeDetectorRef, DoCheck} from '../../src/core';
import {getRenderedText, whenRendered} from '../../src/render3/component';
import {defineComponent, defineDirective, injectChangeDetectorRef} from '../../src/render3/index';
import {LifecycleHooksFeature, defineComponent, defineDirective, injectChangeDetectorRef} from '../../src/render3/index';
import {bind, container, containerRefreshEnd, containerRefreshStart, detectChanges, elementEnd, elementProperty, elementStart, embeddedViewEnd, embeddedViewStart, interpolation1, interpolation2, listener, markDirty, text, textBinding, tick} from '../../src/render3/instructions';

import {containerEl, renderComponent, requestAnimationFrame} from './render_util';
Expand Down Expand Up @@ -39,7 +39,7 @@ describe('change detection', () => {
}

it('should mark a component dirty and schedule change detection', withBody('my-comp', () => {
const myComp = renderComponent(MyComponent);
const myComp = renderComponent(MyComponent, {hostFeatures: [LifecycleHooksFeature]});
expect(getRenderedText(myComp)).toEqual('works');
myComp.value = 'updated';
markDirty(myComp);
Expand All @@ -49,7 +49,7 @@ describe('change detection', () => {
}));

it('should detectChanges on a component', withBody('my-comp', () => {
const myComp = renderComponent(MyComponent);
const myComp = renderComponent(MyComponent, {hostFeatures: [LifecycleHooksFeature]});
expect(getRenderedText(myComp)).toEqual('works');
myComp.value = 'updated';
detectChanges(myComp);
Expand All @@ -58,7 +58,7 @@ describe('change detection', () => {

it('should detectChanges only once if markDirty is called multiple times',
withBody('my-comp', () => {
const myComp = renderComponent(MyComponent);
const myComp = renderComponent(MyComponent, {hostFeatures: [LifecycleHooksFeature]});
expect(getRenderedText(myComp)).toEqual('works');
expect(myComp.doCheckCount).toBe(1);
myComp.value = 'ignore';
Expand All @@ -72,7 +72,7 @@ describe('change detection', () => {
}));

it('should notify whenRendered', withBody('my-comp', async() => {
const myComp = renderComponent(MyComponent);
const myComp = renderComponent(MyComponent, {hostFeatures: [LifecycleHooksFeature]});
await whenRendered(myComp);
myComp.value = 'updated';
markDirty(myComp);
Expand Down Expand Up @@ -352,7 +352,7 @@ describe('change detection', () => {

it('should check the component view when called by component (even when OnPush && clean)',
() => {
const comp = renderComponent(MyComp);
const comp = renderComponent(MyComp, {hostFeatures: [LifecycleHooksFeature]});
expect(getRenderedText(comp)).toEqual('Nancy');

comp.name = 'Bess'; // as this is not an Input, the component stays clean
Expand All @@ -361,7 +361,7 @@ describe('change detection', () => {
});

it('should NOT call component doCheck when called by a component', () => {
const comp = renderComponent(MyComp);
const comp = renderComponent(MyComp, {hostFeatures: [LifecycleHooksFeature]});
expect(comp.doCheckCount).toEqual(1);

// NOTE: in current Angular, detectChanges does not itself trigger doCheck, but you
Expand All @@ -372,7 +372,7 @@ describe('change detection', () => {
});

it('should NOT check the component parent when called by a child component', () => {
const parentComp = renderComponent(ParentComp);
const parentComp = renderComponent(ParentComp, {hostFeatures: [LifecycleHooksFeature]});
expect(getRenderedText(parentComp)).toEqual('1 - Nancy');

parentComp.doCheckCount = 100;
Expand All @@ -383,7 +383,7 @@ describe('change detection', () => {

it('should check component children when called by component if dirty or check-always',
() => {
const parentComp = renderComponent(ParentComp);
const parentComp = renderComponent(ParentComp, {hostFeatures: [LifecycleHooksFeature]});
expect(parentComp.doCheckCount).toEqual(1);

myComp.name = 'Bess';
Expand All @@ -395,7 +395,7 @@ describe('change detection', () => {
});

it('should not group detectChanges calls (call every time)', () => {
const parentComp = renderComponent(ParentComp);
const parentComp = renderComponent(ParentComp, {hostFeatures: [LifecycleHooksFeature]});
expect(myComp.doCheckCount).toEqual(1);

parentComp.cdr.detectChanges();
Expand Down Expand Up @@ -533,7 +533,7 @@ describe('change detection', () => {
});
}

const comp = renderComponent(DetectChangesComp);
const comp = renderComponent(DetectChangesComp, {hostFeatures: [LifecycleHooksFeature]});
expect(getRenderedText(comp)).toEqual('1');
});

Expand Down Expand Up @@ -562,7 +562,7 @@ describe('change detection', () => {
});
}

const comp = renderComponent(DetectChangesComp);
const comp = renderComponent(DetectChangesComp, {hostFeatures: [LifecycleHooksFeature]});
expect(getRenderedText(comp)).toEqual('1');
});

Expand Down Expand Up @@ -954,7 +954,7 @@ describe('change detection', () => {
}

it('should throw if bindings in current view have changed', () => {
const comp = renderComponent(NoChangesComp);
const comp = renderComponent(NoChangesComp, {hostFeatures: [LifecycleHooksFeature]});

expect(() => comp.cdr.checkNoChanges()).not.toThrow();

Expand Down
17 changes: 16 additions & 1 deletion packages/core/test/render3/integration_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import {defineComponent, defineDirective} from '../../src/render3/index';
import {NO_CHANGE, bind, container, containerRefreshEnd, containerRefreshStart, elementAttribute, elementClassNamed, elementEnd, elementProperty, elementStart, elementStyleNamed, embeddedViewEnd, embeddedViewStart, interpolation1, interpolation2, interpolation3, interpolation4, interpolation5, interpolation6, interpolation7, interpolation8, interpolationV, load, projection, projectionDef, text, textBinding} from '../../src/render3/instructions';

import {containerEl, renderToHtml} from './render_util';
import {ComponentFixture, containerEl, renderToHtml} from './render_util';

describe('render3 integration test', () => {

Expand Down Expand Up @@ -274,6 +274,21 @@ describe('render3 integration test', () => {
expect(renderToHtml(Template, {})).toEqual('<todo title="two">two</todo>');
});

it('should support root component with host attribute', () => {
class HostAttributeComp {
static ngComponentDef = defineComponent({
type: HostAttributeComp,
tag: 'host-attr-comp',
factory: () => new HostAttributeComp(),
template: (ctx: HostAttributeComp, cm: boolean) => {},
attributes: ['role', 'button']
});
}

const fixture = new ComponentFixture(HostAttributeComp);
expect(fixture.hostElement.getAttribute('role')).toEqual('button');
});

it('should support component with bindings in template', () => {
/** <p> {{ name }} </p>*/
class MyComp {
Expand Down

0 comments on commit 8353b5c

Please sign in to comment.