codeant-ai-for-open-source[bot] commented on code in PR #39535:
URL: https://github.com/apache/superset/pull/39535#discussion_r3430393714
##########
superset-frontend/packages/superset-ui-core/src/components/ModalTrigger/index.tsx:
##########
@@ -84,8 +90,14 @@ export const ModalTrigger = forwardRef(
setShowModal(true);
};
- if (ref) {
- ref.current = { close, open, showModal }; // eslint-disable-line
+ // Forward both callback refs (e.g. `(value) => setRef(value)`) and
+ // object refs. Without the callback-ref branch, parents that pass a
+ // function ref get silently no-op'd and can't call close/open/showModal.
+ const refValue = { close, open, showModal };
+ if (typeof ref === 'function') {
+ ref(refValue);
+ } else if (ref) {
+ ref.current = refValue; // eslint-disable-line
Review Comment:
**Suggestion:** The callback ref is being invoked directly during render. If
a parent passes a callback ref that updates state (the common pattern shown in
your own comment), this triggers parent updates while this component is
rendering and can cause React warnings or render loops in Strict Mode. Move ref
exposure to `useImperativeHandle` (or a commit-phase effect) so ref updates
happen in React's ref lifecycle, not in render. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Callback-ref consumers hit React render-phase update warnings.
- ⚠️ StrictMode logs noisy warnings during normal modal usage.
- ⚠️ Third-party apps using stateful callback refs destabilized.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. In
`superset-frontend/packages/superset-ui-core/src/components/ModalTrigger/index.tsx:59-80`,
`ModalTrigger` is defined as a `forwardRef` component whose render function
receives a
`ref: ForwardedRef<ModalTriggerRef['current']>`.
2. The comment at `ModalTrigger/index.tsx:93-95` explicitly documents
callback-ref usage
like `(value) => setRef(value)`, implying a parent may pass a ref callback
that calls a
React state setter.
3. On every render of `ModalTrigger`, the block at
`ModalTrigger/index.tsx:96-99` executes
in the render phase: it builds `refValue = { close, open, showModal }` and
then, if
`typeof ref === 'function'`, synchronously invokes `ref(refValue)` inside
the render
function.
4. When a consumer follows the documented pattern and passes a callback ref
that calls a
state setter (for example, `ref={value => setModalRef(value)}` using
`setModalRef` from
`useState`), that setter is invoked from inside `ModalTrigger`'s render at
`index.tsx:96-99`, causing React's "Cannot update a component while
rendering a different
component" warning and potentially unstable re-render loops in Strict Mode.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=2317d18815514a878d2154f881e5afe8&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=2317d18815514a878d2154f881e5afe8&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/packages/superset-ui-core/src/components/ModalTrigger/index.tsx
**Line:** 96:100
**Comment:**
*Logic Error: The callback ref is being invoked directly during render.
If a parent passes a callback ref that updates state (the common pattern shown
in your own comment), this triggers parent updates while this component is
rendering and can cause React warnings or render loops in Strict Mode. Move ref
exposure to `useImperativeHandle` (or a commit-phase effect) so ref updates
happen in React's ref lifecycle, not in render.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39535&comment_hash=fd7893112c8ad3c86663f5523a8d11aa27841b0b29cd587a895361e15b2e9eca&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39535&comment_hash=fd7893112c8ad3c86663f5523a8d11aa27841b0b29cd587a895361e15b2e9eca&reaction=dislike'>👎</a>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
