Skip to content

Distinguish DOM mutation and selector errors - #692

Merged
olavoasantos merged 1 commit into
mainfrom
distinguish-dom-errors
Sep 22, 2026
Merged

olavoasantos merged 1 commit into
mainfrom
distinguish-dom-errors

Conversation

@olavoasantos

@olavoasantos olavoasantos commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Problem

The polyfill reported invalid tree mutations with generic Error objects and treated malformed or unsupported selectors as ordinary non-matches. Callers could not reliably distinguish missing children or references, hierarchy violations, invalid selector syntax, and attributes already owned by another element from unrelated failures or empty results.

Impact

Minor. Consumers of the DOM-like API need stable DOM error names to make reliable decisions. Mutation failures must also be detected before nodes move or Remote DOM hooks, mutation records, and custom-element reactions publish, or the local and remote trees can diverge.

Reproduction

const parent = document.createElement('div');
const foreignParent = document.createElement('section');
const foreignChild = document.createElement('span');
foreignParent.appendChild(foreignChild);

parent.removeChild(foreignChild); // NotFoundError
parent.querySelectorAll(''); // SyntaxError
parent.append(parent); // HierarchyRequestError

Malformed selectors that CSS recovers at end-of-input remain supported:

root.querySelector('div:has(span');
root.querySelector('[data-kind="item]');

Change

  • Add one DOM-exception factory that returns a named DOMException when available and a named Error fallback otherwise.
  • Use the factory for hierarchy, missing-child/reference, custom-element, namespace, document-cloning/importing, event-redispatch, selector, and in-use-attribute errors.
  • Report HierarchyRequestError before NotFoundError when both insertion conditions fail.
  • Convert and prevalidate all variadic mutation arguments before changing either tree, preserving the stack's intentionally stronger multi-argument atomicity. Single-node append() and prepend() use the underlying insertion validation directly instead of walking the ancestor chain twice.
  • Reject empty, malformed, unsupported, repeated/trailing-combinator, nested-:has(), and invalid compound-selector grammar with SyntaxError.
  • Preserve CSS end-of-input recovery for supported :has() / :not() functions and attribute selectors, including quoted string values.
  • Keep the selector implementation deliberately bounded; this does not add unsupported pseudo-classes, selector lists, or a complete CSS parser.

Tests

Adds focused coverage for:

  • invalid children and references;
  • hierarchy failures, including template host relationships;
  • all variadic insertion APIs and conversion failures;
  • unchanged links, owner documents, hooks, mutation publication, and custom-element reactions after rejected operations;
  • duplicate single-node validation;
  • named-error fallback without a global DOMException across public APIs;
  • selector syntax errors through parseSelector(), querySelector(), and querySelectorAll();
  • function, string, and attribute EOF recovery;
  • compound-selector type/universal placement; and
  • supported Unicode, double-hyphen, relative-:has(), and existing SVG/parser controls.

Stack

This is the final polyfill remediation layer. All prerequisite PRs are merged, and this change is reconstructed as one commit directly on current main.

Validation

The reconstructed layer passes locally:

  • lint;
  • type-check and full build;
  • unit tests with coverage: 63 files / 949 tests;
  • bundle size: core-remote 18.77 / 19 kB and polyfill 12.74 / 13 kB; and
  • Playwright: 39/42 on the first run, with the three known iframe interaction flakes passing 3/3 on immediate failed-only rerun.

Local WPT fixtures were not downloaded. Fresh GitHub CI is green: 8/8 checks pass, including changesets, bundle size, lint, type-check, unit tests, Playwright, and classified WPT.

@olavoasantos
olavoasantos changed the base branch from type-comment-hooks to polyfill-correctness-prerequisites September 3, 2026 15:28
@henrytao-me

Copy link
Copy Markdown
Member

Thanks for this. The named errors and conversion staging look right, including the narrow conversion-order fix discussed on #681. I compared 23f091e7 against its holding base 7ec42505 and native Chrome 152. There are a few corrections before this is ready:

1. Preserve CSS end-of-input recovery for supported function selectors

const root = document.createElement('section');
root.innerHTML = '<div data-kind="item"><span></span></div>';

