fix(ivy): handling className as an input properly (#33188)

Prior to this commit, all `className` inputs were not set because the runtime code assumed that the `classMap` instruction is only generated for `[class]` bindings. However the `[className]` binding also produces the same `classMap`, thus the code needs to distinguish between `class` and `className`. This commit adds extra logic to select the right input name and also throws an error in case `[class]` and `[className]` bindings are used on the same element simultaneously.

PR Close #33188
This commit is contained in:
Andrew Kushnir 2019-10-04 19:25:18 -07:00 committed by Matias Niemelä
parent 08cb2fa80f
commit 6f203c9575
8 changed files with 91 additions and 6 deletions

View File

@ -201,6 +201,10 @@ export class StylingBuilder {
const entry: const entry:
BoundStylingEntry = {name: property, value, sourceSpan, hasOverrideFlag, unit: null}; BoundStylingEntry = {name: property, value, sourceSpan, hasOverrideFlag, unit: null};
if (isMapBased) { if (isMapBased) {
if (this._classMapInput) {
throw new Error(
'[class] and [className] bindings cannot be used on the same element simultaneously');
}
this._classMapInput = entry; this._classMapInput = entry;
} else { } else {
(this._singleClassInputs = this._singleClassInputs || []).push(entry); (this._singleClassInputs = this._singleClassInputs || []).push(entry);

View File

@ -19,7 +19,7 @@ import {assertNodeType} from '../node_assert';
import {appendChild} from '../node_manipulation'; import {appendChild} from '../node_manipulation';
import {decreaseElementDepthCount, getElementDepthCount, getIsParent, getLView, getNamespace, getPreviousOrParentTNode, getSelectedIndex, increaseElementDepthCount, setIsNotParent, setPreviousOrParentTNode} from '../state'; import {decreaseElementDepthCount, getElementDepthCount, getIsParent, getLView, getNamespace, getPreviousOrParentTNode, getSelectedIndex, increaseElementDepthCount, setIsNotParent, setPreviousOrParentTNode} from '../state';
import {setUpAttributes} from '../util/attrs_utils'; import {setUpAttributes} from '../util/attrs_utils';
import {getInitialStylingValue, hasClassInput, hasStyleInput} from '../util/styling_utils'; import {getInitialStylingValue, hasClassInput, hasStyleInput, selectClassBasedInputName} from '../util/styling_utils';
import {getNativeByTNode, getTNode} from '../util/view_utils'; import {getNativeByTNode, getTNode} from '../util/view_utils';
import {createDirectivesInstances, elementCreate, executeContentQueries, getOrCreateTNode, renderInitialStyling, resolveDirectives, saveResolvedLocalsInData, setInputsForProperty} from './shared'; import {createDirectivesInstances, elementCreate, executeContentQueries, getOrCreateTNode, renderInitialStyling, resolveDirectives, saveResolvedLocalsInData, setInputsForProperty} from './shared';
@ -132,7 +132,8 @@ export function ɵɵelementEnd(): void {
} }
if (hasClassInput(tNode)) { if (hasClassInput(tNode)) {
setDirectiveStylingInput(tNode.classes, lView, tNode.inputs !['class']); const inputName: string = selectClassBasedInputName(tNode.inputs !);
setDirectiveStylingInput(tNode.classes, lView, tNode.inputs ![inputName]);
} }
if (hasStyleInput(tNode)) { if (hasStyleInput(tNode)) {

View File

@ -853,7 +853,7 @@ function initializeInputAndOutputAliases(tView: TView, tNode: TNode): void {
} }
if (inputsStore !== null) { if (inputsStore !== null) {
if (inputsStore.hasOwnProperty('class')) { if (inputsStore.hasOwnProperty('class') || inputsStore.hasOwnProperty('className')) {
tNode.flags |= TNodeFlags.hasClassInput; tNode.flags |= TNodeFlags.hasClassInput;
} }
if (inputsStore.hasOwnProperty('style')) { if (inputsStore.hasOwnProperty('style')) {

View File

@ -19,7 +19,7 @@ import {activateStylingMapFeature} from '../styling/map_based_bindings';
import {attachStylingDebugObject} from '../styling/styling_debug'; import {attachStylingDebugObject} from '../styling/styling_debug';
import {NO_CHANGE} from '../tokens'; import {NO_CHANGE} from '../tokens';
import {renderStringify} from '../util/misc_utils'; import {renderStringify} from '../util/misc_utils';
import {addItemToStylingMap, allocStylingMapArray, allocTStylingContext, allowDirectStyling, concatString, forceClassesAsString, forceStylesAsString, getInitialStylingValue, getStylingMapArray, hasClassInput, hasStyleInput, hasValueChanged, isContextLocked, isHostStylingActive, isStylingContext, normalizeIntoStylingMap, patchConfig, setValue, stylingMapToString} from '../util/styling_utils'; import {addItemToStylingMap, allocStylingMapArray, allocTStylingContext, allowDirectStyling, concatString, forceClassesAsString, forceStylesAsString, getInitialStylingValue, getStylingMapArray, hasClassInput, hasStyleInput, hasValueChanged, isContextLocked, isHostStylingActive, isStylingContext, normalizeIntoStylingMap, patchConfig, selectClassBasedInputName, setValue, stylingMapToString} from '../util/styling_utils';
import {getNativeByTNode, getTNode} from '../util/view_utils'; import {getNativeByTNode, getTNode} from '../util/view_utils';
@ -402,7 +402,7 @@ function updateDirectiveInputValue(
// directive input(s) in the event that it is falsy during the // directive input(s) in the event that it is falsy during the
// first update pass. // first update pass.
if (newValue || isContextLocked(context, false)) { if (newValue || isContextLocked(context, false)) {
const inputName = isClassBased ? 'class' : 'style'; const inputName: string = isClassBased ? selectClassBasedInputName(tNode.inputs !) : 'style';
const inputs = tNode.inputs ![inputName] !; const inputs = tNode.inputs ![inputName] !;
const initialValue = getInitialStylingValue(context); const initialValue = getInitialStylingValue(context);
const value = normalizeStylingDirectiveInputValue(initialValue, newValue, isClassBased); const value = normalizeStylingDirectiveInputValue(initialValue, newValue, isClassBased);

View File

@ -5,7 +5,7 @@
* Use of this source code is governed by an MIT-style license that can be * Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license * found in the LICENSE file at https://angular.io/license
*/ */
import {TNode, TNodeFlags} from '../interfaces/node'; import {PropertyAliases, TNode, TNodeFlags} from '../interfaces/node';
import {LStylingData, StylingMapArray, StylingMapArrayIndex, TStylingConfig, TStylingContext, TStylingContextIndex, TStylingContextPropConfigFlags} from '../interfaces/styling'; import {LStylingData, StylingMapArray, StylingMapArrayIndex, TStylingConfig, TStylingContext, TStylingContextIndex, TStylingContextPropConfigFlags} from '../interfaces/styling';
import {NO_CHANGE} from '../tokens'; import {NO_CHANGE} from '../tokens';
@ -433,3 +433,9 @@ export function normalizeIntoStylingMap(
return stylingMapArr; return stylingMapArr;
} }
// TODO (matsko|AndrewKushnir): refactor this once we figure out how to generate separate
// `input('class') + classMap()` instructions.
export function selectClassBasedInputName(inputs: PropertyAliases): string {
return inputs.hasOwnProperty('class') ? 'class' : 'className';
}

View File

@ -551,6 +551,74 @@ describe('styling', () => {
expect(capturedMyClassBindingCount).toEqual(1); expect(capturedMyClassBindingCount).toEqual(1);
}); });
it('should write to a `className` input binding', () => {
@Component({
selector: 'comp',
template: `{{className}}`,
})
class Comp {
@Input() className: string = '';
}
@Component({
template: `<comp [className]="'my-className'"></comp>`,
})
class App {
}
TestBed.configureTestingModule({declarations: [Comp, App]});
const fixture = TestBed.createComponent(App);
fixture.detectChanges();
expect(fixture.debugElement.nativeElement.firstChild.innerHTML).toBe('my-className');
});
onlyInIvy('only ivy combines static and dynamic class-related attr values')
.it('should write to a `className` input binding, when static `class` is present', () => {
@Component({
selector: 'comp',
template: `{{className}}`,
})
class Comp {
@Input() className: string = '';
}
@Component({
template: `<comp class="static" [className]="'my-className'"></comp>`,
})
class App {
}
TestBed.configureTestingModule({declarations: [Comp, App]});
const fixture = TestBed.createComponent(App);
fixture.detectChanges();
expect(fixture.debugElement.nativeElement.firstChild.innerHTML).toBe('static my-className');
});
onlyInIvy('in Ivy [class] and [className] bindings on the same element are not allowed')
.it('should throw an error in case [class] and [className] bindings are used on the same element',
() => {
@Component({
selector: 'comp',
template: `{{class}} - {{className}}`,
})
class Comp {
@Input() class: string = '';
@Input() className: string = '';
}
@Component({
template: `<comp [class]="'my-class'" [className]="'className'"></comp>`,
})
class App {
}
TestBed.configureTestingModule({declarations: [Comp, App]});
expect(() => {
const fixture = TestBed.createComponent(App);
fixture.detectChanges();
})
.toThrowError(
'[class] and [className] bindings cannot be used on the same element simultaneously');
});
onlyInIvy('only ivy persists static class/style attrs with their binding counterparts') onlyInIvy('only ivy persists static class/style attrs with their binding counterparts')
.it('should write to a `class` input binding if there is a static class value and there is a binding value', .it('should write to a `class` input binding if there is a static class value and there is a binding value',
() => { () => {

View File

@ -584,6 +584,9 @@
{ {
"name": "saveResolvedLocalsInData" "name": "saveResolvedLocalsInData"
}, },
{
"name": "selectClassBasedInputName"
},
{ {
"name": "selectIndexInternal" "name": "selectIndexInternal"
}, },

View File

@ -1184,6 +1184,9 @@
{ {
"name": "searchTokensOnInjector" "name": "searchTokensOnInjector"
}, },
{
"name": "selectClassBasedInputName"
},
{ {
"name": "selectIndexInternal" "name": "selectIndexInternal"
}, },