Skip to content

Commit 355d077

Browse files
bartvenemanclaude
andauthored
Fix value property to return undefined for nodes without value concept (#260)
## Summary Fixed a regression where certain CSS node types were incorrectly returning `null` instead of `undefined` for their `value` property. This distinction is semantically important: `null` means "has a value, but it is absent", while `undefined` means "value is not a property of this node type". ## Key Changes - **Updated `CSSNode.value` getter** in `src/css-node.ts` to explicitly return `undefined` for node types that don't have a value concept, rather than attempting to read from the arena value fields - **Added comprehensive regression tests** in `src/api.test.ts` covering 20+ node types that should return `undefined` for their value property, including: - Structural nodes: StyleSheet, Rule, Block, SelectorList, Selector - Selector types: TypeSelector, ClassSelector, IdSelector, PseudoClassSelector, PseudoElementSelector, UniversalSelector, Combinator, Nth - Value nodes: Value, Identifier, Hash, String - At-rule nodes: Atrule, MediaQuery - Other nodes: Raw ## Implementation Details The fix adds an explicit type check that returns `undefined` early for all node types except the five that actually store values in the arena: - `DECLARATION` - `FUNCTION` - `ATTRIBUTE_SELECTOR` - `SUPPORTS_QUERY` - `PRELUDE_SELECTORLIST` This ensures the correct semantic distinction between nodes that have no value concept (undefined) and nodes that have a value but it's empty (null). https://claude.ai/code/session_01KETKttcckhFQtg8JzjxvuU --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 51f4516 commit 355d077

2 files changed

Lines changed: 180 additions & 1 deletion

File tree

src/api.test.ts

Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1045,6 +1045,174 @@ describe('CSSNode', () => {
10451045
})
10461046
})
10471047

