Skip to content

Commit

Permalink
fix: do not serialize methods of objects passed to styles
Browse files Browse the repository at this point in the history
  • Loading branch information
jakovljevic-mladen committed Oct 3, 2024
1 parent 7de4674 commit e7d826c
Show file tree
Hide file tree
Showing 4 changed files with 165 additions and 4 deletions.
5 changes: 5 additions & 0 deletions .changeset/tender-files-press.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@builder.io/qwik': patch
---

Do not allow object methods to be serialized with style prop
2 changes: 1 addition & 1 deletion packages/qwik/src/core/error/error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ export const codeToText = (code: number, ...parts: any[]): string => {
if (qDev) {
// Keep one error, one line to make it easier to search for the error message.
const MAP = [
'Error while serializing class attribute', // 0
'Error while serializing class or style attributes', // 0
'Can not serialize a HTML Node that is not an Element', // 1
'Runtime but no instance found on element.', // 2
'Only primitive and object literals can be serialized', // 3
Expand Down
4 changes: 2 additions & 2 deletions packages/qwik/src/core/render/execute-component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ export const stringifyStyle = (obj: any): string => {
for (const key in obj) {
if (Object.prototype.hasOwnProperty.call(obj, key)) {
const value = obj[key];
if (value != null) {
if (value != null && typeof value !== 'function') {
if (key.startsWith('--')) {
chunks.push(key + ':' + value);
} else {
Expand All @@ -201,7 +201,7 @@ export const stringifyStyle = (obj: any): string => {
return String(obj);
};

const setValueForStyle = (styleName: string, value: any) => {
export const setValueForStyle = (styleName: string, value: any) => {
if (typeof value === 'number' && value !== 0 && !isUnitlessNumber(styleName)) {
return value + 'px';
}
Expand Down
158 changes: 157 additions & 1 deletion packages/qwik/src/core/render/execute-component.unit.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
import { suite, test, assert } from 'vitest';
import { serializeClass, serializeClassWithHost } from './execute-component';
import {
serializeClass,
serializeClassWithHost,
stringifyStyle,
setValueForStyle,
} from './execute-component';
import type { QContext } from '../state/context';

const obj = {
Expand Down Expand Up @@ -177,3 +182,154 @@ suite('serializeClassWithHost', () => {
);
});
});

suite('stringifyStyle', () => {
test('should stringify null', () => {
assert.equal(stringifyStyle(null), '');
});

test('should stringify undefined', () => {
assert.equal(stringifyStyle(undefined), '');
});

test('should stringify string', () => {
assert.equal(stringifyStyle('color: red;'), 'color: red;');
});

test('should stringify number', () => {
assert.equal(stringifyStyle(10), '10');
});

test('should stringify boolean', () => {
assert.equal(stringifyStyle(true), 'true');
assert.equal(stringifyStyle(false), 'false');
});

suite('object values', () => {
test('should throw an error for array', () => {
assert.throws(
() => stringifyStyle([]),
'Code(0): Error while serializing class or style attributes'
);
});

suite('regular objects', () => {
test('should stringify object with nullish values', () => {
assert.equal(stringifyStyle({ color: null, backgroundColor: undefined }), '');
});

test('should stringify empty object', () => {
assert.equal(stringifyStyle({}), '');
});

test('should stringify object with single property', () => {
assert.equal(stringifyStyle({ color: 'red' }), 'color:red');
});

test('should stringify object with multiple properties', () => {
assert.equal(
stringifyStyle({ color: 'red', 'background-color': 'blue' }),
'color:red;background-color:blue'
);
});

test('should stringify object with numeric values', () => {
assert.equal(stringifyStyle({ width: 10, height: 20 }), 'width:10px;height:20px');
});

test('should convert camelCase to kebab-case', () => {
assert.equal(stringifyStyle({ backgroundColor: 'blue' }), 'background-color:blue');
});

test('should stringify object with unitless properties', () => {
assert.equal(stringifyStyle({ lineHeight: 1.5 }), 'line-height:1.5');
});

test('should stringify properties that start with two dashes', () => {
assert.equal(stringifyStyle({ '--foo': 'bar' }), '--foo:bar');
});

test('should stringify properties with numeric values that start with two dashes', () => {
assert.equal(stringifyStyle({ '--foo': 10 }), '--foo:10');
});
});

suite('objects with methods', () => {
test('should stringify object with own properties only', () => {
const obj = Object.create({ color: 'red' });
obj.marginTop = '10em';
assert.equal(obj.color, 'red');
assert.equal(stringifyStyle(obj), 'margin-top:10em');
});

test('should ignore object methods', () => {
const obj = {
margin: () => 10,
color: 'red',
backgroundColor: 'blue',
};
assert.equal(stringifyStyle(obj), 'color:red;background-color:blue');
});

test('should stringify object with custom hasOwnProperty method', () => {
const obj = {
hasOwnProperty: () => false,
color: 'red',
backgroundColor: 'blue',
};
assert.equal(stringifyStyle(obj), 'color:red;background-color:blue');
});
});
});
});

suite('setValueForStyle', () => {
suite('properties with units', () => {
test('should not add "px" to numeric zero value (= 0)', () => {
assert.equal(setValueForStyle('margin', 0), '0');
});

test('should add "px" to numeric value that is non-zero (<> 0)', () => {
assert.equal(setValueForStyle('margin', 10), '10px');
});

test('should not add "px" to string zero value (= "0")', () => {
assert.equal(setValueForStyle('margin', '0'), '0');
});

test('should not add "px" to string zero value ending with "px" (= "0px")', () => {
assert.equal(setValueForStyle('margin', '0px'), '0px');
});

test('should not add "px" to string zero value ending with "rem" (= "0rem")', () => {
assert.equal(setValueForStyle('margin', '0rem'), '0rem');
});

test('should not add "px" to string value that is word (= "red")', () => {
assert.equal(setValueForStyle('color', 'red'), 'red');
});

test('should not add "px" to string value that is non-zero ending with "px" (= "10px")', () => {
assert.equal(setValueForStyle('margin', '10px'), '10px');
});

test('should not add "px" to string value that is non-zero ending with "rem" (= "10rem")', () => {
assert.equal(setValueForStyle('margin', '10rem'), '10rem');
});
});

suite('unitless properties', () => {
test('should not add "px" to numeric zero value (= 0)', () => {
assert.equal(setValueForStyle('lineHeight', 0), '0');
});

test('should not add "px" to numeric value that is non-zero (<> 0)', () => {
assert.equal(setValueForStyle('lineHeight', 10), '10');
});

test('should not add "px" to string value', () => {
assert.equal(setValueForStyle('lineHeight', '0'), '0');
assert.equal(setValueForStyle('lineHeight', '10'), '10');
});
});
});

0 comments on commit e7d826c

Please sign in to comment.