Skip to content

fix: don't treat a non-prefix token before | as a namespace#324

Open
spokodev wants to merge 1 commit into
postcss:mainfrom
spokodev:fix/empty-namespace-prefix
Open

fix: don't treat a non-prefix token before | as a namespace#324
spokodev wants to merge 1 commit into
postcss:mainfrom
spokodev:fix/empty-namespace-prefix

Conversation

@spokodev

Copy link
Copy Markdown

Problem

When a bare | introduces an empty namespace, whatever token precedes it is emitted as the namespace prefix — corrupting the selector on a round-trip:

const parser = require("postcss-selector-parser");
const rt = (s) => { let o; parser((r) => (o = r.toString())).processSync(s); return o; };

rt("/* c */|b"); // "/* c */\/\*\ c\ \*\/|b"   — the comment is re-emitted, escaped, as a prefix
rt(".a,|b");     // ".a,\,|b"                    — the comma becomes an escaped prefix
rt(".a > |b");   // ".a >  |b"                   — extra whitespace
rt(" |b");       // "  |b"

These are all valid selectors that should round-trip unchanged.

Root cause

namespace() takes the prefix from the previous token unconditionally:

namespace() {
  const before = (this.prevToken && this.content(this.prevToken)) || true;
  
}

namespace() is reached either after a real prefix has been consumed (*|, ns|, &|, where the previous token is that prefix) or directly from combinator() when a bare | starts a compound. In the second case the previous token is a comment, comma, combinator or whitespace — not a prefix — but it was still used as one.

Fix

Only use the previous token as the prefix when it can actually be one — a word (type selector), the universal *, or the nesting & (these are exactly the tokens whose parsing hands off to namespace()). Otherwise the namespace is empty (true).

const prev = this.prevToken;
const before =
  prev &&
  (prev[TOKEN.TYPE] === tokens.word ||
    prev[TOKEN.TYPE] === tokens.asterisk ||
    prev[TOKEN.TYPE] === tokens.ampersand)
    ? this.content(prev)
    : true;

*|a, ns|a, &|foo, and |a are unchanged.

Test plan

  • Added tests for an empty namespace after a comment, a comma, and a combinator (they fail on main).
  • A round-trip property check over generated selectors flagged exactly these cases; the fix clears them with no regressions. Full suite green (783), eslint/tsc clean.

`namespace()` used the previous token's content as the namespace prefix
whenever a `|` was seen. For an empty namespace written as a bare `|`
(e.g. `|b`), the previous token is whatever precedes it — a comment,
comma, combinator or whitespace — and that was emitted as the prefix.
So `/* c */|b` round-tripped to `/* c */\/\*\ c\ \*\/|b`, `.a,|b` to
`.a,\,|b`, `.a > |b` to `.a >  |b`, and so on.

Only use the previous token as a prefix when it can be one: a type/word,
the universal `*`, or the nesting `&` (the tokens whose parsing hands off
to `namespace()`). Otherwise the namespace is empty.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant