Skip to content

Commit 37bb130

Browse files
authored
Improve string conversion
The previous string conversion was over-complicated, and did not match the specs in that it threw TypeErrors unnecessarily. See #129 and jsdom/jsdom#4010.
1 parent f764e2d commit 37bb130

3 files changed

Lines changed: 11 additions & 101 deletions

File tree

lib/parsers.js

Lines changed: 7 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -96,33 +96,15 @@ const lruCache = new LRUCache({
9696
* Prepares a stringified value.
9797
*
9898
* @param {string|number|null|undefined} value - The value to prepare.
99-
* @param {object} [globalObject=globalThis] - The global object.
10099
* @returns {string} The prepared value.
101100
*/
102-
const prepareValue = (value, globalObject = globalThis) => {
101+
const prepareValue = (value) => {
103102
// `null` is converted to an empty string.
104103
// @see https://webidl.spec.whatwg.org/#LegacyNullToEmptyString
105104
if (value === null) {
106105
return "";
107106
}
108-
const type = typeof value;
109-
switch (type) {
110-
case "string":
111-
return value.trim();
112-
case "number":
113-
return value.toString();
114-
case "undefined":
115-
return "undefined";
116-
case "symbol":
117-
throw new globalObject.TypeError("Can not convert symbol to string.");
118-
default: {
119-
const str = value.toString();
120-
if (typeof str === "string") {
121-
return str.trim();
122-
}
123-
throw new globalObject.TypeError(`Can not convert ${type} to string.`);
124-
}
125-
}
107+
return `${value}`.trim();
126108
};
127109

128110
/**
@@ -164,9 +146,7 @@ const hasCalcFunc = (val) => {
164146
* @returns {object} The AST or a plain object.
165147
*/
166148
const parseCSS = (val, opt, toObject = false) => {
167-
if (typeof val !== "string") {
168-
val = prepareValue(val);
169-
}
149+
val = prepareValue(val);
170150
const ast = cssTree.parse(val, opt);
171151
if (toObject) {
172152
return cssTree.toPlainObject(ast);
@@ -183,9 +163,7 @@ const parseCSS = (val, opt, toObject = false) => {
183163
* @returns {boolean} True if the value is valid, false otherwise.
184164
*/
185165
const isValidPropertyValue = (prop, val) => {
186-
if (typeof val !== "string") {
187-
val = prepareValue(val);
188-
}
166+
val = prepareValue(val);
189167
if (val === "") {
190168
return true;
191169
}
@@ -225,9 +203,7 @@ const isValidPropertyValue = (prop, val) => {
225203
* @returns {string|undefined} The resolved value.
226204
*/
227205
const resolveCalc = (val, opt = { format: "specifiedValue" }) => {
228-
if (typeof val !== "string") {
229-
val = prepareValue(val);
230-
}
206+
val = prepareValue(val);
231207
if (val === "" || hasVarFunc(val) || !hasCalcFunc(val)) {
232208
return val;
233209
}
@@ -276,8 +252,8 @@ const resolveCalc = (val, opt = { format: "specifiedValue" }) => {
276252
* @returns {string|Array<object>|undefined} The parsed value.
277253
*/
278254
const parsePropertyValue = (prop, val, opt = {}) => {
279-
const { caseSensitive, globalObject, inArray } = opt;
280-
val = prepareValue(val, globalObject);
255+
const { caseSensitive, inArray } = opt;
256+
val = prepareValue(val);
281257
if (val === "" || hasVarFunc(val)) {
282258
return val;
283259
} else if (hasCalcFunc(val)) {

test/CSSStyleDeclaration.test.js

Lines changed: 1 addition & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1493,24 +1493,7 @@ describe("regression test for https://github.com/jsdom/cssstyle/issues/129", ()
14931493

14941494
it("throws for setting Symbol", () => {
14951495
const style = new CSSStyleDeclaration();
1496-
assert.throws(
1497-
() => style.setProperty("width", Symbol("foo")),
1498-
(e) => {
1499-
assert.strictEqual(e instanceof TypeError, true);
1500-
assert.strictEqual(e.message, "Can not convert symbol to string.");
1501-
return true;
1502-
}
1503-
);
1504-
assert.throws(
1505-
() => {
1506-
style.width = Symbol("foo");
1507-
},
1508-
(e) => {
1509-
assert.strictEqual(e instanceof TypeError, true);
1510-
assert.strictEqual(e.message, "Can not convert symbol to string.");
1511-
return true;
1512-
}
1513-
);
1496+
assert.throws(() => style.setProperty("width", Symbol("foo")));
15141497
});
15151498
});
15161499

test/parsers.test.js

Lines changed: 3 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -57,63 +57,14 @@ describe("prepareValue", () => {
5757
it("should throw for Symbol", () => {
5858
const input = Symbol("foo");
5959

60-
assert.throws(
61-
() => parsers.prepareValue(input),
62-
(e) => {
63-
assert.strictEqual(e instanceof globalThis.TypeError, true);
64-
assert.strictEqual(e.message, "Can not convert symbol to string.");
65-
return true;
66-
}
67-
);
60+
assert.throws(() => parsers.prepareValue(input));
6861
});
6962

70-
it("should throw with window global for Symbol", () => {
71-
const input = Symbol("foo");
72-
const window = {
73-
TypeError: globalThis.TypeError
74-
};
75-
76-
assert.throws(
77-
() => parsers.prepareValue(input, window),
78-
(e) => {
79-
assert.strictEqual(e instanceof window.TypeError, true);
80-
assert.strictEqual(e.message, "Can not convert symbol to string.");
81-
return true;
82-
}
83-
);
84-
});
85-
86-
it("should throw for object.toString() not converting to string", () => {
63+
it("should get string even if object.toString() not converting to string", () => {
8764
const input = {
8865
toString: () => [1]
8966
};
90-
91-
assert.throws(
92-
() => parsers.prepareValue(input),
93-
(e) => {
94-
assert.strictEqual(e instanceof globalThis.TypeError, true);
95-
assert.strictEqual(e.message, "Can not convert object to string.");
96-
return true;
97-
}
98-
);
99-
});
100-
101-
it("should throw for object.toString() not converting to string", () => {
102-
const input = {
103-
toString: () => [1]
104-
};
105-
const window = {
106-
TypeError: globalThis.TypeError
107-
};
108-
109-
assert.throws(
110-
() => parsers.prepareValue(input, window),
111-
(e) => {
112-
assert.strictEqual(e instanceof window.TypeError, true);
113-
assert.strictEqual(e.message, "Can not convert object to string.");
114-
return true;
115-
}
116-
);
67+
assert.throws(() => parsers.prepareValue(input));
11768
});
11869
});
11970

0 commit comments

Comments
 (0)