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

fix(ivy): call hostBindings function with proper element index (#27694)

Browse Source 
We invoked `hostBindings` function in Create and Update modes with different element index due to the fact that we did not subtract HEADER_OFFSET from the index before passing it to `hostBindings` function in Create mode. Now we subtract HEADER_OFFSET value before invoking `hostBindings`, which makes Ceate and Update calls consistent.

PR Close #27694
This commit is contained in:
Andrew Kushnir 2018-12-15 15:57:57 -08:00 committed by Miško Hevery
parent 4c1cd1bb78
commit 9bfe42840b
3 changed files with 57 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
packages/core/src/render3/component.ts
View File

@ -198,7 +198,7 @@ export function createRootComponent<T>(
if (tView.firstTemplatePass && componentDef.hostBindings) { if (tView.firstTemplatePass && componentDef.hostBindings) {
const rootTNode = getPreviousOrParentTNode(); const rootTNode = getPreviousOrParentTNode();
setCurrentDirectiveDef(componentDef); setCurrentDirectiveDef(componentDef);
componentDef.hostBindings(RenderFlags.Create, component, rootTNode.index); componentDef.hostBindings(RenderFlags.Create, component, rootTNode.index - HEADER_OFFSET);
setCurrentDirectiveDef(null); setCurrentDirectiveDef(null);
} }

2
packages/core/src/render3/instructions.ts
View File

@ -1541,7 +1541,7 @@ function invokeDirectivesHostBindings(tView: TView, viewData: LView, tNode: TNod
if (def.hostBindings) { if (def.hostBindings) {
const previousExpandoLength = expando.length; const previousExpandoLength = expando.length;
setCurrentDirectiveDef(def); setCurrentDirectiveDef(def);
def.hostBindings !(RenderFlags.Create, directive, tNode.index); def.hostBindings !(RenderFlags.Create, directive, tNode.index - HEADER_OFFSET);
setCurrentDirectiveDef(null); setCurrentDirectiveDef(null);
// `hostBindings` function may or may not contain `allocHostVars` call // `hostBindings` function may or may not contain `allocHostVars` call
// (e.g. it may not if it only contains host listeners), so we need to check whether // (e.g. it may not if it only contains host listeners), so we need to check whether

75
packages/core/test/render3/host_binding_spec.ts
View File

@ -54,7 +54,7 @@ describe('host bindings', () => {
allocHostVars(1); allocHostVars(1);
} }
if (rf & RenderFlags.Update) { if (rf & RenderFlags.Update) {
elementProperty(elementIndex, 'id', bind(ctx.id)); elementProperty(elementIndex, 'id', bind(ctx.id), null, true);
} }
} }
}); });
@ -75,7 +75,7 @@ describe('host bindings', () => {
allocHostVars(1); allocHostVars(1);
} }
if (rf & RenderFlags.Update) { if (rf & RenderFlags.Update) {
elementProperty(elIndex, 'id', bind(ctx.id)); elementProperty(elIndex, 'id', bind(ctx.id), null, true);
} }
}, },
template: (rf: RenderFlags, ctx: HostBindingComp) => {} template: (rf: RenderFlags, ctx: HostBindingComp) => {}
@ -84,7 +84,7 @@ describe('host bindings', () => {
it('should support host bindings in directives', () => { it('should support host bindings in directives', () => {
let directiveInstance: Directive|undefined; let directiveInstance: Directive|undefined;
const elementIndices: number[] = [];
class Directive { class Directive {
// @HostBinding('className') // @HostBinding('className')
klass = 'foo'; klass = 'foo';
@ -94,11 +94,12 @@ describe('host bindings', () => {
selectors: [['', 'dir', '']], selectors: [['', 'dir', '']],
factory: () => directiveInstance = new Directive, factory: () => directiveInstance = new Directive,
hostBindings: (rf: RenderFlags, ctx: any, elementIndex: number) => { hostBindings: (rf: RenderFlags, ctx: any, elementIndex: number) => {
elementIndices.push(elementIndex);
if (rf & RenderFlags.Create) { if (rf & RenderFlags.Create) {
allocHostVars(1); allocHostVars(1);
} }
if (rf & RenderFlags.Update) { if (rf & RenderFlags.Update) {
elementProperty(elementIndex, 'className', bind(ctx.klass)); elementProperty(elementIndex, 'className', bind(ctx.klass), null, true);
} }
} }
}); });
@ -112,15 +113,46 @@ describe('host bindings', () => {
directiveInstance !.klass = 'bar'; directiveInstance !.klass = 'bar';
fixture.update(); fixture.update();
expect(fixture.html).toEqual('<span class="bar"></span>'); expect(fixture.html).toEqual('<span class="bar"></span>');
// verify that we always call `hostBindings` function with the same element index
expect(elementIndices.every(id => id === elementIndices[0])).toBeTruthy();
}); });
it('should support host bindings on root component', () => { it('should support host bindings on root component', () => {
const elementIndices: number[] = [];
class HostBindingComp {
// @HostBinding()
id = 'my-id';
static ngComponentDef = defineComponent({
type: HostBindingComp,
selectors: [['host-binding-comp']],
factory: () => new HostBindingComp(),
consts: 0,
vars: 0,
hostBindings: (rf: RenderFlags, ctx: HostBindingComp, elIndex: number) => {
elementIndices.push(elIndex);
if (rf & RenderFlags.Create) {
allocHostVars(1);
}
if (rf & RenderFlags.Update) {
elementProperty(elIndex, 'id', bind(ctx.id), null, true);
}
},
template: (rf: RenderFlags, ctx: HostBindingComp) => {}
});
}
const fixture = new ComponentFixture(HostBindingComp); const fixture = new ComponentFixture(HostBindingComp);
expect(fixture.hostElement.id).toBe('my-id'); expect(fixture.hostElement.id).toBe('my-id');
fixture.component.id = 'other-id'; fixture.component.id = 'other-id';
fixture.update(); fixture.update();
expect(fixture.hostElement.id).toBe('other-id'); expect(fixture.hostElement.id).toBe('other-id');
// verify that we always call `hostBindings` function with the same element index
expect(elementIndices.every(id => id === elementIndices[0])).toBeTruthy();
}); });
it('should support host bindings on nodes with providers', () => { it('should support host bindings on nodes with providers', () => {
@ -150,7 +182,7 @@ describe('host bindings', () => {
allocHostVars(1); allocHostVars(1);
} }
if (rf & RenderFlags.Update) { if (rf & RenderFlags.Update) {
elementProperty(elIndex, 'id', bind(ctx.id)); elementProperty(elIndex, 'id', bind(ctx.id), null, true);
} }
}, },
template: (rf: RenderFlags, ctx: CompWithProviders) => {}, template: (rf: RenderFlags, ctx: CompWithProviders) => {},
@ -186,7 +218,7 @@ describe('host bindings', () => {
allocHostVars(1); allocHostVars(1);
} }
if (rf & RenderFlags.Update) { if (rf & RenderFlags.Update) {
elementProperty(elIndex, 'title', bind(ctx.title)); elementProperty(elIndex, 'title', bind(ctx.title), null, true);
} }
}, },
template: (rf: RenderFlags, ctx: HostTitleComp) => {} template: (rf: RenderFlags, ctx: HostTitleComp) => {}
@ -239,7 +271,7 @@ describe('host bindings', () => {
allocHostVars(1); allocHostVars(1);
} }
if (rf & RenderFlags.Update) { if (rf & RenderFlags.Update) {
elementProperty(elIndex, 'id', bind(ctx.id)); elementProperty(elIndex, 'id', bind(ctx.id), null, true);
} }
}, },
template: (rf: RenderFlags, ctx: HostBindingComp) => {} template: (rf: RenderFlags, ctx: HostBindingComp) => {}
@ -329,7 +361,7 @@ describe('host bindings', () => {
allocHostVars(1); allocHostVars(1);
} }
if (rf & RenderFlags.Update) { if (rf & RenderFlags.Update) {
elementProperty(elIndex, 'title', bind(ctx.value)); elementProperty(elIndex, 'title', bind(ctx.value), null, true);
} }
}, },
inputs: {inputValue: 'inputValue'} inputs: {inputValue: 'inputValue'}
@ -562,10 +594,11 @@ describe('host bindings', () => {
allocHostVars(8); allocHostVars(8);
} }
if (rf & RenderFlags.Update) { if (rf & RenderFlags.Update) {
elementProperty(elIndex, 'id', bind(pureFunction1(3, ff, ctx.id))); elementProperty(elIndex, 'id', bind(pureFunction1(3, ff, ctx.id)), null, true);
elementProperty(elIndex, 'dir', bind(ctx.dir)); elementProperty(elIndex, 'dir', bind(ctx.dir), null, true);
elementProperty( elementProperty(
elIndex, 'title', bind(pureFunction2(5, ff2, ctx.title, ctx.otherTitle))); elIndex, 'title', bind(pureFunction2(5, ff2, ctx.title, ctx.otherTitle)), null,
true);
} }
}, },
template: (rf: RenderFlags, ctx: HostBindingComp) => {} template: (rf: RenderFlags, ctx: HostBindingComp) => {}
@ -640,7 +673,7 @@ describe('host bindings', () => {
allocHostVars(3); allocHostVars(3);
} }
if (rf & RenderFlags.Update) { if (rf & RenderFlags.Update) {
elementProperty(elIndex, 'id', bind(pureFunction1(1, ff, ctx.id))); elementProperty(elIndex, 'id', bind(pureFunction1(1, ff, ctx.id)), null, true);
} }
}, },
template: (rf: RenderFlags, ctx: HostBindingComp) => {} template: (rf: RenderFlags, ctx: HostBindingComp) => {}
@ -671,7 +704,7 @@ describe('host bindings', () => {
allocHostVars(3); allocHostVars(3);
} }
if (rf & RenderFlags.Update) { if (rf & RenderFlags.Update) {
elementProperty(elIndex, 'title', bind(pureFunction1(1, ff1, ctx.title))); elementProperty(elIndex, 'title', bind(pureFunction1(1, ff1, ctx.title)), null, true);
} }
} }
}); });
@ -728,7 +761,7 @@ describe('host bindings', () => {
allocHostVars(3); allocHostVars(3);
} }
if (rf & RenderFlags.Update) { if (rf & RenderFlags.Update) {
elementProperty(elIndex, 'title', bind(pureFunction1(1, ff1, ctx.title))); elementProperty(elIndex, 'title', bind(pureFunction1(1, ff1, ctx.title)), null, true);
} }
} }
}); });
@ -799,10 +832,12 @@ describe('host bindings', () => {
} }
if (rf & RenderFlags.Update) { if (rf & RenderFlags.Update) {
elementProperty( elementProperty(
elIndex, 'id', bind(ctx.condition ? pureFunction1(2, ff, ctx.id) : 'green')); elIndex, 'id', bind(ctx.condition ? pureFunction1(2, ff, ctx.id) : 'green'), null,
true);
elementProperty( elementProperty(
elIndex, 'title', elIndex, 'title',
bind(ctx.otherCondition ? pureFunction1(4, ff1, ctx.title) : 'other title')); bind(ctx.otherCondition ? pureFunction1(4, ff1, ctx.title) : 'other title'), null,
true);
} }
}, },
template: (rf: RenderFlags, ctx: HostBindingComp) => {} template: (rf: RenderFlags, ctx: HostBindingComp) => {}
@ -859,7 +894,7 @@ describe('host bindings', () => {
allocHostVars(1); allocHostVars(1);
} }
if (rf & RenderFlags.Update) { if (rf & RenderFlags.Update) {
elementProperty(elementIndex, 'id', bind(ctx.id)); elementProperty(elementIndex, 'id', bind(ctx.id), null, true);
} }
}, },
factory: () => superDir = new SuperDirective(), factory: () => superDir = new SuperDirective(),
@ -877,7 +912,7 @@ describe('host bindings', () => {
allocHostVars(1); allocHostVars(1);
} }
if (rf & RenderFlags.Update) { if (rf & RenderFlags.Update) {
elementProperty(elementIndex, 'title', bind(ctx.title)); elementProperty(elementIndex, 'title', bind(ctx.title), null, true);
} }
}, },
factory: () => subDir = new SubDirective(), factory: () => subDir = new SubDirective(),
@ -965,7 +1000,7 @@ describe('host bindings', () => {
allocHostVars(1); allocHostVars(1);
} }
if (rf & RenderFlags.Update) { if (rf & RenderFlags.Update) {
elementProperty(elIndex, 'id', bind(ctx.foos.length)); elementProperty(elIndex, 'id', bind(ctx.foos.length), null, true);
} }
}, },
contentQueries: (dirIndex) => { registerContentQuery(query(null, ['foo']), dirIndex); }, contentQueries: (dirIndex) => { registerContentQuery(query(null, ['foo']), dirIndex); },
@ -1024,7 +1059,7 @@ describe('host bindings', () => {
allocHostVars(1); allocHostVars(1);
} }
if (rf & RenderFlags.Update) { if (rf & RenderFlags.Update) {
elementProperty(elIndex, 'id', bind(ctx.myValue)); elementProperty(elIndex, 'id', bind(ctx.myValue), null, true);
} }
}, },
template: (rf: RenderFlags, cmp: HostBindingWithContentHooks) => {} template: (rf: RenderFlags, cmp: HostBindingWithContentHooks) => {}