Skip to content

Make withSize use a wrapper element so that it can stop calling findDOMNode#5988

Open
mstange wants to merge 2 commits into
firefox-devtools:mainfrom
mstange:push-ntlqkrsvrsoz
Open

Make withSize use a wrapper element so that it can stop calling findDOMNode#5988
mstange wants to merge 2 commits into
firefox-devtools:mainfrom
mstange:push-ntlqkrsvrsoz

Conversation

@mstange
Copy link
Copy Markdown
Contributor

@mstange mstange commented Apr 29, 2026

Summary

findDOMNode is deprecated in React 18 and removed in React 19 strict mode. The withSize HOC was the last in-tree caller.

This PR makes withSize render a <div style="display: contents"> wrapper around the wrapped component and observe wrapper.firstElementChild via the existing ResizeObserver wrapper, removing the need for findDOMNode. The HOC is also converted to a function component using useState + useRef + useEffect.

The docstring previously claimed the size only updates on window resize; that hasn't been true since the HOC moved to ResizeObserver, so it's been corrected to "whenever the element's size changes".

Test fixes

EmptyThreadIndicator.test.tsx and Timeline.test.tsx were mocking findDOMNode to inject sizes into the wrapped component. With findDOMNode gone those mocks silently became no-ops — EmptyThreadIndicator snapshots showed zero-width indicators and Timeline's per-describe override fell back to the file-level autoMockElementSize. The second commit replaces the mocks with autoMockElementSize, restoring the previous size assertions.

Snapshot deltas

Every snapshot covering a withSize-wrapped component now includes the new <div style="display: contents"> wrapper. The only width/height changes are in EmptyThreadIndicator.test.tsx.snap (still 30/10/20px after the mock fix).

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 29, 2026

Codecov Report

❌ Patch coverage is 87.50000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.76%. Comparing base (b868d0e) to head (d862f92).
⚠️ Report is 58 commits behind head on main.

Files with missing lines Patch % Lines
src/components/shared/WithSize.tsx 87.50% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5988      +/-   ##
==========================================
- Coverage   85.34%   83.76%   -1.59%     
==========================================
  Files         318      329      +11     
  Lines       31922    34415    +2493     
  Branches     8834     9522     +688     
==========================================
+ Hits        27244    28827    +1583     
- Misses       4246     5159     +913     
+ Partials      432      429       -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@fatadel fatadel mentioned this pull request May 7, 2026
4 tasks
EmptyThreadIndicator and Timeline tests mocked findDOMNode to inject
sizes for the WithSize HOC. After WithSize stopped calling findDOMNode,
those mocks became no-ops: EmptyThreadIndicator snapshots regressed to
zero-width indicators, and Timeline's per-describe override silently
fell back to the file-level autoMockElementSize.

Switch EmptyThreadIndicator to autoMockElementSize so its snapshots
again reflect the 100px container, and drop Timeline's dead override.
@fatadel fatadel marked this pull request as ready for review May 18, 2026 13:04
@fatadel fatadel marked this pull request as draft May 18, 2026 13:10
@fatadel fatadel marked this pull request as ready for review May 18, 2026 13:33
@fatadel fatadel requested a review from canova May 18, 2026 13:40
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.

2 participants