From e2ab99b95efd893c49d15c02cccd72ef82ea1cae Mon Sep 17 00:00:00 2001 From: Ilia Mirkin Date: Wed, 20 Jul 2022 11:37:07 -0400 Subject: [PATCH] fix(common): allow null/undefined to be passed to ngClass input (#39280) (#46906) With strict template type checking, a null/undefined value will raise an error. However the implementation is completely fine with it, and it would be pointless to "fix" it at the callsite and convert to e.g. an empty string. Allow all of the "supported types" to be passed in directly to ngClass. Fixes #39280 PR Close #46906 --- goldens/public-api/common/index.md | 2 +- packages/common/src/directives/ng_class.ts | 2 +- .../common/test/directives/ng_class_spec.ts | 45 ++++++++++++++++++- 3 files changed, 45 insertions(+), 4 deletions(-) diff --git a/goldens/public-api/common/index.md b/goldens/public-api/common/index.md index a59371b0e2aa8..2b23b1ede4030 100644 --- a/goldens/public-api/common/index.md +++ b/goldens/public-api/common/index.md @@ -403,7 +403,7 @@ export class NgClass implements DoCheck { // (undocumented) set ngClass(value: string | string[] | Set | { [klass: string]: any; - }); + } | null | undefined); // (undocumented) ngDoCheck(): void; // (undocumented) diff --git a/packages/common/src/directives/ng_class.ts b/packages/common/src/directives/ng_class.ts index 9c9d815b18de8..3eb0161dca9b0 100644 --- a/packages/common/src/directives/ng_class.ts +++ b/packages/common/src/directives/ng_class.ts @@ -61,7 +61,7 @@ export class NgClass implements DoCheck { } @Input('ngClass') - set ngClass(value: string|string[]|Set|{[klass: string]: any}) { + set ngClass(value: string|string[]|Set|{[klass: string]: any}|null|undefined) { this._removeClasses(this._rawClass); this._applyClasses(this._initialClasses); diff --git a/packages/common/test/directives/ng_class_spec.ts b/packages/common/test/directives/ng_class_spec.ts index 353e272244068..19e2fe3c5e8af 100644 --- a/packages/common/test/directives/ng_class_spec.ts +++ b/packages/common/test/directives/ng_class_spec.ts @@ -115,6 +115,17 @@ import {ComponentFixture, TestBed, waitForAsync} from '@angular/core/testing'; detectChangesAndExpectClassName('bar'); })); + it('should remove active classes when expression evaluates to undefined', waitForAsync(() => { + fixture = createTestComponent('
'); + + detectChangesAndExpectClassName('foo'); + + getComponent().objExpr = undefined; + detectChangesAndExpectClassName(''); + + getComponent().objExpr = {'foo': false, 'bar': true}; + detectChangesAndExpectClassName('bar'); + })); it('should allow multiple classes per expression', waitForAsync(() => { fixture = createTestComponent('
'); @@ -246,6 +257,15 @@ import {ComponentFixture, TestBed, waitForAsync} from '@angular/core/testing'; detectChangesAndExpectClassName(''); })); + it('should remove active classes when switching from string to undefined', + waitForAsync(() => { + fixture = createTestComponent(`
`); + detectChangesAndExpectClassName('foo'); + + getComponent().strExpr = undefined; + detectChangesAndExpectClassName(''); + })); + it('should take initial classes into account when switching from string to null', waitForAsync(() => { fixture = createTestComponent(`
`); @@ -255,6 +275,15 @@ import {ComponentFixture, TestBed, waitForAsync} from '@angular/core/testing'; detectChangesAndExpectClassName('foo'); })); + it('should take initial classes into account when switching from string to undefined', + waitForAsync(() => { + fixture = createTestComponent(`
`); + detectChangesAndExpectClassName('foo'); + + getComponent().strExpr = undefined; + detectChangesAndExpectClassName('foo'); + })); + it('should ignore empty and blank strings', waitForAsync(() => { fixture = createTestComponent(`
`); getComponent().strExpr = ''; @@ -275,6 +304,9 @@ import {ComponentFixture, TestBed, waitForAsync} from '@angular/core/testing'; getComponent().objExpr = null; detectChangesAndExpectClassName('init foo'); + + getComponent().objExpr = undefined; + detectChangesAndExpectClassName('init foo'); })); it('should co-operate with the interpolated class attribute', waitForAsync(() => { @@ -289,6 +321,9 @@ import {ComponentFixture, TestBed, waitForAsync} from '@angular/core/testing'; getComponent().objExpr = null; detectChangesAndExpectClassName(`init foo`); + + getComponent().objExpr = undefined; + detectChangesAndExpectClassName(`init foo`); })); it('should co-operate with the interpolated class attribute when interpolation changes', @@ -315,6 +350,9 @@ import {ComponentFixture, TestBed, waitForAsync} from '@angular/core/testing'; getComponent().objExpr = null; detectChangesAndExpectClassName(`init foo`); + + getComponent().objExpr = undefined; + detectChangesAndExpectClassName(`init foo`); })); it('should co-operate with the class attribute and class.name binding', waitForAsync(() => { @@ -351,6 +389,9 @@ import {ComponentFixture, TestBed, waitForAsync} from '@angular/core/testing'; cmp.objExpr = null; detectChangesAndExpectClassName('init baz'); + + cmp.objExpr = undefined; + detectChangesAndExpectClassName('init baz'); })); }); @@ -419,8 +460,8 @@ class TestComponent { items: any[]|undefined; arrExpr: string[] = ['foo']; setExpr: Set = new Set(); - objExpr: {[klass: string]: any}|null = {'foo': true, 'bar': false}; - strExpr: string|null = 'foo'; + objExpr: {[klass: string]: any}|null|undefined = {'foo': true, 'bar': false}; + strExpr: string|null|undefined = 'foo'; constructor() { this.setExpr.add('foo');