Skip to content

ref(feedbackModal): Move inner components to module scope for static definitions#120042

Open
sentry[bot] wants to merge 2 commits into
masterfrom
seer/refactor-feedback-modal-static-components
Open

ref(feedbackModal): Move inner components to module scope for static definitions#120042
sentry[bot] wants to merge 2 commits into
masterfrom
seer/refactor-feedback-modal-static-components

Conversation

@sentry

@sentry sentry Bot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

This PR addresses a static-component-definitions coding convention violation in static/app/components/featureFeedback/feedbackModal.tsx.

Problem:
The FeedbackModal component defined ModalHeader, ModalBody, and ModalFooter using useCallback directly within its render function. When these useCallback-wrapped functions were used as JSX component types (e.g., <ModalBody>), React treated them as new component identities on every render where their dependencies changed. This led to unnecessary unmounting and remounting of subtrees, state resets, and blocked React Compiler optimizations, as flagged by the StaticComponents diagnostic.

Solution:

  1. Extracted Components: The ModalHeader, ModalBody, and ModalFooter definitions were moved out of the FeedbackModal's render function and refactored into new, module-scope function components: FeedbackModalHeader, FeedbackModalBody, and FeedbackModalFooter.
  2. Explicit Props: These new module-scope components now explicitly accept all necessary dependencies (such as the modal's Header, Body, Footer render props, isSelfHosted, closeModal, handleSubmit, state, etc.) as props.
  3. Updated Usage: The JSX within FeedbackModal was updated to use these new module-scope components directly.
  4. Custom Children Path: For the props.children (custom children) render path, simple bound function wrappers were created. These wrappers delegate to the new module-scope components and are passed as function values, ensuring they are not used as JSX element types within FeedbackModal's render, thus maintaining stable component identities.

Benefit:
This refactor ensures that component types remain stable across renders, improving React's ability to optimize rendering, preventing unnecessary state loss, and adhering to the static-component-definitions convention and React Compiler best practices.

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-35H

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

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

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 db3e91c. Configure here.

>
{bodyChildren}
</FeedbackModalBody>
);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Custom modal body remounts

Medium Severity

For custom children, boundHeader and boundBody are new function references on every FeedbackModal render. Consumers render them as JSX components (for example Body), so React treats the type as changed whenever modal state updates and remounts those subtrees. That regresses the prior useCallback behavior where header and body stayed stable across state changes, and can drop focus while typing in nested fields.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit db3e91c. Configure here.

Comment on lines +1 to 4
import {Fragment, useMemo, useState} from 'react';
import {css, useTheme} from '@emotion/react';
import type {Event} from '@sentry/core';
import {

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 useCallback hook is used but not imported from 'react', which will cause a ReferenceError when the component renders.
Severity: CRITICAL

Suggested Fix

Add useCallback to the import statement from 'react' at the top of the file. The import should look like: import {Fragment, useMemo, useState, useCallback} from 'react';.

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/featureFeedback/feedbackModal.tsx#L1-L4

Potential issue: The `useCallback` hook is used in the `FeedbackModal` component to
define the `handleSubmit` function. However, `useCallback` is not imported from the
'react' library in the import statement at the top of the file. This will cause a
`ReferenceError: useCallback is not defined` at runtime whenever the `FeedbackModal`
component is mounted, which will break the feedback functionality.

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

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