root.querySelector('div:has(span'); // Deliberately no closing parenthesis
  • Native Chrome and the base return the div.
  • This head throws SyntaxError.
  • The same regression affects div:not(.missing and div:has(> span.

readFunctionArgument() now returns null at EOF, but the CSS Syntax function-consumption algorithm returns the function at EOF. Missing closing delimiters at EOF are not automatically invalid selectors.

Please preserve this recovery and correct the test that treats :has(.item as invalid. Keep rejecting genuinely invalid cases such as div:has(), div:has(span)), and trailing/repeated combinators. Add assertions through the parser and both query APIs.

2. Avoid the duplicate ancestor walk for single-node append/prepend

append() prevalidates, then calls appendChild(), which validates the same host-inclusive ancestry again. prepend() follows the same pattern through insertBefore().

I ran the existing 6,000-template fixture directly against both source snapshots, separating construction from serialization. Three rounds with alternating base/head order, Node 24.15.0, without Vitest/coverage:

Phase Base PR
Construction 290 / 294 / 284ms 756 / 549 / 538ms
Serialization 1-2ms 1-4ms

The warm construction runs are roughly twice as expensive. Please avoid redundant validation on the single-node path while preserving the intended multi-argument safeguards.

This demonstrates added overhead in the exact fixture that timed out in CI; it does not independently reproduce the five-second CI timeout. I would address the duplicate traversal before considering a timeout increase.

3. Cover compound-selector grammar, not just individual tokens

With the same fixture:

root.querySelector('[data-kind]div');

Native throws SyntaxError, but both base and head return the div. A type selector cannot follow the attribute selector in that compound. :not(.missing)div has the same problem.

This acceptance is pre-existing, not an introduced matcher regression, but it is a gap in the new syntax-error contract. Please validate type/universal-selector placement and cover these cases through parseSelector(), querySelector(), and querySelectorAll().

Contract and validation notes

  • The stronger atomicity for multi-argument hierarchy failures is a deliberate difference from native DOM. For parent.append(movable, parent), native can detach the supplied nodes before throwing, while this head preserves both original trees. Please distinguish that contract from native conformance; I am not suggesting moving fragment assembly ahead of validation wholesale.
  • The 37 dedicated error-contract cases passed in the recorded CI run; the full run failed on the deep-template timeout. After the corrections and final restack onto integrated main, fresh CI on the published head is still needed. The unpublished local reconstruction is useful supporting evidence, not validation of this live head.

@olavoasantos

Copy link
Copy Markdown
Contributor Author

i'm keeping this branch parked until the prerequisite stacks land, so i haven't updated this head. The structural nested-:has() rejection moved down to #679 where acceptance begins. EOF recovery, compound-selector grammar, the duplicate append/prepend validation, and the remaining named-error coverage will stay in the final #692 reconstruction on current main, followed by fresh full CI.

@henrytao-me

Copy link
Copy Markdown
Member

Thanks, keeping this parked until the prerequisites land and taking the structural nested-:has() guard from #679 makes sense. One addition to the existing EOF correction for the final reconstruction: some of the new malformed-attribute fixtures also need recovery rather than SyntaxError.

const root = document.createElement('section');
const child = document.createElement('div');
child.setAttribute('data-kind', 'item]');
root.appendChild(child);

root.querySelector('[data-kind="item]') === child;
// Native: true, despite the deliberately missing closing quote/bracket.
// Current head: SyntaxError. Base: no match.

I confirmed this in isolated Chrome 153 with CSS1Compat, against unchanged head 23f091e7 / base 7ec42505. The first two quoted-attribute fixtures and their nested :has() variants are accepted natively at EOF. Positive fixtures with the literal values item] and item"] actually select the element; these are not just silently ignored invalid selectors.

Please include string/attribute EOF recovery alongside the function recovery already discussed, and replace those error expectations with exact matching assertions through both query APIs plus parser coverage. Keep the genuinely malformed operator/trailing-junk cases rejected. Base's missing positive match is an inherited gap; head's new exception is separate. Deliberately unsupported CSS remains a different policy boundary.

Fresh validation of this unchanged head:

  • 469 polyfill and 59 focused core tests pass locally, using borrowed Vitest 4.1.2; locked polyfill TypeScript and formatting checks pass. The green suite currently includes these incorrect EOF expectations.
  • Seven public API error paths preserve the named fallback when DOMException is absent.
  • Repeated 6,000-template construction remains about 1.8x base: 291/296/291ms versus 541/534/538ms, with serialization under 3ms. This confirms redundant traversal overhead, not reproduction of the historical five-second CI timeout.

The earlier compound-selector and duplicate single-node validation corrections still apply to the planned reconstruction. No need to duplicate the upstream nested-:has() implementation here. Fresh full CI on the final restacked head is still the gate; these local checks are not integrated-stack or live Shell validation.

@olavoasantos

Copy link
Copy Markdown
Contributor Author

yea, added string/attribute EOF recovery to the final #692 reconstruction checklist alongside function EOF recovery, compound-selector grammar, and duplicate single-node validation. The live branch stays untouched until the prerequisites land; then i'll rebuild it on current main and run fresh CI.

@olavoasantos
olavoasantos changed the base branch from polyfill-correctness-prerequisites to main September 22, 2026 14:48
@olavoasantos
olavoasantos marked this pull request as ready for review September 22, 2026 14:48
@olavoasantos

olavoasantos commented Sep 22, 2026

Copy link
Copy Markdown
Contributor Author

@henrytao-me, the final reconstruction is now on current main at ee48d529.

  • supported :has() / :not() function EOF recovery is preserved;
  • quoted string and attribute-block EOF recovery now matches the literal values in direct and nested selectors;
  • invalid type/universal placement such as [data-kind]div and :not(.missing)div reports SyntaxError; and
  • single-node append() / prepend() no longer add a duplicate wrapper preflight. The 6,000-template construction probe had a 245.4 ms median versus 252.5 ms on current main, while multi-argument validation remains intentionally atomic.

The reconstruction also routes InUseAttributeError through the shared named-error fallback. Fresh CI is green: 8/8 checks pass, including bundle size, Playwright, and classified WPT.

@olavoasantos
olavoasantos merged commit 4f86be9 into main Sep 22, 2026
8 checks passed
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.

4 participants