fix(overrides): upsell footer not rendering in date filter

[nikkikapadia]

Just fixing a bug i've noticed! I git blamed this pr to being the culprit but basically the date filter upsell footer was not rendering because it was being forced to resolve before the async call could return the component. This change waits for that async call to return before deciding if it has been resolved or not.

Before	After

EDIT: Talked to Ryan in DMs and decided to revert sentry's changes so i've just edited the code for that

[ryan953]

isResolved seems unneeded. It'll always be set to true whenever ResolvedComponent is set.

But i don't know if this fixes the issue. The code before #120028 would call getOverride(overrideName)?.() ?? getDefaultComponent() fresh on each render. This will still call it early/once and cache the result it seems

[sentry]

Bug: The getDefaultComponent function calls React.lazy() on every render if defaultComponentPromise is used, which is a React anti-pattern that can cause infinite loops.
_{Severity: LOW}

Suggested Fix

The React.lazy() call should be moved outside the render path. Create the lazy component once when OverrideOrDefault is initially called and store it, rather than re-creating it inside the getDefaultComponent function on every render. This ensures the same component instance is used across renders.

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#L67-L68

Potential issue: The `OverrideOrDefaultComponent` calls `getDefaultComponent()` on every
render. If the `defaultComponentPromise` prop is provided, `getDefaultComponent` will in
turn call `React.lazy(defaultComponentPromise)` on every render. This creates a new,
distinct lazy component instance each time the component renders, which is a known React
anti-pattern. This can lead to an infinite re-rendering loop when used with
`<Suspense>`. While no current code in the repository uses the `defaultComponentPromise`
parameter, the API allows it, creating a latent bug that could be triggered by future
code changes.

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

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.

^{Reviewed by Cursor Bugbot for commit 9382bc6. Configure here.}

[cursor]

Default lazy remounts every render

Medium Severity

Deferring getOverride to render fixes the upsell timing issue, but getDefaultComponent() is also invoked on every render. When defaultComponentPromise is set, that path builds a new lazy() component and wrapper each time, so React treats it as a new type and remounts the subtree (state reset, Suspense flicker). Only the override lookup needs to be late-bound; the default can stay resolved once outside render.

Additional Locations (1)

• static/app/components/overrideOrDefault.tsx#L47-L60

 

^{Reviewed by Cursor Bugbot for commit 9382bc6. Configure here.}

[narsaynorath]

Can you make a variable to cache this so it's stored at the module level and won't re-evaluate unnecessarily every render?

e.g. loosely depicted

let DefaultComponent: React.ComponentType<any>;

export function OverrideOrDefault({ ... }) {
  function getDefaultComponent(): React.ComponentType<any> | undefined {
    ...
    if (defaultComponentPromise) {
      if (DefaultComponent) {
        return DefaultComponent;
      }

      // Lazy adds a complicated type that is not important
      DefaultComponent = lazy(defaultComponentPromise);

      return function (props: Props) {
        return (
          <Suspense fallback={null}>
            <DefaultComponent {...props} />
          </Suspense>
        );
      };
    }

    return defaultComponent;
  }
}

[github-actions]

📊 Type Coverage Diff

Metric	Before	After	Delta
Coverage	94.03%	94.03%	±0%
Typed	136,642	136,642	±0
Untyped	8,670	8,670	±0

🔍 1 new type safety issue introduced

any-typed symbols (1 new)

File	Line	Detail
static/app/components/overrideOrDefault.tsx	67	OverrideComponent (var)

This is informational only and does not block the PR.

[narsaynorath]

Just left a comment trying to mitigate the bot comments on the PR. I'm not too familiar with the component so if you need a deeper dive, let me know, but if this is mostly a revert then it looks good to me

[narsaynorath]

Can you make a variable to cache this so it's stored at the module level and won't re-evaluate unnecessarily every render?

e.g. loosely depicted

let DefaultComponent: React.ComponentType<any>;

export function OverrideOrDefault({ ... }) {
  function getDefaultComponent(): React.ComponentType<any> | undefined {
    ...
    if (defaultComponentPromise) {
      if (DefaultComponent) {
        return DefaultComponent;
      }

      // Lazy adds a complicated type that is not important
      DefaultComponent = lazy(defaultComponentPromise);

      return function (props: Props) {
        return (
          <Suspense fallback={null}>
            <DefaultComponent {...props} />
          </Suspense>
        );
      };
    }

    return defaultComponent;
  }
}