Skip to content
Permalink
Browse files

fixup! fix(ivy): run pre-order hooks in injection order

  • Loading branch information
kara committed Nov 25, 2019
1 parent c9cd56f commit 94db9e58b630a42fb31d753d1939ff9e089d6780
@@ -72,3 +72,14 @@ export function assertFirstCreatePass(tView: TView, errMessage?: string) {
assertEqual(
tView.firstCreatePass, true, errMessage || 'Should only be called in first create pass.');
}

/**
* This is a basic sanity check that an object is probably a directive def. DirectiveDef is
* an interface, so we can't do a direct instanceof check.
*/
export function assertDirectiveDef(obj: any) {
if (obj.type === undefined || obj.selectors == undefined || obj.inputs === undefined) {
throwError(
`Expected a DirectiveDef/ComponentDef and this object does not seem to have the expected shape.`);
}
}
@@ -15,6 +15,7 @@ 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';
@@ -472,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;
}
@@ -527,8 +528,9 @@ export function locateDirectiveOrProvider<T>(
* 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) {
@@ -543,14 +545,14 @@ export function getNodeInjectable(
enterDI(lView, tNode);
try {
value = lView[index] = factory.factory(undefined, tData, lView, tNode);
const tView = lView[TVIEW];
// 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 {
@@ -215,13 +215,14 @@ function multiProvidersFactoryResolver(
* This factory knows how to concatenate itself with the existing `multi` `providers`.
*/
function multiViewProvidersFactoryResolver(
this: NodeInjectorFactory, _: undefined, tData: TData, lData: LView,
this: NodeInjectorFactory, _: undefined, tData: TData, lView: LView,
tNode: TDirectiveHostNode): any[] {
const factories = this.multi !;
let result: any[];
if (this.providerFactory) {
const componentCount = this.providerFactory.componentProviders !;
const multiProviders = getNodeInjectable(tData, lData, this.providerFactory !.index !, tNode);
const multiProviders =
getNodeInjectable(lView, lView[TVIEW], this.providerFactory !.index !, tNode);
// Copy the section of the array which contains `multi` `providers` from the component
result = multiProviders.slice(0, componentCount);
// Insert the `viewProvider` instances.
@@ -1057,8 +1057,7 @@ export function instantiateRootComponent<T>(tView: TView, lView: LView, def: Com
generateExpandoInstructionBlock(tView, rootTNode, 1);
baseResolveDirective(tView, lView, def);
}
const directive =
getNodeInjectable(tView.data, lView, lView.length - 1, rootTNode as TElementNode);
const directive = getNodeInjectable(lView, tView, lView.length - 1, rootTNode as TElementNode);
attachPatchData(directive, lView);
const native = getNativeByTNode(rootTNode, lView);
if (native) {
@@ -1155,7 +1154,7 @@ function instantiateAllDirectives(
addComponentLogic(lView, tNode as TElementNode, def as ComponentDef<any>);
}

const directive = getNodeInjectable(tView.data, lView, i, tNode);
const directive = getNodeInjectable(lView, tView, i, tNode);
attachPatchData(directive, lView);

if (initialInputs !== null) {
@@ -281,7 +281,7 @@ function createResultForNode(lView: LView, tNode: TNode, matchingIdx: number, re
return createSpecialToken(lView, tNode, read);
} else {
// read a token
return getNodeInjectable(lView[TVIEW].data, lView, matchingIdx, tNode as TElementNode);
return getNodeInjectable(lView, lView[TVIEW], matchingIdx, tNode as TElementNode);
}
}

@@ -683,6 +683,62 @@ describe('onChanges', () => {

});

it('should be called on multiple directives in injection order', () => {

const events: any[] = [];

@Directive({
selector: '[dir]',
})
class Dir {
@Input()
dir = '';

ngOnChanges(changes: SimpleChanges) { events.push({name: 'dir', changes}); }
}

@Directive({
selector: '[injectionDir]',
})
class InjectionDir {
@Input()
injectionDir = '';

constructor(public dir: Dir) {}

ngOnChanges(changes: SimpleChanges) { events.push({name: 'injectionDir', changes}); }
}

@Component({
template: `<div [injectionDir]="val" [dir]="val"></div>`,
})
class App {
val = 'a';
}

TestBed.configureTestingModule({
declarations: [App, InjectionDir, Dir],
});
const fixture = TestBed.createComponent(App);
fixture.detectChanges();

expect(events).toEqual([
{
name: 'dir',
changes: {
dir: new SimpleChange(undefined, 'a', true),
}
},
{
name: 'injectionDir',
changes: {
injectionDir: new SimpleChange(undefined, 'a', true),
}
}
]);
});


it('should be called on directives on an element', () => {
const events: any[] = [];

@@ -1531,6 +1587,50 @@ describe('onInit', () => {
expect(initialized).toEqual(['app', 'comp 1', 'dir 1', 'comp 2', 'dir 2']);
});

it('should be called on multiple directives in injection order', () => {

const events: any[] = [];

@Directive({
selector: '[dir]',
})
class Dir {
@Input()
dir = '';

ngOnInit() { events.push('dir'); }
}

@Directive({
selector: '[injectionDir]',
})
class InjectionDir {
@Input()
injectionDir = '';

constructor(public dir: Dir) {}

ngOnInit() { events.push('injectionDir'); }
}

@Component({
template: `<div [injectionDir]="val" [dir]="val"></div>`,
})
class App {
val = 'a';

ngOnInit() { events.push('app'); }
}

TestBed.configureTestingModule({
declarations: [App, InjectionDir, Dir],
});
const fixture = TestBed.createComponent(App);
fixture.detectChanges();

expect(events).toEqual(['app', 'dir', 'injectionDir']);
});

it('should be called on directives before component if component injects directives', () => {
const initialized: string[] = [];

@@ -1869,6 +1969,50 @@ describe('doCheck', () => {
expect(doChecks).toEqual(['app', 'dir 1', 'comp 1', 'dir 2', 'comp 2']);
});

it('should be called on multiple directives in injection order', () => {

const events: any[] = [];

@Directive({
selector: '[dir]',
})
class Dir {
@Input()
dir = '';

ngDoCheck() { events.push('dir'); }
}

@Directive({
selector: '[injectionDir]',
})
class InjectionDir {
@Input()
injectionDir = '';

constructor(public dir: Dir) {}

ngDoCheck() { events.push('injectionDir'); }
}

@Component({
template: `<div [injectionDir]="val" [dir]="val"></div>`,
})
class App {
val = 'a';

ngDoCheck() { events.push('app'); }
}

TestBed.configureTestingModule({
declarations: [App, InjectionDir, Dir],
});
const fixture = TestBed.createComponent(App);
fixture.detectChanges();

expect(events).toEqual(['app', 'dir', 'injectionDir']);
});

it('should be called on directives on an element', () => {
const doChecks: string[] = [];

@@ -1063,7 +1063,7 @@ import {dispatchEvent} from '@angular/platform-browser/testing/src/browser_util'

expect(fixture.componentInstance.myInput !.control).toBeDefined();
expect(fixture.componentInstance.myInput !.control)
.toEqual(fixture.componentInstance.myInput !.cd.control);
.toEqual(fixture.componentInstance.myInput !.controlDir.control);
});
});

@@ -1371,9 +1371,9 @@ export class MyInput implements ControlValueAccessor {

control: AbstractControl|null = null;

constructor(public cd: NgControl) { cd.valueAccessor = this; }
constructor(public controlDir: NgControl) { controlDir.valueAccessor = this; }

ngOnInit() { this.control = this.cd.control; }
ngOnInit() { this.control = this.controlDir.control; }

writeValue(value: any) { this.value = `!${value}!`; }

0 comments on commit 94db9e5

Please sign in to comment.
You can’t perform that action at this time.