From 48025eb3e835e8ffaf94afc3f53b726baeaa4050 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mi=C5=A1ko=20Hevery?= Date: Tue, 25 Feb 2020 10:31:01 -0800 Subject: [PATCH] fix(core): treat `[class]` and `[className]` as unrelated bindings (#35668) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../src/render3/view/styling_builder.ts | 3 +- .../core/src/render3/instructions/property.ts | 3 +- .../core/src/render3/instructions/shared.ts | 2 +- packages/core/test/acceptance/styling_spec.ts | 70 +++++++++++-------- 4 files changed, 44 insertions(+), 34 deletions(-) diff --git a/packages/compiler/src/render3/view/styling_builder.ts b/packages/compiler/src/render3/view/styling_builder.ts index 46252453c8ad8..2ac2d6114ee93 100644 --- a/packages/compiler/src/render3/view/styling_builder.ts +++ b/packages/compiler/src/render3/view/styling_builder.ts @@ -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 diff --git a/packages/core/src/render3/instructions/property.ts b/packages/core/src/render3/instructions/property.ts index dda96ec0534f1..e02a8808a4033 100644 --- a/packages/core/src/render3/instructions/property.ts +++ b/packages/core/src/render3/instructions/property.ts @@ -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); } diff --git a/packages/core/src/render3/instructions/shared.ts b/packages/core/src/render3/instructions/shared.ts index ff00a5679cc4e..8c0fa14d48bc0 100644 --- a/packages/core/src/render3/instructions/shared.ts +++ b/packages/core/src/render3/instructions/shared.ts @@ -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')) { diff --git a/packages/core/test/acceptance/styling_spec.ts b/packages/core/test/acceptance/styling_spec.ts index a73e8e2fa540d..02397afb41e43 100644 --- a/packages/core/test/acceptance/styling_spec.ts +++ b/packages/core/test/acceptance/styling_spec.ts @@ -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: ``, + template: ``, }) class App { } @@ -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: ``, - }) - 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', () => { @@ -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: ``}) + 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: ``}) + class MyApp { + } + + TestBed.configureTestingModule({declarations: [MyApp, MyCmp]}); + const fixture = TestBed.createComponent(MyApp); + fixture.detectChanges(); + expect(fixture.nativeElement.textContent).toEqual('className = unbound'); + }); }); });