Repository navigation
fix: don't count child combinators as selector specificity - #303
Open
Christopher Pruijsen (cpruijsen) wants to merge 2 commits into
Open
Christopher Pruijsen (cpruijsen) wants to merge 2 commits into
Christopher Pruijsen (cpruijsen) wants to merge 2 commits into
Conversation
A selector's parentScopes array mixes '>' child-combinator tokens with parent scope names. The specificity comparator skipped '>' while comparing individual parent scopes, but still counted them in the final array-length comparison, so selectors with more combinators won ties they should not have. Count only real parent scope names for that comparison, and fall back to the rule's source index when two selectors are equally specific, so that the later rule in the theme wins - matching how equal-specificity ties are already resolved for merged selectors. Fixes microsoft#292
assertThemeEqual compared each Theme's root to itself, so expected index values for the combinator-specificity change were never checked. Compare against the expected tree and store the indices produced by merging.
Copilot started reviewing on behalf of
Alexandru Dima (alexdima)
September 14, 2026 11:08
View session
There was a problem hiding this comment.
🟢 Approval recommended
The reviewed changes address the specificity issue and include regression coverage.
Pull request overview
Fixes theme selector specificity so child combinators (>) do not increase specificity, while declaration order resolves ties.
Changes:
- Excludes
>tokens from specificity counts. - Threads declaration indexes through trie rules.
- Adds regression coverage.
File summaries
| File | Description |
|---|---|
src/theme.ts |
Updates specificity comparison and declaration-order tie-breaking. |
src/tests/themes.test.ts |
Adds combinator specificity regression coverage. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
src/theme.tsstores parent scope names and>child-combinator tokens in the same array, and thespecificity comparator skips
>while walking individual scopes but then compares rawparentScopes.length. A selector therefore gains specificity by containing more combinators, whichthe comparator's own scope-walking logic says should not happen.
a > boutranksa bon tokencount rather than on anything meaningful.
The comparator now counts parent scopes through
parentScopeCount, which excludes>, so the lengthcomparison agrees with the walk above it.
Removing that signal needs something correct in its place, which is most of the diff. Raw length was
also acting as an accidental tie-break, so two rules of genuinely equal specificity could swap
depending on how many combinators they were written with.
ThemeTrieElementRulecarries the rule'sdeclaration
index, and the comparator falls back to it when the corrected counts are equal. Laterdeclarations win, which is what a theme author expects and what the rules already did when the counts
happened to differ.
The file-level churn is that
indexhas to reach the comparator: it is threaded through theconstructors,
insert,_doInsertHereandacceptOverwrite. Only the two comparisons are behaviour.Tests cover a
>selector against an equally specific one without it, and the declaration-ordertie-break.
Fixes #292