fix(ivy): inherited listeners from grand super classes invoked multiple times (#30169)

Fixes event listeners that come from more than one level of inheritance being invoked multiple times.

This PR resolves FW-1294.

PR Close #30169
This commit is contained in:
crisbeto 2019-04-29 20:27:32 +02:00 committed by Kara Erickson
parent ad94e02981
commit 37c598db04
2 changed files with 99 additions and 30 deletions

View File

@ -171,28 +171,33 @@ function inheritHostBindings(
definition: DirectiveDef<any>| ComponentDef<any>, definition: DirectiveDef<any>| ComponentDef<any>,
superHostBindings: HostBindingsFunction<any>) { superHostBindings: HostBindingsFunction<any>) {
const prevHostBindings = definition.hostBindings; const prevHostBindings = definition.hostBindings;
if (prevHostBindings) { // If the subclass does not have a host bindings function, we set the subclass host binding
// because inheritance is unknown during compile time, the runtime code // function to be the superclass's (in this feature). We should check if they're the same here
// needs to be informed of the super-class depth so that instruction code // to ensure we don't inherit it twice.
// can distinguish one host bindings function from another. The reason why if (superHostBindings !== prevHostBindings) {
// relying on the directive uniqueId exclusively is not enough is because the if (prevHostBindings) {
// uniqueId value and the directive instance stay the same between hostBindings // because inheritance is unknown during compile time, the runtime code
// calls throughout the directive inheritance chain. This means that without // needs to be informed of the super-class depth so that instruction code
// a super-class depth value, there is no way to know whether a parent or // can distinguish one host bindings function from another. The reason why
// sub-class host bindings function is currently being executed. // relying on the directive uniqueId exclusively is not enough is because the
definition.hostBindings = (rf: RenderFlags, ctx: any, elementIndex: number) => { // uniqueId value and the directive instance stay the same between hostBindings
// The reason why we increment first and then decrement is so that parent // calls throughout the directive inheritance chain. This means that without
// hostBindings calls have a higher id value compared to sub-class hostBindings // a super-class depth value, there is no way to know whether a parent or
// calls (this way the leaf directive is always at a super-class depth of 0). // sub-class host bindings function is currently being executed.
adjustActiveDirectiveSuperClassDepthPosition(1); definition.hostBindings = (rf: RenderFlags, ctx: any, elementIndex: number) => {
try { // The reason why we increment first and then decrement is so that parent
superHostBindings(rf, ctx, elementIndex); // hostBindings calls have a higher id value compared to sub-class hostBindings
} finally { // calls (this way the leaf directive is always at a super-class depth of 0).
adjustActiveDirectiveSuperClassDepthPosition(-1); adjustActiveDirectiveSuperClassDepthPosition(1);
} try {
prevHostBindings(rf, ctx, elementIndex); superHostBindings(rf, ctx, elementIndex);
}; } finally {
} else { adjustActiveDirectiveSuperClassDepthPosition(-1);
definition.hostBindings = superHostBindings; }
prevHostBindings(rf, ctx, elementIndex);
};
} else {
definition.hostBindings = superHostBindings;
}
} }
} }

View File

@ -253,27 +253,91 @@ describe('acceptance integration tests', () => {
expect(clicks).toBe(1); expect(clicks).toBe(1);
}); });
// TODO(crisbeto): this fails even with decorated classes it('should inherit host listeners from superclasses once', () => {
// in master. To be enabled as a part of FW-1294.
xit('should inherit host listeners from undecorated grand superclasses', () => {
let clicks = 0; let clicks = 0;
@Directive({selector: '[baseButton]'})
class BaseButton {
@HostListener('click')
handleClick() { clicks++; }
}
@Component({selector: '[subButton]', template: '<ng-content></ng-content>'})
class SubButton extends BaseButton {
}
@Component({template: '<button subButton>Click me</button>'})
class App {
}
TestBed.configureTestingModule({declarations: [SubButton, BaseButton, App]});
const fixture = TestBed.createComponent(App);
const button = fixture.debugElement.query(By.directive(SubButton)).nativeElement;
button.click();
fixture.detectChanges();
expect(clicks).toBe(1);
});
it('should inherit host listeners from grand superclasses once', () => {
let clicks = 0;
@Directive({selector: '[superBaseButton]'})
class SuperBaseButton { class SuperBaseButton {
@HostListener('click') @HostListener('click')
handleClick() { clicks++; } handleClick() { clicks++; }
} }
class BaseButton extends SuperBaseButton {} @Directive({selector: '[baseButton]'})
class BaseButton extends SuperBaseButton {
}
@Component({selector: '[sub-button]', template: '<ng-content></ng-content>'}) @Component({selector: '[subButton]', template: '<ng-content></ng-content>'})
class SubButton extends BaseButton { class SubButton extends BaseButton {
} }
@Component({template: '<button sub-button>Click me</button>'}) @Component({template: '<button subButton>Click me</button>'})
class App { class App {
} }
TestBed.configureTestingModule({declarations: [SubButton, App]}); TestBed.configureTestingModule({declarations: [SubButton, SuperBaseButton, BaseButton, App]});
const fixture = TestBed.createComponent(App);
const button = fixture.debugElement.query(By.directive(SubButton)).nativeElement;
button.click();
fixture.detectChanges();
expect(clicks).toBe(1);
});
it('should inherit host listeners from grand grand superclasses once', () => {
let clicks = 0;
@Directive({selector: '[superSuperBaseButton]'})
class SuperSuperBaseButton {
@HostListener('click')
handleClick() { clicks++; }
}
@Directive({selector: '[superBaseButton]'})
class SuperBaseButton extends SuperSuperBaseButton {
}
@Directive({selector: '[baseButton]'})
class BaseButton extends SuperBaseButton {
}
@Component({selector: '[subButton]', template: '<ng-content></ng-content>'})
class SubButton extends BaseButton {
}
@Component({template: '<button subButton>Click me</button>'})
class App {
}
TestBed.configureTestingModule(
{declarations: [SubButton, SuperBaseButton, SuperSuperBaseButton, BaseButton, App]});
const fixture = TestBed.createComponent(App); const fixture = TestBed.createComponent(App);
const button = fixture.debugElement.query(By.directive(SubButton)).nativeElement; const button = fixture.debugElement.query(By.directive(SubButton)).nativeElement;