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

refactor(ivy): move hostBindings calls out of template #22833

Closed
wants to merge 2 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
2 changes: 0 additions & 2 deletions modules/benchmarks/src/tree/render3/tree.ts
Expand Up @@ -58,7 +58,6 @@ export class TreeComponent {
e();
}
p(0, 'data', b(ctx.data.left));
TreeComponent.ngComponentDef.h(1, 0);
}
v();
}
Expand All @@ -74,7 +73,6 @@ export class TreeComponent {
e();
}
p(0, 'data', b(ctx.data.right));
TreeComponent.ngComponentDef.h(1, 0);
}
v();
}
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/render3/component.ts
Expand Up @@ -133,7 +133,7 @@ export function renderComponent<T>(
const elementNode = hostElement(hostNode, componentDef);
// Create directive instance with n() and store at index 1 in data array (el is 0)
component = rootContext.component =
getDirectiveInstance(directiveCreate(1, componentDef.n(), componentDef));
getDirectiveInstance(directiveCreate(1, componentDef.factory(), componentDef));
initChangeDetectorIfExisting(elementNode.nodeInjector, component);
} finally {
// We must not use leaveView here because it will set creationMode to false too early,
Expand Down
6 changes: 2 additions & 4 deletions packages/core/src/render3/definition.ts
Expand Up @@ -39,10 +39,10 @@ export function defineComponent<T>(componentDefinition: ComponentDefArgs<T>): Co
const def = <ComponentDef<any>>{
type: type,
diPublic: null,
n: componentDefinition.factory,
factory: componentDefinition.factory,
tag: (componentDefinition as ComponentDefArgs<T>).tag || null !,
template: (componentDefinition as ComponentDefArgs<T>).template || null !,
h: componentDefinition.hostBindings || noop,
hostBindings: componentDefinition.hostBindings || null,
attributes: componentDefinition.attributes || null,
inputs: invertObject(componentDefinition.inputs),
outputs: invertObject(componentDefinition.outputs),
Expand Down Expand Up @@ -158,8 +158,6 @@ export function PublicFeature<T>(definition: DirectiveDef<T>) {

const EMPTY = {};

function noop() {}

/** Swaps the keys and values of an object. */
function invertObject(obj: any): any {
if (obj == null) return EMPTY;
Expand Down
141 changes: 88 additions & 53 deletions packages/core/src/render3/instructions.ts
Expand Up @@ -43,6 +43,13 @@ const _CLEAN_PROMISE = Promise.resolve(null);
*/
export type Sanitizer = (value: any) => string;

/**
* Directive and element indices for top-level directive.
*
* Saved here to avoid re-instantiating an array on every change detection run.
*/
const rootDirectiveIndices = [1, 0];


/**
* This property gets set before entering a template.
Expand Down Expand Up @@ -158,6 +165,9 @@ let cleanup: any[]|null;
*/
let checkNoChangesMode = false;

/** Whether or not this is the first time the current view has been processed. */
let firstTemplatePass = true;

const enum BindingDirection {
Input,
Output,
Expand All @@ -181,6 +191,7 @@ export function enterView(newView: LView, host: LElementNode | LViewNode | null)
bindingIndex = newView && newView.bindingStartIndex || 0;
tData = newView && newView.tView.data;
creationMode = newView && (newView.flags & LViewFlags.CreationMode) === LViewFlags.CreationMode;
firstTemplatePass = newView && newView.tView.firstTemplatePass;

cleanup = newView && newView.cleanup;
renderer = newView && newView.renderer;
Expand Down Expand Up @@ -212,13 +223,33 @@ export function leaveView(newView: LView): void {
enterView(newView, null);
}

/** Refreshes the views of child components, triggering any init/content hooks existing. */
function refreshChildComponents() {
/** Refreshes directives in this view and triggers any init/content hooks. */
function refreshDirectives() {
executeInitAndContentHooks();

const tView = currentView.tView;
// This needs to be set before children are processed to support recursive components
currentView.tView.firstTemplatePass = false;
// so to refresh the component, refresh() needs to be called with (1, 0)
tView.firstTemplatePass = firstTemplatePass = false;

setHostBindings(tView.hostBindings);
refreshChildComponents(tView.components);
}

const components = currentView.tView.components;
/** Sets the host bindings for the current view. */
function setHostBindings(bindings: number[] | null): void {
if (bindings != null) {
for (let i = 0; i < bindings.length; i += 2) {
const dirIndex = bindings[i];
const elementIndex = bindings[i | 1];
const def = tData[dirIndex] as DirectiveDef<any>;
def.hostBindings && def.hostBindings(dirIndex, elementIndex);
}
}
}

/** Refreshes child components in the current view. */
function refreshChildComponents(components: number[] | null): void {
if (components != null) {
for (let i = 0; i < components.length; i++) {
componentRefresh(components[i] + 1, components[i]);
Expand Down Expand Up @@ -398,7 +429,7 @@ export function renderEmbeddedTemplate<T>(

template(context, cm);
refreshDynamicChildren();
refreshChildComponents();
refreshDirectives();
} finally {
leaveView(currentView !.parent !);
isParent = _isParent;
Expand All @@ -416,11 +447,12 @@ export function renderComponentOrTemplate<T>(
}
if (template) {
template(componentOrContext !, creationMode);
refreshChildComponents();
refreshDirectives();
} else {
executeInitAndContentHooks();

// Element was stored at 0 and directive was stored at 1 in renderComponent
// so to refresh the component, refresh() needs to be called with (1, 0)
setHostBindings(rootDirectiveIndices);
componentRefresh(1, 0);
}
} finally {
Expand Down Expand Up @@ -466,7 +498,7 @@ export function elementStart(
let hostComponentDef: ComponentDef<any>|null = null;
let name = nameOrComponentType as string;
if (isHostElement) {
hostComponentDef = currentView.tView.firstTemplatePass ?
hostComponentDef = firstTemplatePass ?
(nameOrComponentType as ComponentType<any>).ngComponentDef :
tData[index + 1] as ComponentDef<any>;
name = hostComponentDef !.tag;
Expand Down Expand Up @@ -501,24 +533,36 @@ export function elementStart(
if (attrs) setUpAttributes(native, attrs);
appendChild(node.parent !, native, currentView);

const elementIndex = index;

if (hostComponentDef) {
// TODO(mhevery): This assumes that the directives come in correct order, which
// is not guaranteed. Must be refactored to take it into account.
const instance = hostComponentDef.n();
storeComponentIndex(index);
const instance = hostComponentDef.factory();
directiveCreate(++index, instance, hostComponentDef, null);
initChangeDetectorIfExisting(node.nodeInjector, instance);
queueComponentIndexForCheck(elementIndex);
if (hostComponentDef.hostBindings) queueHostBindingForCheck(index, elementIndex);
}
hack_declareDirectives(index, directiveTypes, localRefs);
hack_declareDirectives(index, elementIndex, directiveTypes, localRefs);
}
}
return native;
}

/** Stores index of component so it will be queued for refresh during change detection. */
function storeComponentIndex(index: number): void {
if (currentView.tView.firstTemplatePass) {
(currentView.tView.components || (currentView.tView.components = [])).push(index);
/** Stores index of component's host element so it will be queued for view refresh during CD. */
function queueComponentIndexForCheck(elIndex: number): void {
if (firstTemplatePass) {
(currentView.tView.components || (currentView.tView.components = [])).push(elIndex);
}
}

/** Stores index of directive and host element so it will be queued for binding refresh during CD.
*/
function queueHostBindingForCheck(dirIndex: number, elIndex: number): void {
if (firstTemplatePass) {
(currentView.tView.hostBindings || (currentView.tView.hostBindings = [
])).push(dirIndex, elIndex);
}
}

Expand All @@ -534,21 +578,20 @@ export function initChangeDetectorIfExisting(injector: LInjector | null, instanc
* come in the correct order for DI.
*/
function hack_declareDirectives(
index: number, directiveTypes: DirectiveType<any>[] | null | undefined,
index: number, elIndex: number, directiveTypes: DirectiveType<any>[] | null | undefined,
localRefs: string[] | null | undefined, ) {
if (directiveTypes) {
// TODO(mhevery): This assumes that the directives come in correct order, which
// is not guaranteed. Must be refactored to take it into account.
for (let i = 0; i < directiveTypes.length; i++) {
index++;
const directiveType = directiveTypes[i];
const directiveDef = currentView.tView.firstTemplatePass ? directiveType.ngDirectiveDef :
tData[index] as DirectiveDef<any>;
const localNames = currentView.tView.firstTemplatePass ?
findMatchingLocalNames(directiveDef, localRefs, index) :
null;

directiveCreate(index, directiveDef.n(), directiveDef, localNames);
const directiveDef =
firstTemplatePass ? directiveType.ngDirectiveDef : tData[index] as DirectiveDef<any>;
const localNames =
firstTemplatePass ? findMatchingLocalNames(directiveDef, localRefs, index) : null;
directiveCreate(index, directiveDef.factory(), directiveDef, localNames);
if (directiveDef.hostBindings) queueHostBindingForCheck(index, elIndex);
}
}
}
Expand Down Expand Up @@ -598,6 +641,7 @@ export function createTView(): TView {
viewHooks: null,
viewCheckHooks: null,
destroyHooks: null,
hostBindings: null,
components: null
};
}
Expand Down Expand Up @@ -777,12 +821,12 @@ export function elementProperty<T>(
const tNode = node.tNode !;
// if tNode.inputs is undefined, a listener has created outputs, but inputs haven't
// yet been checked
if (tNode.inputs === undefined) {
if (tNode && tNode.inputs === undefined) {
// mark inputs as checked
tNode.inputs = generatePropertyAliases(node.flags, BindingDirection.Input);
}

const inputData = tNode.inputs;
const inputData = tNode && tNode.inputs;
let dataValue: PropertyAliasValue|undefined;
if (inputData && (dataValue = inputData[propName])) {
setInputsForProperty(dataValue, value);
Expand Down Expand Up @@ -1209,7 +1253,7 @@ export function container(
// Containers are added to the current view tree instead of their embedded views
// because views can be removed and re-inserted.
addToViewTree(node.data);
hack_declareDirectives(index, directiveTypes, localRefs);
hack_declareDirectives(index, index, directiveTypes, localRefs);

isParent = false;
ngDevMode && assertNodeType(previousOrParentNode, LNodeFlags.Container);
Expand Down Expand Up @@ -1366,7 +1410,7 @@ function getOrCreateEmbeddedTView(viewIndex: number, parent: LContainerNode): TV

/** Marks the end of an embedded view. */
export function embeddedViewEnd(): void {
refreshChildComponents();
refreshDirectives();
isParent = false;
const viewNode = previousOrParentNode = currentView.node as LViewNode;
const containerNode = previousOrParentNode.parent as LContainerNode;
Expand All @@ -1390,29 +1434,22 @@ export function embeddedViewEnd(): void {
/////////////

/**
* Refreshes the directive.
*
* When it is a component, it also enters the component's view and processes it to update bindings,
* queries, etc.
* Refreshes components by entering the component view and processing its bindings, queries, etc.
*
* @param directiveIndex
* @param elementIndex
*/
export function componentRefresh<T>(directiveIndex: number, elementIndex: number): void {
const template = (tData[directiveIndex] as ComponentDef<T>).template;
if (template != null) {
ngDevMode && assertDataInRange(elementIndex);
const element = data ![elementIndex] as LElementNode;
ngDevMode && assertNodeType(element, LNodeFlags.Element);
ngDevMode &&
assertNotNull(element.data, `Component's host node should have an LView attached.`);
const hostView = element.data !;
ngDevMode && assertDataInRange(elementIndex);
const element = data ![elementIndex] as LElementNode;
ngDevMode && assertNodeType(element, LNodeFlags.Element);
ngDevMode && assertNotNull(element.data, `Component's host node should have an LView attached.`);
const hostView = element.data !;

// Only attached CheckAlways components or attached, dirty OnPush components should be checked
if (viewAttached(hostView) && hostView.flags & (LViewFlags.CheckAlways | LViewFlags.Dirty)) {
ngDevMode && assertDataInRange(directiveIndex);
detectChangesInternal(hostView, element, getDirectiveInstance<T>(data[directiveIndex]));
}
// Only attached CheckAlways components or attached, dirty OnPush components should be checked
if (viewAttached(hostView) && hostView.flags & (LViewFlags.CheckAlways | LViewFlags.Dirty)) {
ngDevMode && assertDataInRange(directiveIndex);
detectChangesInternal(hostView, element, getDirectiveInstance<T>(data[directiveIndex]));
}
}

Expand Down Expand Up @@ -1756,16 +1793,14 @@ function throwErrorIfNoChangesMode(oldValue: any, currValue: any): never|void {
function detectChangesInternal<T>(hostView: LView, hostNode: LElementNode, component: T) {
const componentIndex = hostNode.flags >> LNodeFlags.INDX_SHIFT;
const template = (hostNode.view.tView.data[componentIndex] as ComponentDef<T>).template;
const oldView = enterView(hostView, hostNode);

if (template != null) {
try {
template(component, creationMode);
refreshDynamicChildren();
refreshChildComponents();
} finally {
leaveView(oldView);
}
const oldView = enterView(hostView, hostNode);
try {
template(component, creationMode);
refreshDynamicChildren();
refreshDirectives();
} finally {
leaveView(oldView);
}
}

Expand Down
12 changes: 3 additions & 9 deletions packages/core/src/render3/interfaces/definition.ts
Expand Up @@ -85,17 +85,11 @@ export interface DirectiveDef<T> {
*
* Usually returns the directive instance, but if the directive has a content query,
* it instead returns an array that contains the instance as well as content query data.
*
* NOTE: this property is short (1 char) because it is used in
* component templates which is sensitive to size.
*/
n(): T|[T];
factory(): T|[T];

/**
* Refreshes host bindings on the associated directive. Also calls lifecycle hooks
* like ngOnInit and ngDoCheck, if they are defined on the directive.
*/
h(directiveIndex: number, elementIndex: number): void;
/** Refreshes host bindings on the associated directive. */
hostBindings: ((directiveIndex: number, elementIndex: number) => void)|null;

/**
* Static attributes to set on host element.
Expand Down
8 changes: 8 additions & 0 deletions packages/core/src/render3/interfaces/view.ts
Expand Up @@ -282,6 +282,14 @@ export interface TView {
* current view has finished its check.
*/
components: number[]|null;

/**
* A list of indices for child directives that have host bindings.
*
* Even indices: Directive indices
* Odd indices: Element indices
*/
hostBindings: number[]|null;
}

/**
Expand Down
Expand Up @@ -152,15 +152,15 @@
{
"name": "locateHostElement"
},
{
"name": "noop$2"
},
{
"name": "queueInitHooks"
},
{
"name": "refreshChildComponents"
},
{
"name": "refreshDirectives"
},
{
"name": "refreshDynamicChildren"
},
Expand All @@ -179,6 +179,12 @@
{
"name": "resolveRendererType2"
},
{
"name": "rootDirectiveIndices"
},
{
"name": "setHostBindings"
},
{
"name": "setInputsFromAttrs"
},
Expand Down