Skip to content

[DevTools] Separate breadcrumbs with »#473

Closed
everettbu wants to merge 4 commits into
mainfrom
sebbie/right-double-angle-quote
Closed

[DevTools] Separate breadcrumbs with »#473
everettbu wants to merge 4 commits into
mainfrom
sebbie/right-double-angle-quote

Conversation

@everettbu

Copy link
Copy Markdown

Mirror of facebook/react#35705
Original author: eps1lon


Summary

Stacked on react/react#35700

This avoids a common confusion with the Suspense breadcrumbs by making it clearer that this is a hierarchy. Applied the same styling to the Owner Stack in the Components tab.

localhost_8080_ (5)

Required a refactor how we measure the combined item size which isolates individual Components better and actually uses ResizeObserver instead of just recalculating when the number of owners changes.

How did you test this change?

CleanShot.2026-02-05.at.18.01.17.mp4

@everettbu everettbu added CLA Signed React Core Team Opened by a member of the React Core Team labels Feb 5, 2026
@greptile-apps

greptile-apps Bot commented Feb 5, 2026

Copy link
Copy Markdown

Greptile Overview

Greptile Summary

This change updates the DevTools Owners Stack and Suspense breadcrumbs to visually indicate hierarchy by inserting a » separator between items. It also refactors how the combined items width is measured: instead of summing per-item widths opportunistically, measurement is moved to a ResizeObserver on the flat list container so the overflow logic can be driven by the observed content size.

Main issue to address before merge: in OwnerStack, width measurement now only runs while the non-overflow (flat list) UI is mounted. Once the UI switches to the overflowing dropdown, elementsTotalWidth stops updating even though the displayed content can continue to grow (drilling in), which can leave the overflow state stale and prevent switching back when space allows.

Confidence Score: 3/5

  • This PR is mergeable after fixing a real overflow/measurement logic issue in OwnersStack.
  • Most changes are presentational, but the OwnersStack width measurement now only updates while the flat list is mounted; once overflowing, the total width can become stale and the UI can get stuck in the dropdown state. Existing RO-guard and props-type issues were already flagged in prior threads and should also be addressed separately.
  • packages/react-devtools-shared/src/devtools/views/Components/OwnersStack.js

Important Files Changed

Filename Overview
packages/react-devtools-shared/src/devtools/views/Components/OwnersStack.css Adds inline-flex container + separator styling for new Owners Stack flat-list rendering.
packages/react-devtools-shared/src/devtools/views/Components/OwnersStack.js Refactors non-overflow view into OwnerStackFlatList with ResizeObserver-based width measurement and breadcrumb separators; introduces a layout/measurement bug when overflowing.
packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseBreadcrumbs.css Adds non-selectable separator styling for breadcrumb hierarchy indicator.
packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseBreadcrumbs.js Adds separators between breadcrumb items and moves width measurement into flat list via ResizeObserver; overall fine aside from already-noted RO guard/prop typing issues.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

6 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

