Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ryan953 merged 2 commits into from
Jul 20, 2026
+5 −6
Merged

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

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
91e7ca7
fix(react): Hoist component resolution in OverrideOrDefault
ryan953 Jul 19, 2026
91c0918
Apply suggestion from @ryan953
ryan953 Jul 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 9 additions & 6 deletions static/app/components/overrideOrDefault.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,16 +62,19 @@ export function OverrideOrDefault<H extends OverrideName>({
return defaultComponent;
}

function OverrideOrDefaultComponent(props: Props) {
// Defining the props here is unnecessary and slow for typescript
const OverrideComponent: React.ComponentType<any> =
getOverride(overrideName)?.() ?? getDefaultComponent();
// Resolve the component once at factory scope so that the component type
// identity is stable across renders. Creating components during render gives
// React a new type on every render, causing unnecessary unmount/remount cycles
// and breaking React Compiler's StaticComponents optimization.
Comment thread
ryan953 marked this conversation as resolved.
Outdated
const ResolvedComponent: React.ComponentType<any> | undefined =
getOverride(overrideName)?.() ?? getDefaultComponent();
Comment on lines +65 to +66

@ryan953 ryan953 Jul 20, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

looks like this was pulled out of the function and the value is closed-over now. Keeps OverrideOrDefaultComponent returning stable values across renders which is nice.


if (!OverrideComponent) {
function OverrideOrDefaultComponent(props: Props) {
if (!ResolvedComponent) {
return null;
}
Comment thread
sentry[bot] marked this conversation as resolved.

return <OverrideComponent {...props} />;
return <ResolvedComponent {...props} />;
}

Comment on lines +65 to 75

@sentry sentry Bot Jul 20, 2026

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: The OverrideOrDefault factory is called inside React component render functions, creating a new component type on each render, which causes state loss and remounts.
Severity: MEDIUM

Suggested Fix

Hoist the calls to OverrideOrDefault outside of the React component's render body. The resulting component should be defined once at the module scope, ensuring that the same component reference is used across all renders. For example, move const HookedCustomConfirmAccountClose = OverrideOrDefault({...}) outside the AccountClose function component.

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#L65-L75

Potential issue: The `OverrideOrDefault` factory function is called directly within the
render bodies of some React components, such as in `accountClose.tsx` and
`cronsLandingPanel.tsx`. This creates a new, distinct component function object on every
render. React's reconciliation process treats this new function as a completely
different component type, causing it to unmount the previous component tree and mount a
new one. This leads to the loss of all component state and a performance degradation due
to the unnecessary remounting on every render cycle of the parent component.

Also affects:

  • static/app/views/settings/account/accountClose.tsx:137~141
  • static/app/views/performance/landing/autofixSection.tsx:108
  • static/app/views/crons/cronsLandingPanel.tsx:67

OverrideOrDefaultComponent.displayName = `OverrideOrDefaultComponent(${overrideName})`;
Expand Down
Loading