fix(ivy): instantiate dirs in correct order (#23178)

PR Close #23178
This commit is contained in:
Kara Erickson
2018-04-04 21:21:12 -07:00
committed by Igor Minar
parent d80e9304c6
commit 628303d2cb
8 changed files with 613 additions and 145 deletions

View File

@ -19,7 +19,7 @@ import {EmbeddedViewRef as viewEngine_EmbeddedViewRef, ViewRef as viewEngine_Vie
import {Type} from '../type';
import {assertLessThan, assertNotNull} from './assert';
import {addToViewTree, assertPreviousIsParent, createLContainer, createLNodeObject, getDirectiveInstance, getPreviousOrParentNode, getRenderer, isComponent, renderEmbeddedTemplate} from './instructions';
import {addToViewTree, assertPreviousIsParent, createLContainer, createLNodeObject, getDirectiveInstance, getPreviousOrParentNode, getRenderer, isComponent, renderEmbeddedTemplate, resolveDirective} from './instructions';
import {ComponentTemplate, DirectiveDef} from './interfaces/definition';
import {LInjector} from './interfaces/injector';
import {LContainerNode, LElementNode, LNode, LNodeType, LViewNode, TNodeFlags} from './interfaces/node';
@ -404,8 +404,15 @@ export function getOrCreateInjectable<T>(
}
}
// If we *didn't* find the directive for the token from the candidate injector, we had a false
// positive. Traverse up the tree and continue.
// If we *didn't* find the directive for the token and we are searching the current node's
// injector, it's possible the directive is on this node and hasn't been created yet.
let instance: T|null;
if (injector === di && (instance = searchMatchesQueuedForCreation<T>(node, token))) {
return instance;
}
// The def wasn't found anywhere on this node, so it might be a false positive.
// Traverse up the tree and continue searching.
injector = injector.parent;
}
}
@ -415,6 +422,19 @@ export function getOrCreateInjectable<T>(
throw createInjectionError('Not found', token);
}
function searchMatchesQueuedForCreation<T>(node: LNode, token: any): T|null {
const matches = node.view.tView.currentMatches;
if (matches) {
for (let i = 0; i < matches.length; i += 2) {
const def = matches[i] as DirectiveDef<any>;
if (def.type === token) {
return resolveDirective(def, i + 1, matches, node.view.tView);
}
}
}
return null;
}
/**
* Given a directive type, this function returns the bit in an injector's bloom filter
* that should be used to determine whether or not the directive is present.

View File

@ -0,0 +1,35 @@
/**
* @license
* Copyright Google Inc. All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/
import {TNode} from './interfaces/node';
/** Called when directives inject each other (creating a circular dependency) */
export function throwCyclicDependencyError(token: any): never {
throw new Error(`Cannot instantiate cyclic dependency! ${token}`);
}
/** Called when there are multiple component selectors that match a given node */
export function throwMultipleComponentError(tNode: TNode): never {
throw new Error(`Multiple components match node with tagname ${tNode.tagName}`);
}
/** Throws an ExpressionChangedAfterChecked error if checkNoChanges mode is on. */
export function throwErrorIfNoChangesMode(
creationMode: boolean, checkNoChangesMode: boolean, oldValue: any, currValue: any): never|void {
if (checkNoChangesMode) {
let msg =
`ExpressionChangedAfterItHasBeenCheckedError: Expression has changed after it was checked. Previous value: '${oldValue}'. Current value: '${currValue}'.`;
if (creationMode) {
msg +=
` It seems like the view has been created after its parent and its children have been dirty checked.` +
` Has it been created in a change detection hook ?`;
}
// TODO: include debug context
throw new Error(msg);
}
}

View File

