fix(ivy): host bindings should work if input has same name (#27589)

Previously in Ivy, host bindings did not work if they shared a public name
with an Input because they used the `elementProperty` instruction as is.
This instruction was originally built for inside component templates, so it
would either set a directive input OR a native property. This is the
correct behavior for inside a template, but for host bindings, we always
want the native properties to be set regardless of the presence of an Input.

This change adds an extra argument to `elementProperty` so we can tell it to
ignore directive inputs and only set native properties (if it is in the
context of a host binding).

PR Close #27589
This commit is contained in:
Kara Erickson 2018-12-10 14:51:28 -08:00 committed by Alex Rickabaugh
parent ceb14deb60
commit 452668b581
7 changed files with 103 additions and 33 deletions

View File

@ -150,7 +150,7 @@ describe('compiler compliance: bindings', () => {
$r3$.ɵallocHostVars(1); $r3$.ɵallocHostVars(1);
} }
if (rf & 2) { if (rf & 2) {
$r3$.ɵelementProperty(elIndex, "id", $r3$.ɵbind(ctx.dirId)); $r3$.ɵelementProperty(elIndex, "id", $r3$.ɵbind(ctx.dirId), null, true);
} }
} }
}); });
@ -197,7 +197,7 @@ describe('compiler compliance: bindings', () => {
$r3$.ɵallocHostVars(3); $r3$.ɵallocHostVars(3);
} }
if (rf & 2) { if (rf & 2) {
$r3$.ɵelementProperty(elIndex, "id", $r3$.ɵbind($r3$.ɵpureFunction1(1, $ff$, ctx.id))); $r3$.ɵelementProperty(elIndex, "id", $r3$.ɵbind($r3$.ɵpureFunction1(1, $ff$, ctx.id)), null, true);
} }
}, },
consts: 0, consts: 0,

View File

