[mahirhir]

ID doesn't override value, so assigning to id.value stores the string as-is and toString() emits it unescaped. ClassName has had an escaping value setter since d4dda8c5 ("Escape class name values by default", 2018), but ID never got the same one, even though class and id selectors are both identifiers and follow the same escaping rules.

The result is wrong output for any id value that contains characters needing escapes:

const parser = require("postcss-selector-parser");

const cls = parser.className({ value: "x" });
cls.value = "foo.bar";
cls.toString(); // ".foo\\.bar"   (correct)

const id = parser.id({ value: "x" });
id.value = "foo.bar";
id.toString(); // "#foo.bar"   (wrong: re-parses to id "foo" + class "bar")

Leading digits, spaces and # break the same way: #1abc, #a b, #a#b.

This also fixes a second, worse symptom: on an ID that was parsed with an escape, reassigning value previously had no effect at all, because raws.value survived the assignment and toString() kept emitting the old escaped string:

const id = parser().astSync('#fo\\o').first.first;
id.value = 'bar';
id.toString(); // main: '#fo\o'   branch: '#bar'

The new setter stores through raws, so stale raws.value is replaced on assignment (the test asserts id.raws goes back to {}).

This adds the same setter ClassName uses (cssesc(v, { isIdentifier: true }) stored into raws.value), so id values escape on assignment:

id.value = "foo.bar";
id.toString(); // "#foo\\.bar"

The new test mirrors the existing ClassName#set value test. npm test passes (lint, typecheck, full suite).

[MoOx]

Thanks. This is a real gap, and reusing ClassName's setter verbatim is the right call. ID and ClassName are both identifiers; there's no reason they'd escape differently.

I ran the branch against main: merges cleanly, suite green. I also compared id.value = x against className.value = x across a range of values (foo.bar, 1abc, a b, a#b, @md:flex, non-ASCII) — the output is now identical between the two node types, which is exactly what I wanted to confirm.

This fixes more than the description says

On an ID parsed with an escape, reassigning value had no effect at all, because raws.value survived:

const id = parser().astSync('#fo\\o').first.first;
id.value = 'bar';
id.toString(); // main: '#fo\o'   branch: '#bar'

Your test covers it (id.raws going back to {}), it just isn't called out. Could you add a line about it to the PR description? It's the strongest argument for merging and I'd like it in the changelog.

Docs

API.md doesn't document the escaping behaviour for className either, so I won't make it a condition here. But if you feel like adding a short note under parser.id([props]) and parser.className([props]) — values are escaped on assignment, and setPropertyWithoutEscape() is the escape hatch for pre-escaped input — that'd be a welcome bonus. setPropertyWithoutEscape is currently undocumented, which is part of why this trips people up.

On the release

This changes observable behaviour for existing code: anyone assigning an already-escaped value now gets a double escape.

id.value = 'foo\\.bar'; // main: '#foo\.bar'   branch: '#foo\\\.bar'

That's the same footgun ClassName has had since d4dda8c (2018-03-26), so it's consistency rather than a new hazard, and the previous behaviour was emitting invalid selectors (#foo.bar re-parses as an id plus a class). My inclination is to ship it as a minor (7.2.0) with an explicit changelog note, not a major.

This package sits deep in a lot of dependency trees, so I'd like a second opinion: does minor seem right to you, or would you go major?

[mahirhir]

Updated the description with the raws.value point: on an ID parsed with an escape, assigning value previously had no effect at all, since raws.value survived and toString() kept emitting the old string. The new setter stores through raws, and the test asserts id.raws goes back to {}.

On the release: I'd go minor. The old output was invalid selectors, so this is a bug fix, and the double-escape footgun isn't a new hazard, it's the exact one ClassName has carried since 2018. A changelog note naming setPropertyWithoutEscape as the escape hatch covers the people assigning pre-escaped values; major would oversignal it.

Also pushed a small commit adding notes to API.md under parser.id and parser.className: values escape on assignment, setPropertyWithoutEscape() for pre-escaped input.

[mahirhir]

Friendly ping on this when you have time. The description and the API.md notes you asked for are in, and it still merges cleanly.

[MoOx]

I will look into it soon, don't worry ! Thanks for your patience.

[MoOx]

I pushed one commit to your branch. cssesc stopped being an external dependency on main after you opened this: it is vendored at src/util/cssesc now, so import cssesc from "cssesc" no longer resolved and the whole suite failed to load.

The import now points at the vendored copy, nothing else changed, and npm test passes with 959 tests on top of current main.

I did that rather than asking you to rebase, since it was our change that broke yours. Tell me if you would rather handle it yourself and I will revert my commit.