rusackas commented on code in PR #39535:
URL: https://github.com/apache/superset/pull/39535#discussion_r3430735921
##########
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:
The render-phase ref assignment is pre-existing here — master already does
`ref.current = {...}` in the render body; this PR only adds the callback-ref
branch and fixes the typing. Moving to `useImperativeHandle` would change the
timing semantics for existing consumers, which is out of scope for a TS6
forward-compat pass. Happy to look at it as its own change.
##########
superset-frontend/packages/superset-ui-core/src/components/Select/AsyncSelect.tsx:
##########
@@ -626,14 +642,16 @@ const AsyncSelect = forwardRef(
setAllValuesLoaded(false);
};
- useImperativeHandle(
- ref,
- () => ({
- ...(ref.current as RefSelectProps),
+ useImperativeHandle(ref, () => {
+ const current =
+ ref && typeof ref !== 'function' && ref.current
+ ? (ref.current as RefSelectProps)
+ : ({} as RefSelectProps);
+ return {
+ ...current,
clearCache,
- }),
- [ref],
- );
+ };
+ }, [ref]);
Review Comment:
This spread-from-`ref.current` with `[ref]` deps is exactly what master
already does — the PR only adds a null/function guard so it type-checks under
TS6. Reworking the ref to merge a separate internal select ref is a real
behavior change beyond this forward-compat pass, so I would rather not fold it
in here.
--
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]