@ -13,7 +13,7 @@ import {LContainer, TContainer} from './interfaces/container';
import {LInjector} from './interfaces/injector';
import {CssSelectorList, LProjection, NG_PROJECT_AS_ATTR_NAME} from './interfaces/projection';
import {LQueries} from './interfaces/query';
import {LView, LViewFlags, LifecycleStage, RootContext, TData, TView} from './interfaces/view';
import {CurrentMatchesList, LView, LViewFlags, LifecycleStage, RootContext, TData, TView} from './interfaces/view';
import {LContainerNode, LElementNode, LNode, LNodeType, TNodeFlags, LProjectionNode, LTextNode, LViewNode, TNode, TContainerNode, InitialInputData, InitialInputs, PropertyAliases, PropertyAliasValue,} from './interfaces/node';
import {assertNodeType} from './node_assert';
@ -24,6 +24,7 @@ import {RElement, RText, Renderer3, RendererFactory3, ProceduralRenderer3, Objec
import {isDifferent, stringify} from './util';
import {executeHooks, queueLifecycleHooks, queueInitHooks, executeInitHooks} from './hooks';
import {ViewRef} from './view_ref';
import {throwCyclicDependencyError, throwErrorIfNoChangesMode, throwMultipleComponentError} from './errors';
/**
* Directive (D) sets a property on all component instances using this constant as a key and the
@ -50,6 +51,13 @@ export type Sanitizer = (value: any) => string;
*/
export const _ROOT_DIRECTIVE_INDICES = [0, 0];
/**
* Token set in currentMatches while dependencies are being resolved.
*
* If we visit a directive that has a value set to CIRCULAR, we know we've
* already seen it, and thus have a circular dependency.
*/
export const CIRCULAR = '__CIRCULAR__';
/**
* This property gets set before entering a template.
@ -528,66 +536,93 @@ export function elementStart(
if (attrs) setUpAttributes(native, attrs);
appendChild(node.parent !, native, currentView);
if (firstTemplatePass) {
const tNode = createTNode(name, attrs || null, null);
cacheMatchingDirectivesForNode(tNode);
ngDevMode && assertDataInRange(index - 1);
node.tNode = tData[index] = tNode;
}
hack_declareDirectives(index, localRefs || null);
createDirectivesAndLocals(index, name, attrs, localRefs, null);
return native;
}
function cacheMatchingDirectivesForNode(tNode: TNode): void {
const tView = currentView.tView;
const registry = tView.directiveRegistry;
function createDirectivesAndLocals(
index: number, name: string | null, attrs: string[] | null | undefined,
localRefs: string[] | null | undefined, containerData: TView[] | null) {
const node = previousOrParentNode;
if (firstTemplatePass) {
ngDevMode && assertDataInRange(index - 1);
node.tNode = tData[index] = createTNode(name, attrs || null, containerData);
cacheMatchingDirectivesForNode(node.tNode, currentView.tView, localRefs || null);
} else {
instantiateDirectivesDirectly();
}
saveResolvedLocalsInData();
}
/**
* On first template pass, we match each node against available directive selectors and save
* the resulting defs in the correct instantiation order for subsequent change detection runs
* (so dependencies are always created before the directives that inject them).
*/
function cacheMatchingDirectivesForNode(
tNode: TNode, tView: TView, localRefs: string[] | null): void {
const exportsMap = localRefs ? {'': -1} : null;
const matches = tView.currentMatches = findDirectiveMatches(tNode);
if (matches) {
for (let i = 0; i < matches.length; i += 2) {
const def = matches[i] as DirectiveDef<any>;
const valueIndex = i + 1;
resolveDirective(def, valueIndex, matches, tView);
saveNameToExportMap(matches[valueIndex] as number, def, exportsMap);
}
}
if (exportsMap) cacheMatchingLocalNames(tNode, localRefs, exportsMap);
}
/** Matches the current node against all available selectors. */
function findDirectiveMatches(tNode: TNode): CurrentMatchesList|null {
const registry = currentView.tView.directiveRegistry;
let matches: any[]|null = null;
if (registry) {
let componentFlag = 0;
let size = 0;
for (let i = 0; i < registry.length; i++) {
const def = registry[i];
if (isNodeMatchingSelectorList(tNode, def.selectors !)) {
if ((def as ComponentDef<any>).template) {
if (componentFlag) throwMultipleComponentError(tNode);
componentFlag |= TNodeFlags.Component;
if (tNode.flags & TNodeFlags.Component) throwMultipleComponentError(tNode);
tNode.flags = TNodeFlags.Component;
}
(tView.directives || (tView.directives = [])).push(def);
size++;
if (def.diPublic) def.diPublic(def);
(matches || (matches = [])).push(def, null);
}
}
if (size > 0) {
const startIndex = directives ? directives.length : 0;
buildTNodeFlags(tNode, startIndex, size, componentFlag);
}
}
return matches as CurrentMatchesList;
}
function buildTNodeFlags(tNode: TNode, index: number, size: number, component: number): void {
tNode.flags = (index << TNodeFlags.INDX_SHIFT) | (size << TNodeFlags.SIZE_SHIFT) | component;
}
function throwMultipleComponentError(tNode: TNode): never {
throw new Error(`Multiple components match node with tagname ${tNode.tagName}`);
export function resolveDirective(
def: DirectiveDef<any>, valueIndex: number, matches: CurrentMatchesList, tView: TView): any {
if (matches[valueIndex] === null) {
matches[valueIndex] = CIRCULAR;
const instance = def.factory();
(tView.directives || (tView.directives = [])).push(def);
return directiveCreate(matches[valueIndex] = tView.directives !.length - 1, instance, def);
} else if (matches[valueIndex] === CIRCULAR) {
// If we revisit this directive before it's resolved, we know it's circular
throwCyclicDependencyError(def.type);
}
return null;
}
/** Stores index of component's host element so it will be queued for view refresh during CD. */
function queueComponentIndexForCheck(dirIndex: number, elIndex: number): void {
function queueComponentIndexForCheck(dirIndex: number): void {
if (firstTemplatePass) {
(currentView.tView.components || (currentView.tView.components = [])).push(dirIndex, elIndex);
(currentView.tView.components || (currentView.tView.components = [
])).push(dirIndex, data.length - 1);
}
}
/** Stores index of directive and host element so it will be queued for binding refresh during CD.
*/
function queueHostBindingForCheck(dirIndex: number, elIndex: number): void {
function queueHostBindingForCheck(dirIndex: number): void {
ngDevMode &&
assertEqual(firstTemplatePass, true, 'Should only be called in first template pass.');
(currentView.tView.hostBindings || (currentView.tView.hostBindings = [])).push(dirIndex, elIndex);
(currentView.tView.hostBindings || (currentView.tView.hostBindings = [
])).push(dirIndex, data.length - 1);
}
/** Sets the context for a ChangeDetectorRef to the given instance. */
@ -603,32 +638,21 @@ export function isComponent(tNode: TNode): boolean {
}
/**
* This function instantiates the given directives. It is a hack since it assumes the directives
* come in the correct order for DI.
* This function instantiates the given directives.
*/
function hack_declareDirectives(elementIndex: number, localRefs: string[] | null) {
function instantiateDirectivesDirectly() {
const tNode = previousOrParentNode.tNode !;
const size = (tNode.flags & TNodeFlags.SIZE_MASK) >> TNodeFlags.SIZE_SHIFT;
const exportsMap: {[key: string]: number}|null = firstTemplatePass && localRefs ? {'': -1} : null;
if (size > 0) {
let startIndex = tNode.flags >> TNodeFlags.INDX_SHIFT;
const endIndex = startIndex + size;
const startIndex = tNode.flags >> TNodeFlags.INDX_SHIFT;
const tDirectives = currentView.tView.directives !;
// 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 = startIndex; i < endIndex; i++) {
for (let i = startIndex; i < startIndex + size; i++) {
const def = tDirectives[i] as DirectiveDef<any>;
directiveCreate(elementIndex, def.factory(), def);
saveNameToExportMap(startIndex, def, exportsMap);
startIndex++;
directiveCreate(i, def.factory(), def);
}
}
if (firstTemplatePass) cacheMatchingLocalNames(tNode, localRefs, exportsMap !);
saveResolvedLocalsInData();
}
/** Caches local names and their matching directive indices for query and template lookups. */
@ -710,7 +734,8 @@ export function createTView(
hostBindings: null,
components: null,
directiveRegistry: typeof defs === 'function' ? defs() : defs,
pipeRegistry: typeof pipes === 'function' ? pipes() : pipes
pipeRegistry: typeof pipes === 'function' ? pipes() : pipes,
currentMatches: null
};
}
@ -777,8 +802,8 @@ export function hostElement(
if (firstTemplatePass) {
node.tNode = createTNode(tag as string, null, null);
// Root directive is stored at index 0, size 1
buildTNodeFlags(node.tNode, 0, 1, TNodeFlags.Component);
node.tNode.flags = TNodeFlags.Component;
if (def.diPublic) def.diPublic(def);
currentView.tView.directives = [def];
}
@ -1167,14 +1192,11 @@ export function textBinding<T>(index: number, value: T | NO_CHANGE): void {
* NOTE: directives can be created in order other than the index order. They can also
* be retrieved before they are created in which case the value will be null.
*
* @param elementIndex Index of the host element in the data array
* @param directive The directive instance.
* @param directiveDef DirectiveDef object which contains information about the template.
* @param localRefs Names under which a query can retrieve the directive instance
*/
export function directiveCreate<T>(
elementIndex: number, directive: T, directiveDef: DirectiveDef<T>| ComponentDef<T>): T {
const index = directives ? directives.length : 0;
index: number, directive: T, directiveDef: DirectiveDef<T>| ComponentDef<T>): T {
const instance = baseDirectiveCreate(index, directive, directiveDef);
ngDevMode && assertNotNull(previousOrParentNode.tNode, 'previousOrParentNode.tNode');
@ -1182,7 +1204,7 @@ export function directiveCreate<T>(
const isComponent = (directiveDef as ComponentDef<T>).template;
if (isComponent) {
addComponentLogic(index, elementIndex, directive, directiveDef as ComponentDef<T>);
addComponentLogic(index, directive, directiveDef as ComponentDef<T>);
}
if (firstTemplatePass) {
@ -1190,7 +1212,7 @@ export function directiveCreate<T>(
// any projected components.
queueInitHooks(index, directiveDef.onInit, directiveDef.doCheck, currentView.tView);
if (directiveDef.hostBindings) queueHostBindingForCheck(index, elementIndex);
if (directiveDef.hostBindings) queueHostBindingForCheck(index);
}
if (tNode && tNode.attrs) {
@ -1200,8 +1222,7 @@ export function directiveCreate<T>(
return instance;
}
function addComponentLogic<T>(
index: number, elementIndex: number, instance: T, def: ComponentDef<T>): void {
function addComponentLogic<T>(index: number, instance: T, def: ComponentDef<T>): void {
const tView = getOrCreateTView(def.template, def.directiveDefs, def.pipeDefs);
// Only component views should be added to the view tree directly. Embedded views are
@ -1217,7 +1238,7 @@ function addComponentLogic<T>(
initChangeDetectorIfExisting(previousOrParentNode.nodeInjector, instance, hostView);
if (firstTemplatePass) queueComponentIndexForCheck(index, elementIndex);
if (firstTemplatePass) queueComponentIndexForCheck(index);
}
/**
@ -1240,9 +1261,14 @@ export function baseDirectiveCreate<T>(
ngDevMode && assertDataNext(index, directives);
directives[index] = directive;
const diPublic = directiveDef !.diPublic;
if (diPublic) {
diPublic(directiveDef !);
if (firstTemplatePass) {
const flags = previousOrParentNode.tNode !.flags;
previousOrParentNode.tNode !.flags = (flags & TNodeFlags.SIZE_MASK) === 0 ?
(index << TNodeFlags.INDX_SHIFT) | TNodeFlags.SIZE_SKIP | flags & TNodeFlags.Component :
flags + TNodeFlags.SIZE_SKIP;
} else {
const diPublic = directiveDef !.diPublic;
if (diPublic) diPublic(directiveDef !);
}
if (directiveDef !.attributes != null && previousOrParentNode.type == LNodeType.Element) {
@ -1355,18 +1381,10 @@ export function container(
const node = createLNode(index, LNodeType.Container, undefined, lContainer);
if (node.tNode == null) {
node.tNode = tData[index] = createTNode(tagName || null, attrs || null, []);
}
// Containers are added to the current view tree instead of their embedded views
// because views can be removed and re-inserted.
addToViewTree(currentView, node.data);
if (firstTemplatePass) cacheMatchingDirectivesForNode(node.tNode);
// TODO: handle TemplateRef!
hack_declareDirectives(index, localRefs || null);
createDirectivesAndLocals(index, tagName || null, attrs, localRefs, []);
isParent = false;
ngDevMode && assertNodeType(previousOrParentNode, LNodeType.Container);
@ -1892,22 +1910,6 @@ export function checkNoChanges<T>(component: T): void {
}
}
/** Throws an ExpressionChangedAfterChecked error if checkNoChanges mode is on. */
function throwErrorIfNoChangesMode(oldValue: any, currValue: any): never|void {
if (checkNoChangesMode) {
let msg =
`ExpressionChangedAfterItHasBeenCheckedError: Expression has changed after it was checked. Previous value: '${oldValue}'. Current value: '${currValue}'.`;
if (creationMode) {
msg +=
` It seems like the view has been created after its parent and its children have been dirty checked.` +
` Has it been created in a change detection hook ?`;
}
// TODO: include debug context
throw new Error(msg);
}
}
/** Checks the view of the component provided. Does not gate on dirty checks or execute doCheck. */
export function detectChangesInternal<T>(
hostView: LView, hostNode: LElementNode, def: ComponentDef<T>, component: T) {
@ -1985,7 +1987,7 @@ export function bind<T>(value: T | NO_CHANGE): T|NO_CHANGE {
const changed: boolean = value !== NO_CHANGE && isDifferent(data[bindingIndex], value);
if (changed) {
throwErrorIfNoChangesMode(data[bindingIndex], value);
throwErrorIfNoChangesMode(creationMode, checkNoChangesMode, data[bindingIndex], value);
data[bindingIndex] = value;
}
bindingIndex++;
@ -2165,7 +2167,7 @@ export function bindingUpdated(value: any): boolean {
if (creationMode) {
initBindings();
} else if (isDifferent(data[bindingIndex], value)) {
throwErrorIfNoChangesMode(data[bindingIndex], value);
throwErrorIfNoChangesMode(creationMode, checkNoChangesMode, data[bindingIndex], value);
} else {
bindingIndex++;
return false;

View File

@ -42,6 +42,9 @@ export const enum TNodeFlags {
/** How far to shift the flags to get the number of directives on this node */
SIZE_SHIFT = 1,
/** The amount to add to flags to increment size when each directive is added */
SIZE_SKIP = 2,
/** Mask to get the number of directives on this node */
SIZE_MASK = 0b00000000000000000001111111111110
}

View File

@ -7,7 +7,7 @@
*/
import {LContainer} from './container';
import {ComponentTemplate, DirectiveDefList, PipeDef, PipeDefList} from './definition';
import {ComponentTemplate, DirectiveDef, DirectiveDefList, PipeDef, PipeDefList} from './definition';
import {LElementNode, LViewNode, TNode} from './node';
import {LQueries} from './query';
import {Renderer3} from './renderer';
@ -225,6 +225,24 @@ export interface TView {
/** Static data equivalent of LView.data[]. Contains TNodes. */
data: TData;
/**
* Selector matches for a node are temporarily cached on the TView so the
* DI system can eagerly instantiate directives on the same node if they are
* created out of order. They are overwritten after each node.
*
* <div dirA dirB></div>
*
* e.g. DirA injects DirB, but DirA is created first. DI should instantiate
* DirB when it finds that it's on the same node, but not yet created.
*
* Even indices: Directive defs
* Odd indices:
* - Null if the associated directive hasn't been instantiated yet
* - Directive index, if associated directive has been created
* - String, temporary 'CIRCULAR' token set while dependencies are being resolved
*/
currentMatches: CurrentMatchesList|null;
/**
* Directive and component defs that have already been matched to nodes on
* this view.
@ -397,6 +415,9 @@ export const enum LifecycleStage {
*/
export type TData = (TNode | PipeDef<any>| null)[];
/** Type for TView.currentMatches */
export type CurrentMatchesList = [DirectiveDef<any>, (string | number | null)];
// Note: This hack is necessary so we don't erroneously get a circular dependency
// failure based on types.
export const unusedValueExportToPlacateAjd = 1;