Comment on lines +94 to +102
useLayoutEffect(() => {
const container = containerRef.current;
if (container === null) {
return;
}

const ResizeObserver = container.ownerDocument.defaultView.ResizeObserver;
const observer = new ResizeObserver(entries => {
const entry = entries[0];

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ResizeObserver missing guard

OwnerStackFlatList assumes container.ownerDocument.defaultView.ResizeObserver exists and is constructible. In environments without ResizeObserver (or where defaultView is null), this will throw during useLayoutEffect, breaking the Owners Stack UI. Add a null/feature check (and a safe fallback) before instantiating it.

Also affects packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseBreadcrumbs.js around useLayoutEffect in SuspenseBreadcrumbsFlatList.

Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/react-devtools-shared/src/devtools/views/Components/OwnersStack.js
Line: 94:102

Comment:
**ResizeObserver missing guard**

`OwnerStackFlatList` assumes `container.ownerDocument.defaultView.ResizeObserver` exists and is constructible. In environments without `ResizeObserver` (or where `defaultView` is null), this will throw during `useLayoutEffect`, breaking the Owners Stack UI. Add a null/feature check (and a safe fallback) before instantiating it.

Also affects `packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseBreadcrumbs.js` around `useLayoutEffect` in `SuspenseBreadcrumbsFlatList`.

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines +203 to +207
type SuspenseBreadcrumbsDropdownProps = {
lineage: $ReadOnlyArray<SuspenseNode['id']>,
selectedIndex: number,
selectElement: (id: SuspenseNode['id']) => void,
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unused/incorrect props type

SuspenseBreadcrumbsDropdownProps declares selectedIndex: number but the component neither accepts nor uses it, and callers don't pass it. This will fail Flow type-checking for missing props (and suggests the signature is stale).

Suggested change
type SuspenseBreadcrumbsDropdownProps = {
lineage: $ReadOnlyArray<SuspenseNode['id']>,
selectedIndex: number,
selectElement: (id: SuspenseNode['id']) => void,
};
type SuspenseBreadcrumbsDropdownProps = {
lineage: $ReadOnlyArray<SuspenseNode['id']>,
selectElement: (id: SuspenseNode['id']) => void,
};
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseBreadcrumbs.js
Line: 203:207

Comment:
**Unused/incorrect props type**

`SuspenseBreadcrumbsDropdownProps` declares `selectedIndex: number` but the component neither accepts nor uses it, and callers don't pass it. This will fail Flow type-checking for missing props (and suggests the signature is stale).

```suggestion
type SuspenseBreadcrumbsDropdownProps = {
  lineage: $ReadOnlyArray<SuspenseNode['id']>,
  selectElement: (id: SuspenseNode['id']) => void,
};
```

How can I resolve this? If you propose a fix, please make it concise.

@everettbu
everettbu force-pushed the sebbie/right-double-angle-quote branch from 3b9fc4b to dec687f Compare February 5, 2026 23:16

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Comment on lines 183 to +210
@@ -180,16 +206,14 @@ export default function OwnerStack(): React.Node {
/>
)}
</Fragment>
) : (
<OwnerStackFlatList

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Width measurement never updates
When isOverflowing becomes true, the flat list unmounts and OwnerStackFlatList stops calling setElementsTotalWidth, but elementsTotalWidth can still increase as the dropdown contents grow (drilling deeper). Since useIsOverflowing(elementsBarRef, elementsTotalWidth) depends on this value, the component can get stuck in the overflowing UI and never return to the flat list when there’s actually enough space.

This was previously avoided by the “once overflowing, stop re-measuring” assumption, but that assumption no longer holds once measurement is tied to the flat list being mounted. A fix is to keep observing width regardless of isOverflowing (e.g. observe the bar container and sum/measure its content, or keep the flat list mounted but visually hidden) so elementsTotalWidth reflects the current content size.

Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/react-devtools-shared/src/devtools/views/Components/OwnersStack.js
Line: 183:210

Comment:
**Width measurement never updates**
When `isOverflowing` becomes `true`, the flat list unmounts and `OwnerStackFlatList` stops calling `setElementsTotalWidth`, but `elementsTotalWidth` can still increase as the dropdown contents grow (drilling deeper). Since `useIsOverflowing(elementsBarRef, elementsTotalWidth)` depends on this value, the component can get stuck in the overflowing UI and never return to the flat list when there’s actually enough space.

This was previously avoided by the “once overflowing, stop re-measuring” assumption, but that assumption no longer holds once measurement is tied to the flat list being mounted. A fix is to keep observing width regardless of `isOverflowing` (e.g. observe the bar container and sum/measure its content, or keep the flat list mounted but visually hidden) so `elementsTotalWidth` reflects the current content size.

How can I resolve this? If you propose a fix, please make it concise.

@everettbu

Copy link
Copy Markdown
Author

Upstream PR was closed or merged. Code is synced via branch mirror.

@everettbu everettbu closed this Feb 6, 2026
@everettbu
everettbu deleted the sebbie/right-double-angle-quote branch February 6, 2026 00:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed React Core Team Opened by a member of the React Core Team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants