Skip to content

[DevTools] fix: don't show empty suspended by section#689

Closed
everettbu wants to merge 1 commit into
mainfrom
devtools/dont-show-empty-suspended-by-section
Closed

[DevTools] fix: don't show empty suspended by section#689
everettbu wants to merge 1 commit into
mainfrom
devtools/dont-show-empty-suspended-by-section

Conversation

@everettbu

Copy link
Copy Markdown

Mirror of facebook/react#36011
Original author: hoxyq


The potential paddings and margins for the empty block are already handled via CSS selectors.

@greptile-apps

greptile-apps Bot commented Mar 11, 2026

Copy link
Copy Markdown

Greptile Summary

This PR adds a three-line guard to InspectedElementSuspendedBy that returns null when the groups array is empty, preventing an otherwise-empty "suspended by" container from being rendered. The intent is correct, but the condition is incomplete: groups is derived solely from suspendedBy, so it will always be empty when suspendedBy is null/empty — including the cases where inspectedElement.unknownSuspenders is set to a non-NONE value (PRODUCTION, OLD_VERSION, or THROWN_PROMISE). In those situations the unknownSuspenders informational message (computed in the switch block immediately above the new guard) would be silently dropped, regressing the user-visible guidance that tells developers why suspension details are unavailable.

  • Bug: return null when groups.length === 0 must also check unknownSuspenders === null; otherwise informative production/old-version/thrown-promise messages are suppressed.
  • The fix for the actual empty-section problem is otherwise sound — when both groups and unknownSuspenders are empty/null, returning null correctly avoids rendering the shell.

Confidence Score: 2/5

  • This PR introduces a regression that silently suppresses informative warning messages for suspended components in production/old-version/thrown-promise scenarios.
  • The early-return condition only checks groups.length === 0 but not whether unknownSuspenders is null. This means valid, user-facing diagnostic messages (production build warning, old React version warning, thrown-promise warning) will be hidden whenever suspendedBy is empty — which is precisely the common case those messages are designed for.
  • packages/react-devtools-shared/src/devtools/views/Components/InspectedElementSuspendedBy.js — the new guard at line 587 needs to be tightened.

Important Files Changed

Filename Overview
packages/react-devtools-shared/src/devtools/views/Components/InspectedElementSuspendedBy.js Adds an early return null when groups is empty to avoid rendering an empty "suspended by" shell, but the guard does not account for the unknownSuspenders variable set just above — when suspendedBy is empty but inspectedElement.unknownSuspenders is a non-NONE value, the informative warning message is now silently suppressed.

Fix All in Claude Code Fix All in Codex

Last reviewed commit: 4e4d138

Comment on lines +587 to +589
if (groups.length === 0) {
return null;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Early return suppresses unknownSuspenders messages

groups is only non-empty when suspendedBy contains items. However, the early-exit guard at lines 466–468 lets execution continue when suspendedBy is null/empty and inspectedElement.unknownSuspenders is not UNKNOWN_SUSPENDERS_NONE (i.e., it is UNKNOWN_SUSPENDERS_REASON_PRODUCTION, UNKNOWN_SUSPENDERS_REASON_OLD_VERSION, or UNKNOWN_SUSPENDERS_REASON_THROWN_PROMISE). In those cases groups will always be empty, so this new return null will silently discard the informative unknownSuspenders message that was computed in the switch statement just above.

Concretely: a production-mode component that is suspended with no tracked suspenders would previously show:

"Something suspended but we don't know the exact reason in production builds of React…"

With this change it now renders nothing at all.

The guard should also require that unknownSuspenders is null before short-circuiting:

Suggested change
if (groups.length === 0) {
return null;
}
if (groups.length === 0 && unknownSuspenders === null) {
return null;
}
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/react-devtools-shared/src/devtools/views/Components/InspectedElementSuspendedBy.js
Line: 587-589

Comment:
**Early return suppresses `unknownSuspenders` messages**

`groups` is only non-empty when `suspendedBy` contains items. However, the early-exit guard at lines 466–468 lets execution continue when `suspendedBy` is null/empty _and_ `inspectedElement.unknownSuspenders` is **not** `UNKNOWN_SUSPENDERS_NONE` (i.e., it is `UNKNOWN_SUSPENDERS_REASON_PRODUCTION`, `UNKNOWN_SUSPENDERS_REASON_OLD_VERSION`, or `UNKNOWN_SUSPENDERS_REASON_THROWN_PROMISE`). In those cases `groups` will always be empty, so this new `return null` will silently discard the informative `unknownSuspenders` message that was computed in the `switch` statement just above.

Concretely: a production-mode component that is suspended with no tracked suspenders would previously show:

> "Something suspended but we don't know the exact reason in production builds of React…"

With this change it now renders nothing at all.

The guard should also require that `unknownSuspenders` is null before short-circuiting:

```suggestion
  if (groups.length === 0 && unknownSuspenders === null) {
    return null;
  }
```

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

Fix in Claude Code Fix in Codex

@everettbu

Copy link
Copy Markdown
Author

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

@everettbu everettbu closed this Mar 12, 2026
@everettbu
everettbu deleted the devtools/dont-show-empty-suspended-by-section branch March 12, 2026 21:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants