Skip to content

Commit cb97ead

Browse files
Copilotnzakas
andauthored
fix: comma-separated declaration values parsed as Raw nodes (#101)
* Initial plan * Fix comma-separated declaration values parsed as Raw nodes - Import Comma and LeftCurlyBracket tokens in DeclarationList.js - Fix isElementSelectorStart() to properly handle comma-separated values - Use positive check for LeftCurlyBracket instead of negative exclusion - Ensure pseudo-class detection only returns true when followed by { - Add test cases for comma-separated declaration values Fixes regression introduced in v3.6.7 where properties like background-attachment, font-family, and transition with comma-separated values were incorrectly parsed as Raw nodes instead of Declaration nodes. Co-authored-by: nzakas <38546+nzakas@users.noreply.github.com> * Improve code clarity in isElementSelectorStart function - Restructure control flow for better readability - Handle colon case explicitly with clearer logic - Add more descriptive comments - Separate declaration and selector detection paths Co-authored-by: nzakas <38546+nzakas@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: nzakas <38546+nzakas@users.noreply.github.com>
1 parent 8b1069e commit cb97ead

File tree

2 files changed

+124
-7
lines changed

2 files changed

+124
-7
lines changed

fixtures/ast/declarationList/DeclarationList.json

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -292,5 +292,108 @@
292292
}
293293
]
294294
}
295+
},
296+
"comma-separated values": {
297+
"source": "background-attachment: scroll, scroll;",
298+
"generate": "background-attachment:scroll,scroll",
299+
"ast": {
300+
"type": "DeclarationList",
301+
"children": [
302+
{
303+
"type": "Declaration",
304+
"important": false,
305+
"property": "background-attachment",
306+
"value": {
307+
"type": "Value",
308+
"children": [
309+
{
310+
"type": "Identifier",
311+
"name": "scroll"
312+
},
313+
{
314+
"type": "Operator",
315+
"value": ","
316+
},
317+
{
318+
"type": "Identifier",
319+
"name": "scroll"
320+
}
321+
]
322+
}
323+
}
324+
]
325+
}
326+
},
327+
"multiple comma-separated values": {
328+
"source": "font-family: Arial, sans-serif;",
329+
"generate": "font-family:Arial,sans-serif",
330+
"ast": {
331+
"type": "DeclarationList",
332+
"children": [
333+
{
334+
"type": "Declaration",
335+
"important": false,
336+
"property": "font-family",
337+
"value": {
338+
"type": "Value",
339+
"children": [
340+
{
341+
"type": "Identifier",
342+
"name": "Arial"
343+
},
344+
{
345+
"type": "Operator",
346+
"value": ","
347+
},
348+
{
349+
"type": "Identifier",
350+
"name": "sans-serif"
351+
}
352+
]
353+
}
354+
}
355+
]
356+
}
357+
},
358+
"comma-separated with dimensions": {
359+
"source": "transition: opacity 1s, transform 0.5s;",
360+
"generate": "transition:opacity 1s,transform 0.5s",
361+
"ast": {
362+
"type": "DeclarationList",
363+
"children": [
364+
{
365+
"type": "Declaration",
366+
"important": false,
367+
"property": "transition",
368+
"value": {
369+
"type": "Value",
370+
"children": [
371+
{
372+
"type": "Identifier",
373+
"name": "opacity"
374+
},
375+
{
376+
"type": "Dimension",
377+
"value": "1",
378+
"unit": "s"
379+
},
380+
{
381+
"type": "Operator",
382+
"value": ","
383+
},
384+
{
385+
"type": "Identifier",
386+
"name": "transform"
387+
},
388+
{
389+
"type": "Dimension",
390+
"value": "0.5",
391+
"unit": "s"
392+
}
393+
]
394+
}
395+
}
396+
]
397+
}
295398
}
296399
}

lib/syntax/node/DeclarationList.js

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,9 @@ import {
88
LeftSquareBracket,
99
Colon,
1010
Ident,
11-
RightCurlyBracket
11+
RightCurlyBracket,
12+
LeftCurlyBracket,
13+
Comma
1214
} from '../../tokenizer/index.js';
1315

1416
const AMPERSAND = 0x0026; // U+0026 AMPERSAND (&)
@@ -38,12 +40,17 @@ function isElementSelectorStart() {
3840

3941
const nextTokenType = this.lookupTypeNonSC(1);
4042

41-
// Simple case: if next token is not colon, semicolon, or closing brace, it's likely a selector
42-
if (nextTokenType !== Colon && nextTokenType !== Semicolon && nextTokenType !== RightCurlyBracket) {
43+
// If next token is a left curly bracket, it's definitely a selector (e.g., "div { ... }")
44+
if (nextTokenType === LeftCurlyBracket) {
4345
return true;
4446
}
4547

46-
// Special handling for colon case - could be pseudo-class/pseudo-element
48+
// If next token is semicolon, comma, or closing brace, it's definitely a declaration
49+
if (nextTokenType === Semicolon || nextTokenType === Comma || nextTokenType === RightCurlyBracket) {
50+
return false;
51+
}
52+
53+
// Special handling for colon case - could be pseudo-class/pseudo-element or property
4754
if (nextTokenType === Colon) {
4855
// Look ahead further to see what follows the colon
4956
const afterColonType = this.lookupTypeNonSC(2);
@@ -52,12 +59,19 @@ function isElementSelectorStart() {
5259
// check what comes after that
5360
if (afterColonType === Ident) {
5461
const afterPseudoType = this.lookupTypeNonSC(3);
55-
// If it's followed by { or other selector tokens, it's a selector
56-
// If it's followed by ; or } or EOF, it's not a selector (property)
57-
return afterPseudoType !== Semicolon && afterPseudoType !== RightCurlyBracket && afterPseudoType !== 0; // 0 is EOF
62+
// If it's followed by {, it's definitely a selector (e.g., "div:hover { ... }")
63+
if (afterPseudoType === LeftCurlyBracket) {
64+
return true;
65+
}
66+
// Otherwise, it's a property (e.g., "transition: opacity 1s")
67+
return false;
5868
}
69+
// If after colon there's not an identifier, it's a property
70+
return false;
5971
}
6072

73+
// For other token types (dimensions, numbers, strings, etc.), assume it's a declaration
74+
// This handles cases like multi-value properties
6175
return false;
6276
}
6377

0 commit comments

Comments
 (0)