fix(core): handle invalid classes in class array bindings (#49924)

When binding an array to `class` like `[class]="['foo', 'bar']"`, the runtime treats it the same as a literal binding with all the values being `true`, e.g. `{foo: true, bar: true}`. While object literals can only have string keys, arrays can have any value which can lead to errors if the array contains non-string values.

These changes add some logic to stringify the keys and ignore invalid ones.

Fixes #48473.

PR Close #49924
This commit is contained in:
Kristiyan Kostadinov
2023-04-19 10:06:56 +02:00
committed by Jessica Janiuk
parent 0454158084
commit 2fff8fadbe
2 changed files with 151 additions and 1 deletions
@@ -136,7 +136,7 @@ export function styleStringParser(keyValueArray: KeyValueArray<any>, text: strin
*/
export function ɵɵclassMap(classes: {[className: string]: boolean|undefined|null}|string|undefined|
null): void {
checkStylingMap(keyValueArraySet, classStringParser, classes, true);
checkStylingMap(classKeyValueArraySet, classStringParser, classes, true);
}
/**
@@ -616,6 +616,27 @@ export function styleKeyValueArraySet(keyValueArray: KeyValueArray<any>, key: st
keyValueArraySet(keyValueArray, key, unwrapSafeValue(value));
}
/**
* Class-binding-specific function for setting the `value` for a `key`.
*
* See: `keyValueArraySet` for details
*
* @param keyValueArray KeyValueArray to add to.
* @param key Style key to add.
* @param value The value to set.
*/
export function classKeyValueArraySet(keyValueArray: KeyValueArray<any>, key: unknown, value: any) {
// We use `classList.add` to eventually add the CSS classes to the DOM node. Any value passed into
// `add` is stringified and added to the `class` attribute, e.g. even null, undefined or numbers
// will be added. Stringify the key here so that our internal data structure matches the value in
// the DOM. The only exceptions are empty strings and strings that contain spaces for which
// the browser throws an error. We ignore such values, because the error is somewhat cryptic.
const stringKey = String(key);
if (stringKey !== '' && !stringKey.includes(' ')) {
keyValueArraySet(keyValueArray, stringKey, value);
}
}
/**
* Update map based styling.
*
@@ -300,6 +300,135 @@ describe('styling', () => {
});
});
describe('non-string class keys', () => {
it('should allow null in a class array binding', () => {
@Component({
template: `<div [class]="['a', null, 'c']" [class.extra]="true"></div>`,
standalone: true,
})
class Cmp {
}
const fixture = TestBed.createComponent(Cmp);
fixture.detectChanges();
const div = fixture.nativeElement.querySelector('div');
expect(div.getAttribute('class')).toBe('a c null extra');
});
it('should allow undefined in a class array binding', () => {
@Component({
template: `<div [class]="['a', undefined, 'c']" [class.extra]="true"></div>`,
standalone: true,
})
class Cmp {
}
const fixture = TestBed.createComponent(Cmp);
fixture.detectChanges();
const div = fixture.nativeElement.querySelector('div');
expect(div.getAttribute('class')).toBe('a c undefined extra');
});
it('should allow zero in a class array binding', () => {
@Component({
template: `<div [class]="['a', 0, 'c']" [class.extra]="true"></div>`,
standalone: true,
})
class Cmp {
}
const fixture = TestBed.createComponent(Cmp);
fixture.detectChanges();
const div = fixture.nativeElement.querySelector('div');
expect(div.getAttribute('class')).toBe('0 a c extra');
});
it('should allow false in a class array binding', () => {
@Component({
template: `<div [class]="['a', false, 'c']" [class.extra]="true"></div>`,
standalone: true,
})
class Cmp {
}
const fixture = TestBed.createComponent(Cmp);
fixture.detectChanges();
const div = fixture.nativeElement.querySelector('div');
expect(div.getAttribute('class')).toBe('a c false extra');
});
it('should ignore an empty string in a class array binding', () => {
@Component({
template: `<div [class]="['a', '', 'c']" [class.extra]="true"></div>`,
standalone: true,
})
class Cmp {
}
const fixture = TestBed.createComponent(Cmp);
fixture.detectChanges();
const div = fixture.nativeElement.querySelector('div');
expect(div.getAttribute('class')).toBe('a c extra');
});
it('should ignore a string containing spaces in a class array binding', () => {
@Component({
template: `<div [class]="['a', 'hello there', 'c']" [class.extra]="true"></div>`,
standalone: true,
})
class Cmp {
}
const fixture = TestBed.createComponent(Cmp);
fixture.detectChanges();
const div = fixture.nativeElement.querySelector('div');
expect(div.getAttribute('class')).toBe('a c extra');
});
it('should ignore a string containing spaces in a class object literal binding', () => {
@Component({
template:
`<div [class]="{a: true, 'hello there': true, c: true}" [class.extra]="true"></div>`,
standalone: true,
})
class Cmp {
}
const fixture = TestBed.createComponent(Cmp);
fixture.detectChanges();
const div = fixture.nativeElement.querySelector('div');
expect(div.getAttribute('class')).toBe('a c extra');
});
it('should ignore an object literal in a class array binding', () => {
@Component({
template: `<div [class]="['a', {foo: true}, 'c']" [class.extra]="true"></div>`,
standalone: true,
})
class Cmp {
}
const fixture = TestBed.createComponent(Cmp);
fixture.detectChanges();
const div = fixture.nativeElement.querySelector('div');
expect(div.getAttribute('class')).toBe('a c extra');
});
it('should handle a string array in a class array binding', () => {
@Component({
template: `<div [class]="['a', ['foo', 'bar'], 'c']" [class.extra]="true"></div>`,
standalone: true,
})
class Cmp {
}
const fixture = TestBed.createComponent(Cmp);
fixture.detectChanges();
const div = fixture.nativeElement.querySelector('div');
expect(div.getAttribute('class')).toBe('a c foo,bar extra');
});
});
it('should bind [class] as input to directive', () => {
@Component({
template: `