1048+
describe('value property - undefined for nodes without a value concept', () => {
1049+
// Regression: these node types were incorrectly returning null instead of undefined.
1050+
// null means "has a value, but it is absent"; undefined means "value is not a property of this node type".
1051+
//
1052+
// These types intentionally omit `value` from their TypeScript interfaces, so we
1053+
// access the runtime getter via the base CSSNode class to test the JS behavior.
1054+
function get_value(node: unknown) {
1055+
return (node as CSSNode).value
1056+
}
1057+
1058+
test('StyleSheet.value is undefined', () => {
1059+
const root = parse('div { color: red; }')
1060+
expect(get_value(root)).toBeUndefined()
1061+
})
1062+
1063+
test('Rule.value is undefined', () => {
1064+
const root = parse('div { color: red; }')
1065+
const rule = root.first_child!
1066+
expect(get_value(rule)).toBeUndefined()
1067+
})
1068+
1069+
test('SelectorList.value is undefined', () => {
1070+
const root = parse('div { color: red; }')
1071+
const rule = root.first_child!
1072+
const selectorList = rule.first_child!
1073+
expect(selectorList.type_name).toBe('SelectorList')
1074+
expect(get_value(selectorList)).toBeUndefined()
1075+
})
1076+
1077+
test('Selector.value is undefined', () => {
1078+
const root = parse('div { color: red; }')
1079+
const selector = root.first_child!.first_child!.first_child!
1080+
expect(selector.type_name).toBe('Selector')
1081+
expect(get_value(selector)).toBeUndefined()
1082+
})
1083+
1084+
test('TypeSelector.value is undefined', () => {
1085+
const root = parse('div { color: red; }')
1086+
const typeSelector = root.first_child!.first_child!.first_child!.first_child!
1087+
expect(typeSelector.type_name).toBe('TypeSelector')
1088+
expect(get_value(typeSelector)).toBeUndefined()
1089+
})
1090+
1091+
test('ClassSelector.value is undefined', () => {
1092+
const root = parse('.foo { color: red; }')
1093+
const classSelector = root.first_child!.first_child!.first_child!.first_child!
1094+
expect(classSelector.type_name).toBe('ClassSelector')
1095+
expect(get_value(classSelector)).toBeUndefined()
1096+
})
1097+
1098+
test('IdSelector.value is undefined', () => {
1099+
const root = parse('#bar { color: red; }')
1100+
const idSelector = root.first_child!.first_child!.first_child!.first_child!
1101+
expect(idSelector.type_name).toBe('IdSelector')
1102+
expect(get_value(idSelector)).toBeUndefined()
1103+
})
1104+
1105+
test('PseudoClassSelector.value is undefined', () => {
1106+
const root = parse('a:hover { color: red; }')
1107+
const selector = root.first_child!.first_child!.first_child!
1108+
const pseudo = selector.first_child!.next_sibling!
1109+
expect(pseudo.type_name).toBe('PseudoClassSelector')
1110+
expect(get_value(pseudo)).toBeUndefined()
1111+
})
1112+
1113+
test('PseudoElementSelector.value is undefined', () => {
1114+
const root = parse('p::before { content: ""; }')
1115+
const selector = root.first_child!.first_child!.first_child!
1116+
const pseudo = selector.first_child!.next_sibling!
1117+
expect(pseudo.type_name).toBe('PseudoElementSelector')
1118+
expect(get_value(pseudo)).toBeUndefined()
1119+
})
1120+
1121+
test('Combinator.value is undefined', () => {
1122+
const root = parse('div > span { color: red; }')
1123+
const selector = root.first_child!.first_child!.first_child!
1124+
const combinator = selector.first_child!.next_sibling!
1125+
expect(combinator.type_name).toBe('Combinator')
1126+
expect(get_value(combinator)).toBeUndefined()
1127+
})
1128+
1129+
test('UniversalSelector.value is undefined', () => {
1130+
const root = parse('* { color: red; }')
1131+
const universal = root.first_child!.first_child!.first_child!.first_child!
1132+
expect(universal.type_name).toBe('UniversalSelector')
1133+
expect(get_value(universal)).toBeUndefined()
1134+
})
1135+
1136+
test('NthSelector.value is undefined', () => {
1137+
const root = parse('li:nth-child(2n+1) { color: red; }')
1138+
const selector = root.first_child!.first_child!.first_child!
1139+
const pseudo = selector.first_child!.next_sibling!
1140+
const nth = pseudo.first_child!
1141+
expect(nth.type_name).toBe('Nth')
1142+
expect(get_value(nth)).toBeUndefined()
1143+
})
1144+
1145+
test('Block.value is undefined', () => {
1146+
const root = parse('div { color: red; }')
1147+
const block = (root.first_child! as Rule).block!
1148+
expect(get_value(block)).toBeUndefined()
1149+
})
1150+
1151+
test('Value node.value is undefined', () => {
1152+
const root = parse('div { color: red; }')
1153+
const block = (root.first_child! as Rule).block!
1154+
const decl = block.first_child! as Declaration
1155+
const valueNode = decl.value!
1156+
expect(valueNode.type_name).toBe('Value')
1157+
expect(get_value(valueNode)).toBeUndefined()
1158+
})
1159+
1160+
test('Identifier.value is undefined', () => {
1161+
const root = parse('div { color: red; }')
1162+
const block = (root.first_child! as Rule).block!
1163+
const decl = block.first_child! as Declaration
1164+
const identifier = decl.value!.first_child!
1165+
expect(identifier.type_name).toBe('Identifier')
1166+
expect(get_value(identifier)).toBeUndefined()
1167+
})
1168+
1169+
test('Hash.value is undefined', () => {
1170+
const root = parse('div { color: #fff; }')
1171+
const block = (root.first_child! as Rule).block!
1172+
const decl = block.first_child! as Declaration
1173+
const hash = decl.value!.first_child!
1174+
expect(hash.type_name).toBe('Hash')
1175+
expect(get_value(hash)).toBeUndefined()
1176+
})
1177+
1178+
test('String.value is undefined', () => {
1179+
const root = parse('p::before { content: "hello"; }')
1180+
const block = (root.first_child! as Rule).block!
1181+
const decl = block.first_child! as Declaration
1182+
const str = decl.value!.first_child!
1183+
expect(str.type_name).toBe('String')
1184+
expect(get_value(str)).toBeUndefined()
1185+
})
1186+
1187+
test('Atrule.value is undefined', () => {
1188+
const root = parse('@media screen { div { color: red; } }', {
1189+
parse_atrule_preludes: false,
1190+
})
1191+
const atrule = root.first_child!
1192+
expect(atrule.type_name).toBe('Atrule')
1193+
expect(get_value(atrule)).toBeUndefined()
1194+
})
1195+
1196+
test('MediaQuery.value is undefined', () => {
1197+
const root = parse('@media screen and (min-width: 600px) { div { color: red; } }')
1198+
const prelude = root.first_child!.first_child!
1199+
const mediaQuery = prelude.first_child!
1200+
expect(mediaQuery.type_name).toBe('MediaQuery')
1201+
expect(get_value(mediaQuery)).toBeUndefined()
1202+
})
1203+
1204+
test('Raw.value is undefined', () => {
1205+
const root = parse('div { color: red; }', {
1206+
parse_selectors: false,
1207+
parse_values: false,
1208+
})
1209+
const rule = root.first_child!
1210+
const raw = rule.first_child!
1211+
expect(raw.type_name).toBe('Raw')
1212+
expect(get_value(raw)).toBeUndefined()
1213+
})
1214+
})
1215+
10481216
describe('NODE_TYPES namespace', () => {
10491217
test('should work as alternative to individual imports', async () => {
10501218
// Import namespace object

src/css-node.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -415,7 +415,18 @@ export class CSSNode {
415415
return this.get_content()
416416
}
417417

418-
// For other nodes, return as string
418+
// Only these types store a value in the arena value_start/value_length field.
419+
// Every other node type has no value concept and must return undefined.
420+
if (
421+
type !== DECLARATION &&
422+
type !== FUNCTION &&
423+
type !== ATTRIBUTE_SELECTOR &&
424+
type !== SUPPORTS_QUERY &&
425+
type !== PRELUDE_SELECTORLIST
426+
) {
427+
return undefined
428+
}
429+
419430
let start = this.arena.get_value_start(this.index)
420431
let length = this.arena.get_value_length(this.index)
421432
if (length === 0) return null

0 commit comments

Comments
 (0)