Distinguish DOM mutation and selector errors - #692
olavoasantos wants to merge 1 commit into
Conversation
|
Thanks for this. The named errors and conversion staging look right, including the narrow conversion-order fix discussed on #681. I compared 1. Preserve CSS end-of-input recovery for supported function selectorsconst root = document.createElement('section');
root.innerHTML = '<div data-kind="item"><span></span></div>';
root.querySelector('div:has(span'); // Deliberately no closing parenthesis
Please preserve this recovery and correct the test that treats 2. Avoid the duplicate ancestor walk for single-node append/prepend
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:
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 tokensWith the same fixture: root.querySelector('[data-kind]div');Native throws 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 Contract and validation notes
|
|
i'm keeping this branch parked until the prerequisite stacks land, so i haven't updated this head. The structural nested- |
|
Thanks, keeping this parked until the prerequisites land and taking the structural nested- 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 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:
The earlier compound-selector and duplicate single-node validation corrections still apply to the planned reconstruction. No need to duplicate the upstream nested- |
|
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 |
Problem
The polyfill reported invalid tree mutations with generic
Errorobjects and treated some malformed or unsupported selectors as non-matches. Callers could not reliably distinguish a missing child or reference node, a hierarchy violation, and invalid selector syntax from other failures or an ordinary empty result.Impact
Minor. Code using the exposed DOM-like API cannot make DOM-style error decisions when every mutation failure is a generic error or invalid selector input silently produces no match. Mutation failures also need to report the error before they move nodes or emit Remote DOM hooks, so the local tree and remote receiver remain synchronized.
Reproduction
Before this change, the first operation threw a generic
Error, and the empty selector returned an empty result. They now throw errors namedNotFoundErrorandSyntaxError, respectively. The regression tests likewise verifyHierarchyRequestErrorfor an attempted ancestor insertion; in every mutation-error case, parent/child links and insert/remove hook calls remain unchanged.Change
DOMExceptionwhen available and a namedErrorfallback when it is not.NotFoundErrorfor invalid child or reference nodes andHierarchyRequestErrorfor invalid insertion hierarchy, with hierarchy validation taking precedence where both conditions apply.SyntaxErrorrather than silently returning no matches; supported Unicode and double-hyphen identifiers and relative:has()selectors remain covered.Tests
Adds focused error-contract coverage for invalid children and references, hierarchy violations (including template host relationships), all variadic insertion APIs, and conversion failures after a cross-document node has been staged. The tests assert error names, unchanged links, no hook calls, and no custom-element reactions. Selector coverage exercises malformed/unsupported syntax through the parser,
querySelector(), andquerySelectorAll(), and verifies the no-DOMExceptionfallback.Stack
This draft is the final fan-in for the subsystem stacks. The live PR remains parked on
polyfill-correctness-prerequisites; its historical head also contains a bundle-size gate update that was separately landed by PR #706. An unpublished maintainer-local reconstruction isolates this error-contract layer on the fully integrated post-#706 base, but it is supporting evidence rather than a reviewable branch. After the prerequisite stacks merge, rebase this PR ontomainand rerun the complete validation suite.Validation
The live PR’s historical CI is not a current green validation signal: its lint, type-check, bundle-size, Playwright, and classified Web Platform Test checks passed, but its unit-test check failed.
An unpublished maintainer-local reconstruction passed:
These local results are supporting evidence only. Fresh GitHub CI is still required after the live PR is rebased.