fix(common): allow null/undefined to be passed to ngClass input (#39280) (#46906)

imirkin · AndrewKushnir · commit e2ab99b95efd · 2022-08-01T12:55:13.000-07:00
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
diff --git 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<string> | {
         [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
@@ -61,7 +61,7 @@ export class NgClass implements DoCheck {
   }
 
   @Input('ngClass')
-  set ngClass(value: string|string[]|Set<string>|{[klass: string]: any}) {
+  set ngClass(value: string|string[]|Set<string>|{[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
@@ -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('<div [ngClass]="objExpr"></div>');
+
+           detectChangesAndExpectClassName('foo');
+
+           getComponent().objExpr = undefined;
+           detectChangesAndExpectClassName('');
+
+           getComponent().objExpr = {'foo': false, 'bar': true};
+           detectChangesAndExpectClassName('bar');
+         }));
 
       it('should allow multiple classes per expression', waitForAsync(() => {
            fixture = createTestComponent('<div [ngClass]="objExpr"></div>');
@@ -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(`<div [ngClass]="strExpr"></div>`);
+           detectChangesAndExpectClassName('foo');
+
+           getComponent().strExpr = undefined;
+           detectChangesAndExpectClassName('');
+         }));
+
       it('should take initial classes into account when switching from string to null',
          waitForAsync(() => {
            fixture = createTestComponent(`<div class="foo" [ngClass]="strExpr"></div>`);
@@ -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(`<div class="foo" [ngClass]="strExpr"></div>`);
+           detectChangesAndExpectClassName('foo');
+
+           getComponent().strExpr = undefined;
+           detectChangesAndExpectClassName('foo');
+         }));
+
       it('should ignore empty and blank strings', waitForAsync(() => {
            fixture = createTestComponent(`<div class="foo" [ngClass]="strExpr"></div>`);
            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<string> = new Set<string>();
-  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');

Original file line number	Diff line number	Diff line change
`@@ -61,7 +61,7 @@ export class NgClass implements DoCheck {`
`61`	`61`	`}`
`62`	`62`
`63`	`63`	`@Input('ngClass')`
`64`		`- set ngClass(value: string\|string[]\|Set<string>\|{[klass: string]: any}) {`
	`64`	`+ set ngClass(value: string\|string[]\|Set<string>\|{[klass: string]: any}\|null\|undefined) {`
`65`	`65`	`this._removeClasses(this._rawClass);`
`66`	`66`	`this._applyClasses(this._initialClasses);`
`67`	`67`