Skip to content

Commit

Permalink
fix(core): treat [class] and [className] as unrelated bindings (#…
Browse files Browse the repository at this point in the history
…35668)

Before this change `[class]` and `[className]` were both converted into `ɵɵclassMap`. The implication of this is that at runtime we could not differentiate between the two and as a result we treated `@Input('class')` and `@Input('className)` as equivalent.

This change makes `[class]` and `[className]` distinct. The implication of this is that `[class]` becomes `ɵɵclassMap` instruction but  `[className]` becomes `ɵɵproperty' instruction. This means that `[className]` will no longer participate in styling and will overwrite the DOM `class` value.

Fix #35577

PR Close #35668
  • Loading branch information
mhevery authored and atscott committed Mar 2, 2020
1 parent 11c2e8c commit 48025eb
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 34 deletions.
3 changes: 1 addition & 2 deletions packages/compiler/src/render3/view/styling_builder.ts
Expand Up @@ -202,8 +202,7 @@ export class StylingBuilder {
let binding: BoundStylingEntry|null = null;
const prefix = name.substring(0, 6);
const isStyle = name === 'style' || prefix === 'style.' || prefix === 'style!';
const isClass = !isStyle &&
(name === 'class' || name === 'className' || prefix === 'class.' || prefix === 'class!');
const isClass = !isStyle && (name === 'class' || prefix === 'class.' || prefix === 'class!');
if (isStyle || isClass) {
const isMapBased = name.charAt(5) !== '.'; // style.prop or class.prop makes this a no
const property = name.substr(isMapBased ? 5 : 6); // the dot explains why there's a +1
Expand Down
3 changes: 1 addition & 2 deletions packages/core/src/render3/instructions/property.ts
Expand Up @@ -53,6 +53,5 @@ export function setDirectiveInputsWhichShadowsStyling(
const inputs = tNode.inputs !;
const property = isClassBased ? 'class' : 'style';
// We support both 'class' and `className` hence the fallback.
const stylingInputs = inputs[property] || (isClassBased && inputs['className']);
setInputsForProperty(tView, lView, stylingInputs, property, value);
setInputsForProperty(tView, lView, inputs[property], property, value);
}
2 changes: 1 addition & 1 deletion packages/core/src/render3/instructions/shared.ts
Expand Up @@ -907,7 +907,7 @@ function initializeInputAndOutputAliases(tView: TView, tNode: TNode): void {
}

if (inputsStore !== null) {
if (inputsStore.hasOwnProperty('class') || inputsStore.hasOwnProperty('className')) {
if (inputsStore.hasOwnProperty('class')) {
tNode.flags |= TNodeFlags.hasClassInput;
}
if (inputsStore.hasOwnProperty('style')) {
Expand Down
70 changes: 41 additions & 29 deletions packages/core/test/acceptance/styling_spec.ts
Expand Up @@ -1129,17 +1129,17 @@ describe('styling', () => {
});

onlyInIvy('only ivy combines static and dynamic class-related attr values')
.it('should write to a `className` input binding, when static `class` is present', () => {
.it('should write combined class attribute and class binding to the class input', () => {
@Component({
selector: 'comp',
template: `{{className}}`,
})
class Comp {
@Input() className: string = '';
@Input('class') className: string = '';
}

@Component({
template: `<comp class="static" [className]="'my-className'"></comp>`,
template: `<comp class="static" [class]="'my-className'"></comp>`,
})
class App {
}
Expand All @@ -1150,32 +1150,6 @@ describe('styling', () => {
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')
.it('should write to a `class` input binding if there is a static class value and there is a binding value',
() => {
Expand Down Expand Up @@ -3475,6 +3449,44 @@ describe('styling', () => {
expectClass(cmp1).toEqual({foo: true, bar: true});
expectClass(cmp2).toEqual({foo: true, bar: true});
});

it('should not bind [class] to @Input("className")', () => {
@Component({
selector: 'my-cmp',
template: `className = {{className}}`,
})
class MyCmp {
@Input()
className: string = 'unbound';
}
@Component({template: `<my-cmp [class]="'bound'"></my-cmp>`})
class MyApp {
}

TestBed.configureTestingModule({declarations: [MyApp, MyCmp]});
const fixture = TestBed.createComponent(MyApp);
fixture.detectChanges();
expect(fixture.nativeElement.textContent).toEqual('className = unbound');
});

it('should not bind class to @Input("className")', () => {
@Component({
selector: 'my-cmp',
template: `className = {{className}}`,
})
class MyCmp {
@Input()
className: string = 'unbound';
}
@Component({template: `<my-cmp class="bound"></my-cmp>`})
class MyApp {
}

TestBed.configureTestingModule({declarations: [MyApp, MyCmp]});
const fixture = TestBed.createComponent(MyApp);
fixture.detectChanges();
expect(fixture.nativeElement.textContent).toEqual('className = unbound');
});
});
});

Expand Down

0 comments on commit 48025eb

Please sign in to comment.