fix(coding-conventions): Make activity section icon component static#120047
fix(coding-conventions): Make activity section icon component static#120047sentry[bot] wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
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.
Reviewed by Cursor Bugbot for commit fe6c622. Configure here.
| user: GroupActivity['user']; | ||
| }) => React.ComponentType<SVGIconProps>; | ||
| propsFunction?: (data: GroupActivity['data']) => Record<string, unknown>; | ||
| }) => React.ReactNode; |
There was a problem hiding this comment.
Missed icon mapping consumer update
High Severity
componentFunction was renamed to renderIcon and now returns a React.ReactNode instead of a component type, but progressActivityTooltip.tsx still reads componentFunction and treats the result as a component. That shared consumer needs the same ReactNode-based update as index.tsx, or typecheck fails and a naive rename would render a node as a component.
Reviewed by Cursor Bugbot for commit fe6c622. Configure here.
| [GroupActivityType.NOTE]: { | ||
| Component: IconChat, | ||
| defaultProps: {}, | ||
| componentFunction: ({user, sentry_app}) => { | ||
| renderIcon: ({user, sentry_app}) => { | ||
| if (sentry_app) { | ||
| return function () { | ||
| return <SentryAppAvatar sentryApp={sentry_app} />; | ||
| }; | ||
| return <SentryAppAvatar sentryApp={sentry_app} />; | ||
| } | ||
| return user ? () => <StyledUserAvatar user={user} /> : IconChat; | ||
| return user ? <StyledUserAvatar user={user} /> : <IconChat size="xs" />; | ||
| }, | ||
| }, | ||
| [GroupActivityType.SET_RESOLVED]: {Component: IconCheckmark, defaultProps: {}}, |
There was a problem hiding this comment.
Bug: The progress tooltip was not updated to use the new renderIcon property, causing CREATE_ISSUE activities to display a generic icon instead of a provider-specific one.
Severity: LOW
Suggested Fix
Update progressActivityTooltip.tsx to use the new renderIcon property from the icon mapping to render the icon, similar to the implementation in index.tsx. This will ensure that provider-specific icons are correctly displayed for CREATE_ISSUE activities.
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/views/issueDetails/activitySection/groupActivityIcons.tsx#L59-L69
Potential issue: In `progressActivityTooltip.tsx`, the `ProgressActivityItem` component
attempts to render an icon for a `CREATE_ISSUE` activity. It references the
`componentFunction` property on the icon mapping, which no longer exists after a
refactor and was replaced by `renderIcon`. Because `componentFunction` is `undefined`,
the code falls back to rendering the generic `IconAdd` component. This happens when a
`CREATE_ISSUE` activity appears in the progress tooltip, which can occur if a group has
no other progress-specific activities. The result is that a generic 'add' icon is
displayed instead of the correct provider-specific icon (e.g., GitHub, Jira).
Did we get this right? 👍 / 👎 to inform future reviews.


This PR addresses a
static-component-definitionsviolation instatic/app/views/issueDetails/activitySection/index.tsx.Root Cause:
The
Iconcomponent for activity items was being dynamically created during render viacomponentFunctioningroupActivityIcons.tsx. This meant that on every re-render, React would see a new component identity, leading to unnecessary unmounting and re-mounting of the icon, state resets, and blocking of React Compiler optimizations.Solution:
groupActivityIcons.tsx:componentFunctiontorenderIconin theIconWithDefaultPropsinterface.renderIconfromReact.ComponentType<SVGIconProps>toReact.ReactNode.NOTEandCREATE_ISSUEentries to directly return JSX elements (e.g.,<SentryAppAvatar />,<IconGithub />) instead of returning component types or functions that render components.index.tsx:iconNodereturned byiconMapping.renderIcon(if available) or fall back to rendering the staticIconcomponent with its props.This change ensures that the icon component is no longer created dynamically during render, adhering to the
static-component-definitionsconvention and improving rendering performance and stability.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-350
Comment
@sentry <feedback>on this PR to have Autofix iterate on the changes.