Fix invalid HTML nesting flagged by validateDOMNesting - #9156
Draft
eventualbuddha wants to merge 1 commit into
Draft
Fix invalid HTML nesting flagged by validateDOMNesting#9156eventualbuddha wants to merge 1 commit into
eventualbuddha wants to merge 1 commit into
Conversation
Ten product sites wrapped block-level content in a `<p>`: `<P>` is widely used as a spacing wrapper, and handing it a `SearchSelect`, `Callout`, `ul`, or another `<P>` produces invalid DOM. `P` now accepts `as="div"` (mirroring the existing `HeadingProps` `as`, as in `<H2 as="h1">`), and the offending sites use it. `StyledP` overrides every UA `p` style, and `as` reuses the same styled component, so the generated class and emitted CSS are unchanged. React dedupes this warning per test file by `ancestorTag|childTag`, so the 13 reported warnings were really 10 distinct sites. To find them all I temporarily patched `Node.prototype.appendChild`/`insertBefore` in every jsdom package's setup to record any flow-content element inserted under a `<p>`; that surfaced sites React never reported, including the `<P>`-in-`<P>` in `internal_connection_problem_screen`. Post-fix the audit records zero. Validation, beyond all 11 jsdom suites staying green with identical test counts and zero `validateDOMNesting` output: captured the full document (including the injected styled-components stylesheet) from the affected tests before and after. `<head>` was byte-identical across all 99 captures, and the only edits anywhere were `<p …>` to `<div …>` with identical attributes plus the matching close tags. Then replayed 61 of those pages in Chromium. The before-DOM can't be screenshotted from a file — the HTML parser auto-closes the invalid `<p>` and hoists the block out, which React's client-side DOM construction never does — so each page loads the valid after-DOM and rebuilds the before-DOM in place via `createElement`/`appendChild`/`replaceChild`, asserting the reconstruction re-serializes to exactly the captured before-DOM (61/61 exact). Result: zero of 5228 elements changed geometry or any of 35 computed CSS properties, and the residual pixel deltas exactly equal a div-to-div re-creation control, i.e. rasterization noise rather than an effect of the tag change. Alternatives considered: hoisting the block content out of the `<P>` as a sibling is semantically cleaner but changes spacing at nearly every site (flex gap in `ballot_style_select`, UA `ul` margins in `districts_screen`, the inner `<P>`'s margin in the two nested cases). A new div-based libs/ui component duplicates `StyledP`'s styles and emits a second class for no behavioral gain. Making `StyledP` render a `div` outright is a one-liner but drops the `paragraph` ARIA role from every paragraph in the repo, including voter-facing screens, and would need an audit of the existing `p` type selectors. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Overview
Ten product sites wrapped block-level content in a
<p>, which React flags asvalidateDOMNestingnoise on every test run.libs/ui'sPnow acceptsas="div"— mirroring the existing<H2 as="h1">idiom — and the offending sites use it. Becauseasreuses the same styled component, the generated class and emitted CSS are unchanged.See the commit for the audit method, validation, and alternatives considered.
Demo Video or Screenshot
No visual change — that's the point. Before/after DOM and Chromium rendering are identical; details in the commit message.
Testing Plan
validateDOMNestingoutput. Note thatvitest.config.shared.mtssetsreporters: []outside CI, which suppresses console output — reproducing this locally needs--reporter=default.appendChild/insertBeforeto record any flow-content element inserted under a<p>) records zero hits across all jsdom packages after the fix, and found sites React's deduped warning never reported.<head>byte-identical across 99 captures; the only edits anywhere are<p …>→<div …>with identical attributes.lint(which type-checks) clean on all 13 packages that useP.Checklist
🤖 Generated with Claude Code