cargdev/angular
1
0
Fork 0
Code Issues 5 Pull Requests Actions Packages Projects Releases Wiki Activity

fix(ivy): run pre-order hooks in injection order (#34026)

Browse Source 
This commit fixes a compatibility bug where pre-order lifecycle
hooks (onInit, doCheck, OnChanges) for directives on the same
host node were executed based on the order the directives were
matched, rather than the order the directives were instantiated
(i.e. injection order).

This discrepancy can cause issues with forms, where it is common
to inject NgControl and try to extract its control property in
ngOnInit. As the NgControl directive is injected, it should be
instantiated before the control value accessor directive (and
thus its hooks should run first). This ensures that the NgControl
ngOnInit can set up the form control before the ngOnInit
for the control value accessor tries to access it.

Closes #32522

PR Close #34026
This commit is contained in:
Kara Erickson
2019-11-24 20:56:18 -08:00
committed by Matias Niemelä
parent caf5cffd53
commit ebe3229da5
11 changed files with 384 additions and 45 deletions
Download Patch File Download Diff File Expand all files Collapse all files

19
packages/core/src/render3/di.ts
View File

@ -15,8 +15,10 @@ import {InjectFlags} from '../di/interface/injector';
import {Type} from '../interface/type';
import {assertDefined, assertEqual} from '../util/assert';
import {assertDirectiveDef} from './assert';
import {getFactoryDef} from './definition';
import {NG_ELEMENT_ID, NG_FACTORY_DEF} from './fields';
import {registerPreOrderHooks} from './hooks';
import {DirectiveDef, FactoryFn} from './interfaces/definition';
import {NO_PARENT_INJECTOR, NodeInjectorFactory, PARENT_INJECTOR, RelativeInjectorLocation, RelativeInjectorLocationFlags, TNODE, isFactory} from './interfaces/injector';
import {AttributeMarker, TContainerNode, TDirectiveHostNode, TElementContainerNode, TElementNode, TNode, TNodeProviderIndexes, TNodeType} from './interfaces/node';
@ -471,7 +473,7 @@ function searchTokensOnInjector<T>(
const injectableIdx = locateDirectiveOrProvider(
tNode, currentTView, token, canAccessViewProviders, isHostSpecialCase);
if (injectableIdx !== null) {
return getNodeInjectable(currentTView.data, lView, injectableIdx, tNode as TElementNode);
return getNodeInjectable(lView, currentTView, injectableIdx, tNode as TElementNode);
} else {
return NOT_FOUND;
}
@ -519,15 +521,16 @@ export function locateDirectiveOrProvider<T>(
}
/**
* Retrieve or instantiate the injectable from the `lData` at particular `index`.
* Retrieve or instantiate the injectable from the `LView` at particular `index`.
*
* This function checks to see if the value has already been instantiated and if so returns the
* cached `injectable`. Otherwise if it detects that the value is still a factory it
* instantiates the `injectable` and caches the value.
*/
export function getNodeInjectable(
tData: TData, lView: LView, index: number, tNode: TDirectiveHostNode): any {
lView: LView, tView: TView, index: number, tNode: TDirectiveHostNode): any {
let value = lView[index];
const tData = tView.data;
if (isFactory(value)) {
const factory: NodeInjectorFactory = value;
if (factory.resolving) {
@ -542,6 +545,16 @@ export function getNodeInjectable(
enterDI(lView, tNode);
try {
value = lView[index] = factory.factory(undefined, tData, lView, tNode);
// This code path is hit for both directives and providers.
// For perf reasons, we want to avoid searching for hooks on providers.
// It does no harm to try (the hooks just won't exist), but the extra
// checks are unnecessary and this is a hot path. So we check to see
// if the index of the dependency is in the directive range for this
// tNode. If it's not, we know it's a provider and skip hook registration.
if (tView.firstCreatePass && index >= tNode.directiveStart) {
ngDevMode && assertDirectiveDef(tData[index]);
registerPreOrderHooks(index, tData[index] as DirectiveDef<any>, tView);
}
} finally {
if (factory.injectImpl) setInjectImplementation(previousInjectImplementation);
setIncludeViewProviders(previousIncludeViewProviders);