From 4d04fdc463580cf109623bda5c3112069c63f186 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mi=C5=A1ko=20Hevery?= Date: Tue, 25 Feb 2020 09:28:13 -0800 Subject: [PATCH 1/3] style: Reformat test comment for line breaks --- packages/core/test/acceptance/styling_spec.ts | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/packages/core/test/acceptance/styling_spec.ts b/packages/core/test/acceptance/styling_spec.ts index 63278e8bcf399..a73e8e2fa540d 100644 --- a/packages/core/test/acceptance/styling_spec.ts +++ b/packages/core/test/acceptance/styling_spec.ts @@ -3460,13 +3460,12 @@ describe('styling', () => { class MyApp { // When the first view in the list gets CD-ed, everything works. // When the second view gets CD-ed, the styling has already created the data structures - // in the `TView`. As a result when - // `[class.foo]` runs it already knows that `[class]` is a duplicate and hence it - // should check with it. While the resolution is happening it reads the value of the - // `[class]`, however `[class]` has not yet executed and therefore it does not have - // normalized value in its `LView`. The result is that the assertions fails as it - // expects an - // `KeyValueArray`. + // in the `TView`. As a result when `[class.foo]` runs it already knows that `[class]` + // is a duplicate and hence it can overwrite the `[class.foo]` binding. While the + // styling resolution is happening the algorithm reads the value of the `[class]` + // (because it overwrites `[class.foo]`), however `[class]` has not yet executed and + // therefore it does not have normalized value in its `LView`. The result is that the + // assertions fails as it expects an `KeyValueArray`. } TestBed.configureTestingModule({declarations: [MyApp, MyCmp, HostStylingsDir]}); From 67e2a7973ebd24d929984604e0fdb1973a7e0b6a 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 2/3] fix(core): treat `[class]` and `[className]` as unrelated bindings 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 --- .../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 | 51 ++++++++----------- 4 files changed, 25 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..fa210759148f5 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 to a `class` input binding, when static `class` is present', () => { @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,25 @@ 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'); + }); }); }); From a072342a855b61b4079e0afe8aa2b51605df5b3d Mon Sep 17 00:00:00 2001 From: Misko Hevery Date: Fri, 28 Feb 2020 16:02:23 -0800 Subject: [PATCH 3/3] fixup! fix(core): treat `[class]` and `[className]` as unrelated bindings --- packages/core/test/acceptance/styling_spec.ts | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/packages/core/test/acceptance/styling_spec.ts b/packages/core/test/acceptance/styling_spec.ts index fa210759148f5..02397afb41e43 100644 --- a/packages/core/test/acceptance/styling_spec.ts +++ b/packages/core/test/acceptance/styling_spec.ts @@ -1129,7 +1129,7 @@ describe('styling', () => { }); onlyInIvy('only ivy combines static and dynamic class-related attr values') - .it('should write to a `class` 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}}`, @@ -3468,6 +3468,25 @@ describe('styling', () => { 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'); + }); }); });