Skip to content

JSDOM recursion fix - #176

Open
charlesverge wants to merge 3 commits into
dperini:masterfrom
charlesverge:matches-fix
Open

JSDOM recursion fix#176
charlesverge wants to merge 3 commits into
dperini:masterfrom
charlesverge:matches-fix

Conversation

@charlesverge

Copy link
Copy Markdown

Summary

  • capture the browser platform's Element#matches implementation once, before nwsapi can replace it
  • prevent CommonJS and headless environments from dynamically delegating back through a module-backed node.matches
  • add Jest regression coverage for all affected display-state pseudo-classes
  • update GitHub Actions to run the Jest suite with the latest Node.js release and current action versions

Problem

The native display-state detection introduced in nwsapi 2.2.26 assumes that node.matches is a native browser implementation when _matches is unavailable. In environments such as jsdom, Element#matches delegates back to the same nwsapi instance:

element.matches(':modal')
-> nwsapi.match(':modal')
-> isModal()
-> matchesNative(':modal')
-> element.matches(':modal')
-> ...

The resulting stack overflow is caught by matchesNative(), so the caller eventually receives false, but only after substantial recursive work and CPU usage.

Solution

Native matcher provenance is now established when the factory is created:

  • the browser bootstrap captures the platform matcher from Element.prototype
  • the captured matcher is passed into the nwsapi factory and retained in its closure
  • matchesNative() calls only that trusted reference
  • when no platform matcher is supplied, matchesNative() is initialized as a direct false-returning function

The helper no longer probes node.matches or its vendor-prefixed aliases during selector evaluation. A module-backed matcher therefore cannot be mistaken for a native implementation and called recursively.

CommonJS and headless embedders with a genuine platform matcher can supply it explicitly. Otherwise, nwsapi uses its existing observable DOM-state fallbacks.

Why This Approach

A synchronous re-entrancy flag limits the recursion but still calls the module-backed matcher once for every outer state check. That causes an unnecessary round trip through the host and another nwsapi selector evaluation. It also adds repeated matcher discovery and mutable guard state to a hot path.

Capturing matcher provenance once removes the recursive route entirely:

  • zero calls to a module-backed Element#matches
  • no repeated matcher-chain lookup
  • no try/finally re-entrancy bookkeeping
  • no factory-wide flag that can suppress unrelated nested native matching
  • one direct call when a trusted platform matcher is available

Testing

The Jest regression suite verifies that:

  • module-backed Element#matches is never called for :open, :closed, :modal, :fullscreen, :picture-in-picture, or :popover-open
  • an explicitly supplied platform matcher is called exactly once
  • an element-level matcher is not consulted when a platform matcher was captured
  • exceptions from the platform matcher retain the existing false fallback

Validation performed:

npm test
Test Suites: 1 passed, 1 total
Tests:       3 passed, 3 total

The original jsdom 26.1.0 reproduction also completes without recursion.

CI

The Node workflow now:

  • runs on pushes and pull requests targeting the repository's master branch
  • uses actions/checkout@v7 and actions/setup-node@v7
  • resolves the latest available Node.js release
  • installs dependencies and executes npm test

jdalton added a commit to jdalton/nwsapi that referenced this pull request Sep 4, 2026
Four defects, all present in plain 2.2.27 and all reproduced here before
being fixed. Three arrived with the 2.2.25 compiler rework (upstream
7a22775); the fourth arrived with the 2.2.26 state pseudo-classes.

Forgiving :is()/:where(). The fallback tested /(:(?:is|where)\\x28)/, where
the doubled escape matches a literal backslash rather than an opening
parenthesis, so ':not(:is(svg|div))' raised "unknown pseudo-class selector"
instead of matching nothing.

EOF-terminated arguments. The linguistic, logicalsel and treestruct groups
lost their '(?:\x29|$)' terminator, so ':not([class]' and
'meta[charset="utf-8"' were parse errors rather than being closed by EOF the
way the CSS Syntax parser closes any open construct
(/css/selectors/missing-right-token.html). Restoring the terminator alone
reintroduces the bug it papered over: with '[^()]*|.*' the greedy
alternative swallows the closing parenthesis of a nested argument, so
':not(:is(div))' compiles ':is(div))'. A regular expression cannot track
nesting, so the argument of :is, :where, :matches, :not and :has is now
delimited by matchLogical(), which scans for the balanced closing
parenthesis, honoring quotes and escapes, and falls back to EOF.

:has() anchoring. The relative argument was compiled by prefixing ':scope ',
then collect() was called directly. ':scope' compiles to a comparison
against Snapshot.from, which only select() keeps up to date, so inside
:has() it still pointed at the outer query context: the ancestor walk was
not bounded by the element under test and '.x:has(.d .e)' matched an .x
whose only .d was itself, while ':has(child)' on an element outside the
document matched nothing. ':scope' cannot stand in for the anchor in any
case, since an explicit ':scope' inside the argument keeps referring to the
scoping root of the outer query, which
/css/selectors/has-argument-with-explicit-scope.html asserts. The implied
anchor is now the private ':-nwsapi-anchor' pseudo-class, compiled against
Snapshot.anchor, which has() sets around the argument and restores in a
finally block. The sibling arguments take the same path with the parent as
the collection context, which retires the open-coded '+' branch and the '~'
branch that ignored its argument entirely.

