Skip to content

fix(react): Hoist component resolution in OverrideOrDefault#120028

Open
sentry[bot] wants to merge 1 commit into
masterfrom
seer/fix/hoist-override-or-default-component
Open

fix(react): Hoist component resolution in OverrideOrDefault#120028
sentry[bot] wants to merge 1 commit into
masterfrom
seer/fix/hoist-override-or-default-component

Conversation

@sentry

@sentry sentry Bot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

This PR addresses the static-component-definitions warning in static/app/components/overrideOrDefault.tsx.

Problem:
The OverrideOrDefaultComponent was calling getOverride(overrideName)?.() ?? getDefaultComponent() directly within its render function. When defaultComponentPromise was provided, getDefaultComponent() would create a new React.lazy() instance and an anonymous wrapper component on every render. This caused React to perceive a new component type on each re-render, leading to unnecessary unmounting/remounting of the component subtree, state resets, and preventing React Compiler optimizations.

Solution:
The component resolution logic (both getOverride and getDefaultComponent calls) has been hoisted out of the OverrideOrDefaultComponent's render function and into the factory function scope of OverrideOrDefault. Now, the ResolvedComponent is determined only once when OverrideOrDefault is initially called. This ensures that OverrideOrDefaultComponent always renders a stable component type, resolving the static-component-definitions violation and allowing for proper React state management and compiler optimizations.

Legal Boilerplate

Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.

Fixes CODING-CONVENTIONS-36Y

Comment @sentry <feedback> on this PR to have Autofix iterate on the changes.

@github-actions github-actions Bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Jul 19, 2026
function OverrideOrDefaultComponent(props: Props) {
if (!ResolvedComponent) {
return null;
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Bug: Calling OverrideOrDefault inside a render function creates a new component on every render, causing React to unmount and remount the component tree, losing all state.
Severity: HIGH

Suggested Fix

Move the invocations of OverrideOrDefault from within the component render bodies to the module scope. This ensures the component is created only once, maintaining a stable reference across renders and preserving its state.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: static/app/components/overrideOrDefault.tsx#L75

Potential issue: The `OverrideOrDefault` factory function is intended to be called once
at the module scope. However, it is being called inside the render function of React
components in files like `accountClose.tsx` and `cronsLandingPanel.tsx`. Because the
`defaultComponent` prop is a new inline function on each render, `OverrideOrDefault`
generates a new component type every time the parent re-renders. This causes React to
unmount and remount the entire component subtree, leading to the loss of all internal
state and breaking user interactions, such as with confirmation modals.

Also affects:

  • accountClose.tsx:137
  • cronsLandingPanel.tsx:67

Did we get this right? 👍 / 👎 to inform future reviews.

@github-actions

Copy link
Copy Markdown
Contributor

📊 Type Coverage Diff

Metric Before After Delta
Coverage 94.00% 94.00% ±0%
Typed 136,101 136,101 ±0
Untyped 8,686 8,686 ±0
🔍 1 new type safety issue introduced

any-typed symbols (1 new)

File Line Detail
static/app/components/overrideOrDefault.tsx 69 ResolvedComponent (var)

This is informational only and does not block the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant