Skip to content

fix(tabs): stop tooltips in overflowMenuItems from crashing the page#115993

Merged
TkDodo merged 3 commits into
masterfrom
tkdodo/fix/tab-overflow-menu-tooltips
May 21, 2026
Merged

fix(tabs): stop tooltips in overflowMenuItems from crashing the page#115993
TkDodo merged 3 commits into
masterfrom
tkdodo/fix/tab-overflow-menu-tooltips

Conversation

@TkDodo
Copy link
Copy Markdown
Collaborator

@TkDodo TkDodo commented May 21, 2026

This is a tricky one that I stumbled upon while fixing any types with no-unsafe-member-access. TypeScript didn’t catch this because item.props comes directly from react-aria and is hardcoded to any which is really bad.

Basically, TabList.Item allows passing in tooltips as tooltipProps, so we can do:

<TabList.Item tooltip={{ title: 'hello '}}>

This renders the tooltip on the TabItem fine, however when we have many tabs, the overflowing ones are rendered into a MenuItemList, which has a different type for the tooltip prop:

/**
* Optional tooltip that appears when the use hovers over the item. This is
* not very visible - if possible, add additional text via the `details`
* prop instead.
*/
tooltip?: ExtraContent;
/**
* Additional props to be passed into <Tooltip />.
*/
tooltipOptions?: Omit<TooltipProps, 'children' | 'title' | 'className'>;

Basically tooltip is a ReactNode or a function that produces a ReactNode, and tooltipOptions are separate.

So what happens at runtime is that we pass tooltip={{ title: 'hello '}} to the MenuitemList, which renders it as-is because it's not a function, which makes react crash the page because it cannot render objects.


This fix is a hotfix at best to prevent this from happening. I think a proper fix would consolidate how we pass tooltips around: Do we just want tooltip to be the props object everywhere? The extra options are a weird API. However, the functional syntax to produce a tooltip doesn’t go well with extra options in one object. Best I could come up with is:

tooltip: Omit<TooltipProps, 'title'> & { title: ReactNode | (state) => ReactNode) }

This would be doable in a follow-up but has a larger surface area, would disallow passing string tooltips (which we do a lot, would need to change to {title: 'hello'} and with react-select and react-aria being any it's pretty hard to figure out where we produce options

@TkDodo TkDodo marked this pull request as ready for review May 21, 2026 12:41
@TkDodo TkDodo requested a review from a team as a code owner May 21, 2026 12:41
@github-actions github-actions Bot added the Scope: Frontend Automatically applied to PRs that change frontend components label May 21, 2026
Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

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

Fix All in Cursor

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

Reviewed by Cursor Bugbot for commit 1e14f02. Configure here.

Comment thread static/app/components/core/tabs/tabList.tsx
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 21, 2026

📊 Type Coverage Diff

✅ No new type safety issues introduced. Coverage: 93.60%

Comment thread static/app/components/core/tabs/tabList.tsx
Copy link
Copy Markdown
Member

@natemoo-re natemoo-re left a comment

Choose a reason for hiding this comment

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

Slightly difficult to trace this change through, but your logic seems sound and the changes look straightforward enough!

Comment thread static/app/components/core/tabs/tabList.tsx
@TkDodo TkDodo merged commit bbb41be into master May 21, 2026
71 checks passed
@TkDodo TkDodo deleted the tkdodo/fix/tab-overflow-menu-tooltips branch May 21, 2026 13:57
JonasBa pushed a commit that referenced this pull request May 21, 2026
…115993)

This is a tricky one that I stumbled upon while fixing `any` types with
`no-unsafe-member-access`. TypeScript didn’t catch this because
`item.props` comes directly from `react-aria` and is hardcoded to `any`
which is really bad.

Basically, `TabList.Item` allows passing in tooltips as `tooltipProps`,
so we can do:

```
<TabList.Item tooltip={{ title: 'hello '}}>
```

This renders the tooltip on the TabItem fine, however when we have many
tabs, the overflowing ones are rendered into a `MenuItemList`, which has
a different type for the `tooltip` prop:


https://github.com/getsentry/sentry/blob/f9bda0535267decaccdedefa0d30cd423d1a2d80/static/app/components/core/menuListItem/menuListItem.tsx#L205-L214

Basically `tooltip` is a ReactNode or a function that produces a
ReactNode, and `tooltipOptions` are separate.

So what happens at runtime is that we pass `tooltip={{ title: 'hello
'}}` to the `MenuitemList`, which renders it as-is because it's not a
function, which makes react crash the page because it cannot render
objects.

---

This fix is a hotfix at best to prevent this from happening. I think a
proper fix would consolidate how we pass `tooltips` around: Do we just
want `tooltip` to be the props object everywhere? The extra `options`
are a weird API. However, the functional syntax to produce a tooltip
doesn’t go well with extra options in one object. Best I could come up
with is:

```
tooltip: Omit<TooltipProps, 'title'> & { title: ReactNode | (state) => ReactNode) }
```

This would be doable in a follow-up but has a larger surface area, would
disallow passing string tooltips (which we do a lot, would need to
change to `{title: 'hello'}` _and_ with `react-select` and `react-aria`
being `any` it's pretty hard to figure out where we produce `options`
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.

2 participants