Skip to content

Anchor selectors with :scope so scoping is real - #4

Open
myabc wants to merge 1 commit into
refactor/scoped-queryfrom
fix/scope-selectors
Open

Anchor selectors with :scope so scoping is real#4
myabc wants to merge 1 commit into
refactor/scoped-queryfrom
fix/scope-selectors

Conversation

@myabc

@myabc myabc commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Ticket

N/A — internal architecture review of this library.

What are you trying to accomplish?

Make the README's scoping promise true. root.querySelector(sel) matches document-wide and only filters results to descendants, so .menu li binds via a .menu ancestor outside the controller element.

What approach did you choose and why?

Prefix :scope onto every top-level comma-separated alternative (anchorToScope, small depth/quote/escape/comment-aware scanner). Rejected: single prefix (comma bypasses the anchor: :scope .a, .b) and :scope :is(sel) wrap (combinator left-hand sides inside :is() still match outside ancestors).

Only a leading :scope proves an alternative is rooted and passes through; a non-leading :scope (:not(:scope) .item, .outer :scope .item, [data-x=":scope"] .item) does not anchor and is prefixed too — at worst unmatchable, failing closed instead of leaking. CSS comments are stripped while scanning: their contents would otherwise corrupt the quote tracking and hide commas (.none/*"*/, .outer .item). Invalid input still fails closed via the existing try/catch. Bonus: relative selectors (> li) now work.

Also documents the selector trust model (README security note + CONTEXT.md): override attributes = same trust as any Stimulus data-* attribute; evaluation is read-only, fails closed, returns only descendants. CSS.escape is user guidance, not a library concern.

Testing: the suite runs in real Chromium (#6), so the scopedQuery tests exercise the actual leak — unanchored combinators genuinely match through outside ancestors there. Guarantee pinned twice: unit tests on the pure rewrite + browser-executed integration tests. Bypasses (comma, comment, non-leading :scope) each confirmed leaking before the fix and closed after.

Stacked on #2; chain #6#1#2 ← this. Minor changeset (behavior change).

Verification: bun run test 60 pass in Chromium, bun run typecheck clean, bun run build clean.

Merge checklist

  • Added/updated tests
  • Lookbook docs n/a — npm library; README + CONTEXT.md updated
  • Tested major browsers — suite runs in real Chromium in CI; :scope is baseline in all modern browsers

Copilot AI lite review requested due to automatic review settings August 10, 2026 18:41
@myabc myabc added the enhancement New feature or request label Aug 10, 2026
@myabc myabc added the security label Aug 10, 2026
@myabc
myabc requested a review from oliverguenther August 10, 2026 18:43

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR tightens Stimulus element lookup scoping by ensuring selectors are anchored to the controller element via :scope, preventing combinators (and comma-separated selector lists) from matching through ancestors outside the root element.

Changes:

  • Add anchorToScope() to prefix :scope onto each top-level comma-separated selector alternative and use it inside scopedQuery.
  • Expand unit/integration coverage around anchoring, commas in nested/quoted contexts, and relative selectors.
  • Document the anchoring behavior and selector trust model in README/CONTEXT, and add a minor changeset.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/query.ts Introduces selector list splitting + :scope anchoring and applies it to scopedQuery.
test/query.test.ts Adds tests for the selector rewrite and for anchored query behavior (including commas/relative selectors).
README.md Documents the anchoring guarantee and adds a security/trust note for selector overrides.
CONTEXT.md Records the anchoring decision and trust model rationale for future maintainers.
.changeset/proud-scopes-anchor.md Publishes the behavior change as a minor release note.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/query.ts
The README promised lookups "scoped to the controller's own element",
but querySelector matches selectors document-wide and only filters
results to descendants: ".menu li" bound via a .menu ancestor
outside the controller element.

Prefix :scope onto every top-level comma-separated alternative, split
by a depth/quote/escape/comment-aware scanner — a single prefix would
be bypassed by the comma (":scope .a, .b" leaves .b unanchored), and
a :scope :is() wrap was rejected because combinator left-hand sides
inside :is() still match ancestors outside the root. CSS comments are
stripped while scanning: their contents would otherwise corrupt the
quote tracking (".none/*\"*/, .x .item" hid the comma). Only a
leading :scope proves an alternative is rooted and passes through;
a non-leading :scope (":not(:scope) .item", ".outer :scope .item")
does not anchor and is prefixed too — at worst unmatchable, failing
closed instead of leaking. Relative selectors like "> li" now work.

The guarantee is pinned twice: unit tests on the pure anchorToScope
rewrite, and scopedQuery tests that exercise the real leak — the
suite runs in actual Chromium (vitest browser mode), where unanchored
combinators genuinely match through outside ancestors.

Also record the selector trust model (overrides are as trusted as any
Stimulus data-* attribute; CSS.escape is user guidance, not a library
concern) in README and CONTEXT.md.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request security

Development

Successfully merging this pull request may close these issues.

2 participants