Skip to content

fix(select): merge raw trigger element event handlers #1215

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
afc163 merged 1 commit into from
Feb 26, 2026
+38 −2
Merged

fix(select): merge raw trigger element event handlers #1215

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 17 additions & 2 deletions src/SelectInput/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,14 +219,29 @@ export default React.forwardRef<SelectInputRef, SelectInputProps>(function Selec
};

if (RootComponent) {
const originProps = (RootComponent as any).props || {};
const mergedProps = { ...originProps, ...domProps };

Object.keys(originProps).forEach((key) => {
const originVal = originProps[key];
const domVal = domProps[key];

if (typeof originVal === 'function' && typeof domVal === 'function') {
mergedProps[key] = (...args: any[]) => {
domVal(...args);
originVal(...args);
};
}
});
Comment on lines +225 to +235
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 26, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

仅合并事件处理函数，避免误合并普通函数属性

当前会合并所有同名函数属性，不仅是事件处理器。这样可能误伤非事件函数（例如依赖返回值的函数），与本次修复目标不一致。建议只对事件键（如 onFocus/onBlur）做组合调用。

🔧 建议修改 
 Object.keys(originProps).forEach((key) => {
   const originVal = originProps[key];
   const domVal = domProps[key];

-  if (typeof originVal === 'function' && typeof domVal === 'function') {
+  const isEventHandler = /^on[A-Z]/.test(key);
+  if (isEventHandler && typeof originVal === 'function' && typeof domVal === 'function') {
     mergedProps[key] = (...args: any[]) => {
       domVal(...args);
       originVal(...args);
     };
   }
 });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
Object.keys(originProps).forEach((key) => {
const originVal = originProps[key];
const domVal = domProps[key];
if (typeof originVal === 'function' && typeof domVal === 'function') {
mergedProps[key] = (...args: any[]) => {
domVal(...args);
originVal(...args);
};
}
});
Object.keys(originProps).forEach((key) => {
const originVal = originProps[key];
const domVal = domProps[key];
const isEventHandler = /^on[A-Z]/.test(key);
if (isEventHandler && typeof originVal === 'function' && typeof domVal === 'function') {
mergedProps[key] = (...args: any[]) => {
domVal(...args);
originVal(...args);
};
}
});
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/SelectInput/index.tsx` around lines 225 - 235, The current merging loop
in SelectInput (iterating Object.keys(originProps) and combining any same-named
functions into mergedProps) wrongly combines non-event functions; change the
condition to only compose keys that are event handlers (e.g., match /^on[A-Z]/)
and where both originProps[key] and domProps[key] are functions, leaving other
same-named functions untouched; use the existing symbols originProps, domProps,
mergedProps and only create the combined wrapper for keys matching the event
pattern so non-event functions that rely on return values are not altered.


if (React.isValidElement<any>(RootComponent)) {
return React.cloneElement(RootComponent, {
...domProps,
...mergedProps,
ref: composeRef((RootComponent as any).ref, rootRef),
});
}

return <RootComponent {...domProps} ref={rootRef} />;
return <RootComponent {...mergedProps} ref={rootRef} />;
Comment on lines +222 to +244
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Feb 26, 2026

Choose a reason for hiding this comment

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

medium

处理 RootComponent 的逻辑可以进行重构以提高代码清晰度和类型安全。通过首先检查 RootComponent 是否为 ReactElement，我们可以将元素和组件类型的处理逻辑分开。这避免了在 RootComponent 是组件类型时不必要地执行属性合并逻辑，同时也移除了 any 类型断言，使代码更健壮且易于维护。

    if (React.isValidElement(RootComponent)) {
      const originProps = RootComponent.props || {};
      const mergedProps = { ...originProps, ...domProps };

      Object.keys(originProps).forEach(key => {
        const originVal = originProps[key];
        const domVal = domProps[key];

        if (typeof originVal === 'function' && typeof domVal === 'function') {
          mergedProps[key] = (...args: any[]) => {
            domVal(...args);
            originVal(...args);
          };
        }
      });

      return React.cloneElement(RootComponent, {
        ...mergedProps,
        ref: composeRef(RootComponent.ref, rootRef),
      });
    }

    return <RootComponent {...domProps} ref={rootRef} />;

}

return (
Expand Down
21 changes: 21 additions & 0 deletions tests/Custom.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,25 @@ describe('Select.Custom', () => {

expect(onPopupVisibleChange).toHaveBeenCalledWith(true);
});

it('should not override raw input element event handlers', () => {
const onFocus = jest.fn();
const onBlur = jest.fn();

const { getByPlaceholderText } = render(
<Select
showSearch
options={[{ value: 'a', label: 'A' }]}
getRawInputElement={() => (
<input placeholder="focus me" onFocus={onFocus} onBlur={onBlur} />
)}
/>,
);

fireEvent.focus(getByPlaceholderText('focus me'));
fireEvent.blur(getByPlaceholderText('focus me'));

expect(onFocus).toHaveBeenCalled();
expect(onBlur).toHaveBeenCalled();
});
});
Loading