ReviewOS

stacks/very-happy-dom

getBoundingClientRect ignores the cascade that getComputedStyle now resolves

#1600
Closed glennmichael123 opened this 8 days ago ·
8 days ago

Problem

Since 3af7403 (0.1.11) getComputedStyle() resolves the cascade, so a stylesheet rule reaches it. getBoundingClientRect() was not brought along, and still reads inline styles only. The two APIs now disagree about the same element.

import { GlobalRegistrator } from 'very-happy-dom'

GlobalRegistrator.register()
document.head.innerHTML = '<style>.box { width: 120px; height: 40px }</style>'
document.body.innerHTML = '<div class="box" id="a">a</div>'

const el = document.getElementById('a')!
getComputedStyle(el).getPropertyValue('width')  // "120px"  ✅ cascade applied
el.getBoundingClientRect()                      // { width: 0, height: 0 } ❌

Setting style="width: 120px" inline gives a rect of 120. Only the cascade path is missing.

Why it matters

This is the failure mode the testing guidance warns about, arriving through a new door. Someone confirms getComputedStyle reports the class-based width, reasonably concludes styles are wired up, and writes a geometry assertion. It compares 0 to 0 and passes while checking nothing.

Before 0.1.11 the guidance was simple — no cascade, no layout, keep geometry out. Now the line runs between two APIs that look equally live, and the one that silently answers 0 is the one people reach for when asserting size.

It is also what makes size-touching specs unportable. Twelve of twelve Playwright specs surveyed in a downstream app could not move here, and element geometry was one of the four structural blockers.

Proposed scope

Have the box metrics read the same resolved style getComputedStyle() already computes:

  • getBoundingClientRect() — width/height from the cascade, keeping the origin at 0, 0
  • offsetWidth / offsetHeight, clientWidth / clientHeight — the same, with border and padding accounted for as they are today for inline styles

That is a box-metrics fix, not a layout engine.

Explicitly out of scope, and should stay documented as absent: normal flow, positioning, stacking and overlap, scroll offsets, percentage or viewport-relative units that need a containing block, and anything requiring paint. x/y/top/left stay 0.

Acceptance

  • An element sized only by a stylesheet reports that size from getBoundingClientRect(), offsetWidth/offsetHeight and clientWidth/clientHeight.
  • Specificity, !important and inline-over-stylesheet precedence decide the winner, exactly as getComputedStyle() already decides it.
  • An element with no width from any source still reports 0, and the docs still say geometry beyond explicit sizes is not computed.
  • A test asserts the two APIs agree, so they cannot drift apart again.

Note

stacksjs/stacks docs/guide/testing.md has been corrected to describe the post-0.1.11 behaviour and to name this specific mismatch, so the trap is at least written down while this is open.

8 days ago

Fixed in 4060bad. CI green.

Both APIs share one resolution now, which is the point — a second, divergent copy is what caused this.

document.head.innerHTML = '<style>.box { width: 120px; height: 40px }</style>'
document.body.innerHTML = '<div class="box"></div>'

getComputedStyle(el).getPropertyValue('width')  // "120px"
el.getBoundingClientRect()                      // { width: 120, height: 40 }  ✅
el.offsetWidth                                  // 120  ✅

Acceptance, all four:

  • A stylesheet-only size reaches getBoundingClientRect(), offsetWidth/offsetHeight and clientWidth/clientHeight.
  • Specificity, !important and inline-over-sheet decide the winner, because resolveProperty decides it — the same call getComputedStyle() makes. A !important rule beating an inline style attribute is covered.
  • An element nobody sized still reports 0, and x/y/top/left stay at the origin, asserted so a future change cannot quietly start reporting invented coordinates.
  • A test asserts the rect and the computed style agree about the same element, so they cannot drift apart again.

11 tests in test/box-metrics-cascade.test.ts, written failing first: 6 of 11 failed against the old implementation, and the 5 that passed were the cases that already worked (inline wins, unsized reports zero, the width attribute, position not computed, and a document with no stylesheets).

A percentage now resolves against a parent sized by a stylesheet too, which fell out of routing both through the same path.

Cost

Measured with warmup, median of five runs:

beforeafter
no stylesheets0.45 µs0.45 µs
small stylesheet—3.12 µs
200-rule stylesheet—~30 µs

A document with no stylesheets skips matching entirely, so the common unit-test path is untouched. The 200-rule figure is worth being plain about: it is what getComputedStyle() already cost for a single property read (31.6 µs measured), because the expense is collectCascade matching every rule against the element. This change did not introduce that, it inherits it — and a rect now builds the cascade once for both dimensions rather than once each, so it lands slightly under a single getComputedStyle() call.

If that matching cost is worth attacking, it belongs in its own issue and would speed up getComputedStyle() as much as the metrics.

Still absent

Flow, positioning, stacking and overlap, scroll offsets, and percentage or viewport units needing a containing block. stacksjs/stacks docs/guide/testing.md names layout and pixels as the remaining gaps, and no longer claims the cascade is missing.

Sign in to comment on this issue.