feat: add inputProps prop to pass props to internal input#1221
feat: add inputProps prop to pass props to internal input#1221EmilyyyLiu wants to merge 2 commits intoreact-component:masterfrom
Conversation
- Add inputProps to BaseSelectProps, SelectInputProps and InputProps - Merge user inputProps with default props in Content/index.tsx - Add test case for inputProps - Update README with API documentation Close ant-design/ant-design#38012
|
Someone is attempting to deploy a commit to the React Component Team on Vercel. A member of the Team first needs to authorize it. |
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Walkthrough为 Select 组件引入可选的 inputProps(React.InputHTMLAttributes),在 BaseSelect → SelectInput → SelectContent → Input 链路中合并并应用到实际渲染的 input 元素;同时添加单元测试并在 README 中记录该 prop。 ChangesInput Props 透传功能
Sequence Diagram(s)sequenceDiagram
participant BaseSelect
participant SelectInput
participant SelectContent
participant Input
participant HTMLInput
BaseSelect->>SelectInput: 接收 props (含 inputProps)
SelectInput->>SelectContent: 在 context 中包含 inputProps
SelectContent->>Input: 传递 userInputProps
Input->>Input: 合并 inputProps 到 sharedInputProps
Input->>HTMLInput: 渲染并应用最终属性
Estimated code review effort🎯 2 (简单) | ⏱️ ~12 分钟 Possibly related PRs
Suggested reviewers
诗歌
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1221 +/- ##
=======================================
Coverage 99.44% 99.44%
=======================================
Files 31 31
Lines 1256 1256
Branches 437 459 +22
=======================================
Hits 1249 1249
Misses 7 7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Code Review
This pull request introduces an inputProps property to allow passing standard HTML input attributes to the internal input element across BaseSelect, SelectInput, and Input components. The changes include documentation and a test case. Feedback highlights that inputProps should be destructured in SelectInput to avoid React warnings on the wrapper element, and user-provided styles or classes within inputProps should be merged rather than overwritten in the Input component.
| components: ComponentsConfig; | ||
| children?: React.ReactElement; | ||
| /** Props passed to the internal input element */ | ||
| inputProps?: React.InputHTMLAttributes<HTMLInputElement>; |
There was a problem hiding this comment.
The inputProps prop added to the interface should also be destructured in the SelectInput component body (around line 76). Currently, it is not destructured, which means it remains in restProps and is subsequently included in domProps. This leads to inputProps being passed as a DOM attribute to the wrapper div element at line 261, which will trigger a React warning about unknown props on a DOM element.
There was a problem hiding this comment.
已经修改了,使用DEFAULT_OMIT_PROPS 排除了
| ...inputProps, | ||
| ref: inputRef as React.Ref<HTMLInputElement>, | ||
| style: { | ||
| ...styles?.input, |
There was a problem hiding this comment.
The style and className properties from inputProps are currently overwritten by the component's explicit assignments that follow the spread. To ensure user-provided styles and classes are applied, they should be merged. Note that className also needs to be merged into the inputCls definition (around line 54), though that line is outside the current diff hunk.
| ...inputProps, | |
| ref: inputRef as React.Ref<HTMLInputElement>, | |
| style: { | |
| ...styles?.input, | |
| ...inputProps, | |
| ref: inputRef as React.Ref<HTMLInputElement>, | |
| style: { | |
| ...styles?.input, | |
| ...inputProps?.style, |
There was a problem hiding this comment.
暂时不考虑这个style问题,应该统一由styles处理
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/SelectInput/index.tsx (1)
55-70:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
inputProps缺少过滤,对象值会被展开到根<div>上,触发 React DOM 警告
inputProps未在组件函数体中显式解构,也未加入DEFAULT_OMIT_PROPS,因此它会经由domProps最终展开到根<div>上。React 会在控制台输出警告:Warning: React does not recognize the
inputPropsprop on a DOM element…对比
tabIndex、focused等同类 "仅供内部消费" 的 props,它们已加入DEFAULT_OMIT_PROPS以阻止 DOM 透传,inputProps应当遵循相同模式。
inputProps通过contextValue = { ...props, ... }已正确进入SelectInputContext,功能链路本身正常,只需防止其漏出到 DOM 即可。🐛 修复:将 `inputProps` 加入 `DEFAULT_OMIT_PROPS`
const DEFAULT_OMIT_PROPS = [ 'value', 'onChange', 'removeIcon', 'placeholder', 'maxTagCount', 'maxTagTextLength', 'maxTagPlaceholder', 'choiceTransitionName', 'onInputKeyDown', 'onPopupScroll', 'tabIndex', 'activeValue', 'onSelectorRemove', 'focused', + 'inputProps', ] as const;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/SelectInput/index.tsx` around lines 55 - 70, DEFAULT_OMIT_PROPS currently omits internal-only props but is missing inputProps, which causes inputProps to be spread into domProps and onto the root <div>; add 'inputProps' to the DEFAULT_OMIT_PROPS array so it is omitted from domProps (preventing React DOM warnings) while leaving the code that injects inputProps into SelectInputContext / contextValue unchanged.
🧹 Nitpick comments (1)
src/SelectInput/Content/index.tsx (1)
24-30: ⚡ Quick win
inputProps中的事件处理函数会被静默覆盖,类型定义过于宽泛当前的合并顺序:
Content/index.tsx:userInputProps.onKeyDown被onInputKeyDown替换(而非合并),用户的onKeyDown直接被丢弃。Input.tsx:onChange、onBlur等内部处理函数位于...inputProps之后,用户通过inputProps传入的同名事件处理函数全部被静默忽略。但当前类型
React.InputHTMLAttributes<HTMLInputElement>包含完整的事件处理函数类型,用户很可能传入onChange、onBlur等并期望它们生效,实际上却静默无效,难以排查。建议收窄类型,明确排除事件处理函数(
React.DOMAttributes中的内容),例如:- inputProps?: React.InputHTMLAttributes<HTMLInputElement>; + inputProps?: Omit<React.InputHTMLAttributes<HTMLInputElement>, keyof React.DOMAttributes<HTMLInputElement>>;或在 README 和 JSDoc 中明确说明事件处理函数会被忽略。若需要合并
onKeyDown,可改为:- onKeyDown: onInputKeyDown, + onKeyDown: (e: React.KeyboardEvent<HTMLInputElement>) => { + userInputProps?.onKeyDown?.(e); + onInputKeyDown?.(e); + },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/SelectInput/Content/index.tsx` around lines 24 - 30, The current merging silently overwrites user event handlers: userInputProps (and final inputProps) uses React.InputHTMLAttributes which includes DOM event handlers, so user onChange/onBlur/onKeyDown get ignored by internal handlers in Content/index.tsx (sharedInputProps) and Input.tsx; fix by narrowing the type used for userInputProps/sharedInputProps to explicitly exclude DOM event handlers (e.g. Omit<React.InputHTMLAttributes<HTMLInputElement>, keyof React.DOMAttributes<any>>) or by whitelisting only safe attributes, and change the onKeyDown merge in Content/index.tsx to call both handlers (invoke userInputProps.onKeyDown if present, then onInputKeyDown, or vice versa) so user callbacks are preserved; also update README/JSDoc to state that event handlers are either ignored or merged based on chosen approach.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/SelectInput/index.tsx`:
- Around line 55-70: DEFAULT_OMIT_PROPS currently omits internal-only props but
is missing inputProps, which causes inputProps to be spread into domProps and
onto the root <div>; add 'inputProps' to the DEFAULT_OMIT_PROPS array so it is
omitted from domProps (preventing React DOM warnings) while leaving the code
that injects inputProps into SelectInputContext / contextValue unchanged.
---
Nitpick comments:
In `@src/SelectInput/Content/index.tsx`:
- Around line 24-30: The current merging silently overwrites user event
handlers: userInputProps (and final inputProps) uses React.InputHTMLAttributes
which includes DOM event handlers, so user onChange/onBlur/onKeyDown get ignored
by internal handlers in Content/index.tsx (sharedInputProps) and Input.tsx; fix
by narrowing the type used for userInputProps/sharedInputProps to explicitly
exclude DOM event handlers (e.g.
Omit<React.InputHTMLAttributes<HTMLInputElement>, keyof
React.DOMAttributes<any>>) or by whitelisting only safe attributes, and change
the onKeyDown merge in Content/index.tsx to call both handlers (invoke
userInputProps.onKeyDown if present, then onInputKeyDown, or vice versa) so user
callbacks are preserved; also update README/JSDoc to state that event handlers
are either ignored or merged based on chosen approach.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a88676fa-d39c-416c-af6f-ebdf9d169fe9
📒 Files selected for processing (6)
README.mdsrc/BaseSelect/index.tsxsrc/SelectInput/Content/index.tsxsrc/SelectInput/Input.tsxsrc/SelectInput/index.tsxtests/BaseSelect.test.tsx
|
|
||
| // >>> Input | ||
| /** Props passed to the internal input element */ | ||
| inputProps?: React.InputHTMLAttributes<HTMLInputElement>; |
There was a problem hiding this comment.
已经有 components 可以自定义子组件,inputProps 有点多余了。
There was a problem hiding this comment.
可以components.input = Input 。我去回复issue吧
Background
Users need to customize the internal input element of Select component (e.g., disable spellCheck). Currently there's no way to pass props to the internal input.
Changes
inputPropsto BaseSelectProps interfaceinputPropsto SelectInputProps interfaceinputPropstype and merge to sharedInputPropsUsage
Related Issue
Summary by CodeRabbit
发布说明
新功能
测试
文档