@ -1047,8 +1047,8 @@ describe('compiler compliance: styling', () => {
$r3$.ɵelementStyling($_c0$, $_c1$, $r3$.ɵdefaultStyleSanitizer, ctx); $r3$.ɵelementStyling($_c0$, $_c1$, $r3$.ɵdefaultStyleSanitizer, ctx);
} }
if (rf & 2) { if (rf & 2) {
$r3$.ɵelementProperty(elIndex, "id", $r3$.ɵbind(ctx.id)); $r3$.ɵelementProperty(elIndex, "id", $r3$.ɵbind(ctx.id), null, true);
$r3$.ɵelementProperty(elIndex, "title", $r3$.ɵbind(ctx.title)); $r3$.ɵelementProperty(elIndex, "title", $r3$.ɵbind(ctx.title), null, true);
$r3$.ɵelementStylingMap(elIndex, ctx.myClass, ctx.myStyle, ctx); $r3$.ɵelementStylingMap(elIndex, ctx.myClass, ctx.myStyle, ctx);
$r3$.ɵelementStylingApply(elIndex, ctx); $r3$.ɵelementStylingApply(elIndex, ctx);
} }
@ -1095,8 +1095,8 @@ describe('compiler compliance: styling', () => {
$r3$.ɵelementStyling($_c0$, $_c1$, null, ctx); $r3$.ɵelementStyling($_c0$, $_c1$, null, ctx);
} }
if (rf & 2) { if (rf & 2) {
$r3$.ɵelementProperty(elIndex, "id", $r3$.ɵbind(ctx.id)); $r3$.ɵelementProperty(elIndex, "id", $r3$.ɵbind(ctx.id), null, true);
$r3$.ɵelementProperty(elIndex, "title", $r3$.ɵbind(ctx.title)); $r3$.ɵelementProperty(elIndex, "title", $r3$.ɵbind(ctx.title), null, true);
$r3$.ɵelementStyleProp(elIndex, 0, ctx.myWidth, null, ctx); $r3$.ɵelementStyleProp(elIndex, 0, ctx.myWidth, null, ctx);
$r3$.ɵelementClassProp(elIndex, 0, ctx.myFooClass, ctx); $r3$.ɵelementClassProp(elIndex, 0, ctx.myFooClass, ctx);
$r3$.ɵelementStylingApply(elIndex, ctx); $r3$.ɵelementStylingApply(elIndex, ctx);

View File

@ -576,7 +576,7 @@ describe('ngtsc behavioral tests', () => {
} }
if (rf & 2) { if (rf & 2) {
i0.ɵelementAttribute(elIndex, "hello", i0.ɵbind(ctx.foo)); i0.ɵelementAttribute(elIndex, "hello", i0.ɵbind(ctx.foo));
i0.ɵelementProperty(elIndex, "prop", i0.ɵbind(ctx.bar)); i0.ɵelementProperty(elIndex, "prop", i0.ɵbind(ctx.bar), null, true);
i0.ɵelementClassProp(elIndex, 0, ctx.someClass, ctx); i0.ɵelementClassProp(elIndex, 0, ctx.someClass, ctx);
i0.ɵelementStylingApply(elIndex, ctx); i0.ɵelementStylingApply(elIndex, ctx);
} }

View File

@ -691,16 +691,15 @@ function createHostBindingsFunction(
const value = binding.expression.visit(valueConverter); const value = binding.expression.visit(valueConverter);
const bindingExpr = bindingFn(bindingContext, value); const bindingExpr = bindingFn(bindingContext, value);
const {bindingName, instruction} = getBindingNameAndInstruction(name); const {bindingName, instruction, extraParams} = getBindingNameAndInstruction(name);
const instructionParams: o.Expression[] = [
elVarExp, o.literal(bindingName), o.importExpr(R3.bind).callFn([bindingExpr.currValExpr])
];
updateStatements.push(...bindingExpr.stmts); updateStatements.push(...bindingExpr.stmts);
updateStatements.push(o.importExpr(instruction) updateStatements.push(
.callFn([ o.importExpr(instruction).callFn(instructionParams.concat(extraParams)).toStmt());
elVarExp,
o.literal(bindingName),
o.importExpr(R3.bind).callFn([bindingExpr.currValExpr]),
])
.toStmt());
} }
} }
@ -752,8 +751,9 @@ function createStylingStmt(
} }
function getBindingNameAndInstruction(bindingName: string): function getBindingNameAndInstruction(bindingName: string):
{bindingName: string, instruction: o.ExternalReference} { {bindingName: string, instruction: o.ExternalReference, extraParams: o.Expression[]} {
let instruction !: o.ExternalReference; let instruction !: o.ExternalReference;
const extraParams: o.Expression[] = [];
// Check to see if this is an attr binding or a property binding // Check to see if this is an attr binding or a property binding
const attrMatches = bindingName.match(ATTR_REGEX); const attrMatches = bindingName.match(ATTR_REGEX);
@ -762,9 +762,13 @@ function getBindingNameAndInstruction(bindingName: string):
instruction = R3.elementAttribute; instruction = R3.elementAttribute;
} else { } else {
instruction = R3.elementProperty; instruction = R3.elementProperty;
extraParams.push(
o.literal(null), // TODO: This should be a sanitizer fn (FW-785)
o.literal(true) // host bindings must have nativeOnly prop set to true
);
} }
return {bindingName, instruction}; return {bindingName, instruction, extraParams};
} }
function createHostListeners( function createHostListeners(

View File

@ -938,7 +938,7 @@ export function elementAttribute(
} }
/** /**
* Update a property on an Element. * Update a property on an element.
* *
* If the property name also exists as an input property on one of the element's directives, * If the property name also exists as an input property on one of the element's directives,
* the component property will be set instead of the element property. This check must * the component property will be set instead of the element property. This check must
@ -949,17 +949,21 @@ export function elementAttribute(
* renaming as part of minification. * renaming as part of minification.
* @param value New value to write. * @param value New value to write.
* @param sanitizer An optional function used to sanitize the value. * @param sanitizer An optional function used to sanitize the value.
* @param nativeOnly Whether or not we should only set native properties and skip input check
* (this is necessary for host property bindings)
*/ */
export function elementProperty<T>( export function elementProperty<T>(
index: number, propName: string, value: T | NO_CHANGE, sanitizer?: SanitizerFn | null): void { index: number, propName: string, value: T | NO_CHANGE, sanitizer?: SanitizerFn | null,
nativeOnly?: boolean): void {
if (value === NO_CHANGE) return; if (value === NO_CHANGE) return;
const lView = getLView(); const lView = getLView();
const element = getNativeByIndex(index, lView) as RElement | RComment; const element = getNativeByIndex(index, lView) as RElement | RComment;
const tNode = getTNode(index, lView); const tNode = getTNode(index, lView);
const inputData = initializeTNodeInputs(tNode); let inputData: PropertyAliases|null|undefined;
let dataValue: PropertyAliasValue|undefined; let dataValue: PropertyAliasValue|undefined;
if (inputData && (dataValue = inputData[propName])) { if (!nativeOnly && (inputData = initializeTNodeInputs(tNode)) &&
(dataValue = inputData[propName])) {
setInputsForProperty(lView, dataValue, value); setInputsForProperty(lView, dataValue, value);
if (isComponent(tNode)) markDirtyIfOnPush(lView, index + HEADER_OFFSET); if (isComponent(tNode)) markDirtyIfOnPush(lView, index + HEADER_OFFSET);
if (ngDevMode) { if (ngDevMode) {

View File

@ -1671,20 +1671,18 @@ function declareTests(config?: {useJit: boolean}) {
expect(el.title).toBeFalsy(); expect(el.title).toBeFalsy();
}); });
fixmeIvy('FW-711: elementProperty instruction should not be used in host bindings') it('should work when a directive uses hostProperty to update the DOM element', () => {
.it('should work when a directive uses hostProperty to update the DOM element', () => { TestBed.configureTestingModule({declarations: [MyComp, DirectiveWithTitleAndHostProperty]});
TestBed.configureTestingModule( const template = '<span [title]="ctxProp"></span>';
{declarations: [MyComp, DirectiveWithTitleAndHostProperty]}); TestBed.overrideComponent(MyComp, {set: {template}});
const template = '<span [title]="ctxProp"></span>'; const fixture = TestBed.createComponent(MyComp);
TestBed.overrideComponent(MyComp, {set: {template}});
const fixture = TestBed.createComponent(MyComp);
fixture.componentInstance.ctxProp = 'TITLE'; fixture.componentInstance.ctxProp = 'TITLE';
fixture.detectChanges(); fixture.detectChanges();
const el = getDOM().querySelector(fixture.nativeElement, 'span'); const el = getDOM().querySelector(fixture.nativeElement, 'span');
expect(getDOM().getProperty(el, 'title')).toEqual('TITLE'); expect(getDOM().getProperty(el, 'title')).toEqual('TITLE');
}); });
}); });
describe('logging property updates', () => { describe('logging property updates', () => {

View File

@ -367,6 +367,70 @@ describe('host bindings', () => {
expect(initHookComp.title).toEqual('input2-changes-init-check'); expect(initHookComp.title).toEqual('input2-changes-init-check');
}); });
it('should support host bindings with the same name as inputs', () => {
let hostBindingInputDir !: HostBindingInputDir;
class HostBindingInputDir {
// @Input()
disabled = false;
// @HostBinding('disabled')
hostDisabled = false;
static ngDirectiveDef = defineDirective({
type: HostBindingInputDir,
selectors: [['', 'hostBindingDir', '']],
factory: () => hostBindingInputDir = new HostBindingInputDir(),
hostBindings: (rf: RenderFlags, ctx: HostBindingInputDir, elIndex: number) => {
if (rf & RenderFlags.Create) {
allocHostVars(1);
}
if (rf & RenderFlags.Update) {
elementProperty(elIndex, 'disabled', bind(ctx.hostDisabled), null, true);
}
},
inputs: {disabled: 'disabled'}
});
}
/** <input hostBindingDir [disabled]="isDisabled"> */
class App {
isDisabled = true;
static ngComponentDef = defineComponent({
type: App,
selectors: [['app']],
factory: () => new App(),
template: (rf: RenderFlags, ctx: App) => {
if (rf & RenderFlags.Create) {
element(0, 'input', ['hostBindingDir', '']);
}
if (rf & RenderFlags.Update) {
elementProperty(0, 'disabled', bind(ctx.isDisabled));
}
},
consts: 1,
vars: 1,
directives: [HostBindingInputDir]
});
}
const fixture = new ComponentFixture(App);
const hostBindingEl = fixture.hostElement.querySelector('input') as HTMLInputElement;
expect(hostBindingInputDir.disabled).toBe(true);
expect(hostBindingEl.disabled).toBe(false);
fixture.component.isDisabled = false;
fixture.update();
expect(hostBindingInputDir.disabled).toBe(false);
expect(hostBindingEl.disabled).toBe(false);
hostBindingInputDir.hostDisabled = true;
fixture.update();
expect(hostBindingInputDir.disabled).toBe(false);
expect(hostBindingEl.disabled).toBe(true);
});
it('should support host bindings on second template pass', () => { it('should support host bindings on second template pass', () => {
/** <div hostBindingDir></div> */ /** <div hostBindingDir></div> */
const Parent = createComponent('parent', (rf: RenderFlags, ctx: any) => { const Parent = createComponent('parent', (rf: RenderFlags, ctx: any) => {