Re-entry under jsdom. matchesNative() reached for node.matches at match
time. jsdom wires Element.prototype.matches back into nwsapi, so resolving
':modal' called jsdom, which called nwsapi, which resolved ':modal' again
until the stack was exhausted, and the RangeError was then swallowed and
reported as a plain false. Measured against 2.2.27, one NW.match(':modal',
element) makes 5,428,790 re-entrant calls; it is 0 after this change.
Provenance is established once, when the factory runs, from
global.Element.prototype, and node.matches is never consulted; a
re-entrancy guard remains for hosts that pass their window as the global,
where the captured matcher can itself be a delegating wrapper. That is
upstream dperini#172, dperini#171 and dperini#177, combining the approaches of
their PRs dperini#176 and dperini#170.

Attribute selector after a pseudo-class. The combinator alternative inside
the validator's pseudo-class pattern was '[>+~][^>+~]', which consumes the
character after the combinator; when that is the '[' of an attribute
selector the attribute can no longer be parsed and the whole selector is
rejected. The top-level combinator pattern already uses a lookahead, so the
two now agree. "[class*='a' i]:not(:empty) + [class*='b']" is upstream
dperini#175, which reaches jsdom users through
@testing-library/user-event. The error it raised named a selector with
commas where its quotes should be, because emit() was passed the array of
fragments the validator did match rather than the selector; it now names the
selector.

/css/selectors/has-relative-argument.html now passes in full. 16 entries
leave the WPT baseline, 336 remain, none added. A 'node' Playwright project
covers the regressions that only appear when nwsapi is the engine behind a
host's matches().
jdalton added a commit to jdalton/nwsapi that referenced this pull request Sep 4, 2026
matchesNative() reaches for node.matches at match time, on the assumption that it is the host's own implementation. jsdom wires Element.prototype.matches back into nwsapi, so resolving ':modal' calls jsdom, which calls nwsapi, which resolves ':modal' again, until the stack is exhausted — and the RangeError is then swallowed and reported as a plain false.
 Measured against 2.2.27, one NW.match(':modal', element) makes 5,428,790 re-entrant calls to Element.prototype.matches. It is 0 with this change.
 Provenance is established once, when the factory runs, from global.Element.prototype, and node.matches is never consulted. A host that passes only a document, as jsdom does, has no native matcher and therefore no native state to read, which is the correct outcome rather than a workaround. A re-entrancy guard stays in place for hosts that pass their window as the global, where the captured matcher can itself be a delegating wrapper.
 This combines the two approaches already proposed in dperini#176 and dperini#170.

References:

- Spec: https://drafts.csswg.org/selectors-4/#modal-state — ':modal' and the state pseudo-classes
- Spec: https://html.spec.whatwg.org/#attr-dialog-open — the dialog open attribute, and the 'is modal' flag that has no reflection
- Chromium: https://github.com/chromium/chromium/blob/155.0.8041.1/third_party/blink/renderer/core/css/selector_checker.cc#L3199 — ':modal' asks the element, not another selector engine
- MDN: https://developer.mozilla.org/en-US/docs/Web/CSS/:modal

Closes dperini#172
Closes dperini#171
Closes dperini#177
jdalton added a commit to jdalton/nwsapi that referenced this pull request Sep 5, 2026
matchesNative() reaches for node.matches at match time, on the assumption that it is the host's own implementation. jsdom wires Element.prototype.matches back into nwsapi, so resolving ':modal' calls jsdom, which calls nwsapi, which resolves ':modal' again, until the stack is exhausted — and the RangeError is then swallowed and reported as a plain false.
 Measured against 2.2.27, one NW.match(':modal', element) makes 5,428,790 re-entrant calls to Element.prototype.matches. With this change the first query makes one call and every query after it makes none, so fifty of them run in under a millisecond.
 The matcher is taken from the node's own realm, through ownerDocument.defaultView, and memoized per document. A matcher belonging to another realm answers a foreign node wrong, or throws a brand check that the existing catch turns into a silent false, and the realm this module loaded in is not reliably the node's: the documented Node shape is nwsapi({ document, DOMException }), which carries no Element at all.
 A re-entrancy guard makes that safe. When the host matcher routes back into this engine, the nested call returns the outer answer instead of recursing, and the trip is recorded, so a delegating host is asked at most once per document and never again. Under jsdom that settles to zero calls after the first; in a browser the host keeps answering, which is where ':modal' and ':popover-open' have to come from.
 This combines the two approaches already proposed in dperini#176 and dperini#170.

References:

- Spec: https://drafts.csswg.org/selectors-4/#modal-state — ':modal' and the state pseudo-classes
- Spec: https://html.spec.whatwg.org/#attr-dialog-open — the dialog open attribute, and the 'is modal' flag that has no reflection
- Chromium: https://github.com/chromium/chromium/blob/155.0.8041.1/third_party/blink/renderer/core/css/selector_checker.cc#L3199 — ':modal' asks the element, not another selector engine
- MDN: https://developer.mozilla.org/en-US/docs/Web/CSS/:modal

Closes dperini#172
Closes dperini#171
Closes dperini#177
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