refactor(input): merge textarea into rc-input and deduplicate shared logic#163
Open
QDyanbing wants to merge 5 commits intoreact-component:masterfrom
Open
refactor(input): merge textarea into rc-input and deduplicate shared logic#163QDyanbing wants to merge 5 commits intoreact-component:masterfrom
QDyanbing wants to merge 5 commits intoreact-component:masterfrom
Conversation
Walkthrough此 PR 在包 Changes
Sequence Diagram(s)sequenceDiagram
participant U as User / App
participant TextArea as TextArea (wrapper)
participant Resizable as ResizableTextArea
participant Calc as calculateNodeHeight
participant RO as ResizeObserver
U->>TextArea: render / 输入事件
TextArea->>Resizable: 转发 props、value、ref
Resizable->>Calc: 请求 auto-size（content, minRows/maxRows）
Calc-->>Resizable: 返回 计算高度/样式
Resizable->>RO: observe(textarea)
RO-->>Resizable: resize 事件
Resizable->>TextArea: 触发 onResize / 更新高度
TextArea->>U: 暴露 ref 方法（focus、blur、nativeElement）
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
Contributor
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly refactors the input component library by integrating the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Comment on lines
+92
to
+93
| | allowClear | boolean \| { clearIcon } | false | If allow to remove textarea content with clear icon | | ||
| | showCount | boolean \| { formatter } | false | Whether show text count | |
Contributor
There was a problem hiding this comment.
TextArea API 文档中 allowClear 和 showCount 的类型定义可以更详细一些，与 Input 组件的文档保持一致，为开发者提供更清晰的信息。
Suggested change
| | allowClear | boolean \| { clearIcon } | false | If allow to remove textarea content with clear icon | | |
| | showCount | boolean \| { formatter } | false | Whether show text count | | |
| | allowClear | boolean \| { clearIcon: ReactNode } | false | If allow to remove textarea content with clear icon | | |
| | showCount | boolean \| { formatter: ({ value: string, count: number, maxLength?: number }) => ReactNode } | false | Whether show text count | |
| } | ||
|
|
||
| const computedStyleCache: Record<string, NodeType> = {}; | ||
| let hiddenTextarea: HTMLTextAreaElement; |
Contributor
There was a problem hiding this comment.
这里的 hiddenTextarea 是一个模块级别的变量，在第一次调用 calculateAutoSizeStyle 时被创建并添加到 document.body 中，但之后从未被移除。这在单页应用中可能会导致微小的内存泄漏，并且在测试环境中可能会对其他测试产生副作用。虽然这是为了性能优化（避免重复创建元素），但建议考虑一种在不需要时进行清理的机制，例如在模块卸载时移除该元素。
| @@ -0,0 +1,487 @@ | |||
| // @ts-nocheck | |||
Contributor
There was a problem hiding this comment.
文件顶部的 // @ts-nocheck 注释会禁用整个文件的 TypeScript 类型检查，这可能会隐藏潜在的类型错误。建议移除此注释，并修复可能出现的任何类型问题，以提高测试代码的健壮性。在 tests/TextArea.focus.test.tsx 和 tests/utils.ts 中也存在同样的问题。
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #163 +/- ##
==========================================
+ Coverage 97.57% 98.35% +0.78%
==========================================
Files 4 12 +8
Lines 206 426 +220
Branches 80 137 +57
==========================================
+ Hits 201 419 +218
- Misses 5 7 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (10)
src/hooks/useMergedValue.ts (1)
5-17: 实现正确，类型定义可以更精确Hook 的逻辑正确：
- 正确处理了
undefined和null的情况，转换为空字符串formatValue始终返回string，满足下游useCountDisplay的类型要求小建议：
MergedValue类型未包含null，但代码检查了它。虽然这是防御性编程（useControlledState可能在某些边缘情况返回null），可以考虑在类型中显式包含：♻️ 可选的类型改进
-type MergedValue = string | number | readonly string[] | bigint | undefined; +type MergedValue = string | number | readonly string[] | bigint | undefined | null;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useMergedValue.ts` around lines 5 - 17, The hook handles null at runtime but the generic constraint ValueType extends MergedValue doesn't include null, so update the types to reflect that defensive check: modify the generic or the MergedValue alias so it explicitly allows null (e.g. ValueType extends MergedValue | null or add null to MergedValue), and ensure useControlledState and downstream consumers like useCountDisplay remain compatible with the widened type; update the type signature in useMergedValue to reference the adjusted MergedValue so formatValue logic and typings align.tests/utils.ts (2)
25-31: wait() 循环次数的选择100 次迭代是经验值还是有特定原因？如果是为了确保所有微任务和定时器都被处理，建议添加注释说明。过多的迭代可能影响测试性能，过少则可能导致间歇性失败。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/utils.ts` around lines 25 - 31, The wait() helper currently loops 100 times with no explanation; either document why 100 iterations are required or make it configurable: update the wait function to accept an optional iterations parameter (default 100) and add a concise comment above wait() explaining that the loop ensures all microtasks and timers flush (and why 100 was chosen), or adjust the default to a smaller, justified value; reference the wait() function and the loop that calls act()/jest.runAllTimers()/Promise.resolve() so reviewers can find and update the exact implementation.
3-6: 重复导入同一函数
onEsResize和onLibResize都从同一路径导入_rs，它们是完全相同的引用。如果这是为了兼容 ES/CommonJS 模块格式，应该从不同的路径导入：♻️ 如果确实需要两种格式
import { _rs as onEsResize, - _rs as onLibResize, -} from '@rc-component/resize-observer/lib/utils/observerUtil'; +} from '@rc-component/resize-observer/es/utils/observerUtil'; +import { + _rs as onLibResize, +} from '@rc-component/resize-observer/lib/utils/observerUtil';如果不需要区分，可以简化：
♻️ 简化导入
import { - _rs as onEsResize, - _rs as onLibResize, + _rs as onResize, } from '@rc-component/resize-observer/lib/utils/observerUtil'; // ... - onLibResize([{ target }]); - onEsResize([{ target }]); + onResize([{ target }]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/utils.ts` around lines 3 - 6, The file imports the same symbol twice (_rs) under two names (onEsResize, onLibResize) causing redundant references; either import the CommonJS and ESM variants from their respective module paths (use '@rc-component/resize-observer/lib/utils/observerUtil' for the lib/CommonJS one and the ESM entry for the ES variant) so onLibResize and onEsResize point to different actual exports, or simplify by removing one alias and use a single import (keep just onEsResize or onLibResize) referencing _rs; update the import line(s) that mention _rs, onEsResize, and onLibResize in tests/utils.ts accordingly.tests/setupFilesAfterEnv.js (1)
3-4: RAF shim 的类型签名不匹配
window.requestAnimationFrame的签名是(callback: FrameRequestCallback) => number，而window.setTimeout的签名是(handler: TimerHandler, timeout?: number) => number。虽然在 JavaScript 运行时能正常工作，但如果启用严格 TypeScript 检查，可能会报类型错误。建议使用类型断言来明确意图：
♻️ 建议的修改
-window.requestAnimationFrame = window.setTimeout; -window.cancelAnimationFrame = window.clearTimeout; +window.requestAnimationFrame = window.setTimeout as unknown as typeof window.requestAnimationFrame; +window.cancelAnimationFrame = window.clearTimeout as unknown as typeof window.cancelAnimationFrame;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/setupFilesAfterEnv.js` around lines 3 - 4, The RAF shim assigns window.requestAnimationFrame and window.cancelAnimationFrame to setTimeout/clearTimeout causing TypeScript signature mismatch; update the assignments in tests/setupFilesAfterEnv.js to use explicit type assertions so signatures match (e.g., cast window.setTimeout to the FrameRequestCallback => number type for requestAnimationFrame and cast window.clearTimeout to the (handle: number) => void type for cancelAnimationFrame) so the runtime shim remains the same but TypeScript no longer reports type errors for requestAnimationFrame and cancelAnimationFrame.src/calculateNodeHeight.tsx (1)
52-59: 缓存键可能为 undefined 导致意外共享当元素没有
id、data-reactid或name属性时，nodeRef将为undefined。虽然第 87-88 行的条件判断useCache && nodeRef防止了 undefined 键被存储，但as string类型断言掩盖了这个问题。建议明确处理这种情况：
♻️ 更安全的实现
export function calculateNodeStyling(node: HTMLElement, useCache = false) { - const nodeRef = (node.getAttribute('id') || + const nodeRef: string | null = node.getAttribute('id') || node.getAttribute('data-reactid') || - node.getAttribute('name')) as string; + node.getAttribute('name'); - if (useCache && computedStyleCache[nodeRef]) { + if (useCache && nodeRef && computedStyleCache[nodeRef]) { return computedStyleCache[nodeRef]; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/calculateNodeHeight.tsx` around lines 52 - 59, In calculateNodeStyling, nodeRef is currently asserted as a string which can mask a real undefined when the element lacks id/data-reactid/name; stop using the "as string" assertion, treat nodeRef as possibly undefined, and change cache accesses to guarded checks (e.g., if (useCache && nodeRef && computedStyleCache[nodeRef]) return ...), and likewise only write into computedStyleCache[nodeRef] when nodeRef is present; update all references in calculateNodeStyling and any caching logic to avoid using undefined as a cache key.assets/index.less (1)
38-49: 样式选择器命名不一致第 38 行硬编码了
.rc-textarea-affix-wrapper，而第 51 行使用了 LESS 变量插值.@{textarea-prefix-cls}-out-of-range。建议保持一致性：♻️ 建议使用变量统一管理
-@textarea-prefix-cls: rc-textarea; -.rc-textarea-affix-wrapper { +@textarea-prefix-cls: rc-textarea; + +.@{textarea-prefix-cls}-affix-wrapper { display: inline-block; box-sizing: border-box; textarea {此外，根据
src/BaseInput.tsx的实现，affixWrapperPrefixCls是动态计算的（${prefixCls}-affix-wrapper）。如果用户传入自定义prefixCls，硬编码的选择器将不会生效。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@assets/index.less` around lines 38 - 49, The stylesheet hardcodes the selector .rc-textarea-affix-wrapper which can break when a custom prefixCls is used; replace the hardcoded selector with the interpolated variable used elsewhere (e.g. .@{textarea-prefix-cls}-affix-wrapper) so it matches the existing .@{textarea-prefix-cls}-out-of-range usage and the dynamic affixWrapperPrefixCls (`${prefixCls}-affix-wrapper`) computed in src/BaseInput.tsx; update the nested textarea block accordingly to use the variable-based selector so styles apply for custom prefixCls values.tests/TextArea.focus.test.tsx (1)
1-1: 不要用@ts-nocheck关闭整文件类型保护。Line 1 会把这组新增测试里对
TextAreaRef、DOM 查询和可空值的类型回归一起静默掉。更稳妥的是把可空值显式收窄后移除这行，并在去掉后跑一次仓库现有的 typecheck。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/TextArea.focus.test.tsx` at line 1, 删除文件顶部的 "@ts-nocheck" 并修复因其被忽略的类型错误：在测试中显式收窄可空类型（例如对 TextAreaRef 的引用、DOM 查询结果和任何可能为 null/undefined 的变量），使用适当的 type guards 或断言（例如 if (!node) throw 或 expect(node).not.toBeNull() 再访问属性，或使用 non-null assertion Только 在确定安全时），更新涉及 TextAreaRef 和所有 DOM 查询 的变量以满足 TypeScript（例如 getByRole/getByTestId 返回可能为 null 的情况），然后移除注释后运行整个仓库的 typecheck 确认没有回归。docs/examples/textarea-show-count.tsx (1)
40-54:Count.exceedFormatter这个示例当前看不出效果。这里的初始值没有超过
max: 5，页面上不会进入“超限裁剪”的分支，读者很难理解 formatter 的作用。建议换成一个实际超限的默认值。📝 建议调整示例
- <TextArea - defaultValue="🔥" + <TextArea + defaultValue="🔥🔥🔥" count={{ show: true, max: 5, exceedFormatter: (val, { max }) => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/examples/textarea-show-count.tsx` around lines 40 - 54, The example for TextArea.count.exceedFormatter doesn't demonstrate its behavior because defaultValue is not over the max; update the TextArea props so defaultValue exceeds count.max (e.g., set defaultValue to a string longer than 5 characters/emojis) so the exceedFormatter branch is executed and readers can see the trimming behavior for the exceedFormatter function on the TextArea component.tests/TextArea.test.tsx (1)
160-179: 存在重复测试场景，建议合并为参数化用例。这两段都在验证
onPressEnter与onKeyDown交互，语义重复且用例名也重复，后续维护成本较高。Also applies to: 297-315
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/TextArea.test.tsx` around lines 160 - 179, The two tests duplicate verifying TextArea's onKeyDown and onPressEnter behavior; replace them with a single parameterized test (e.g., using test.each or a small loop) that iterates over key events (e.g., keyCode 65 -> expect onKeyDown called only; keyCode 13 -> expect onKeyDown and onPressEnter called) to cover both scenarios in one case; update the test name to reflect parameterization, reuse the existing fakeHandleKeyDown and fakeHandlePressEnter mocks and the TextArea render, and remove the redundant block around lines 297-315 so only the new parameterized test remains.src/TextArea.tsx (1)
60-60: Line 60 的非空断言模式可以改进，但当前崩溃风险已被消除。Line 60 仍保留了
?.textArea!的非空断言模式，但实际上调用点已通过防御性编程妥善处理：
- Line 75:
getTextArea()?.focus()使用可选链- Line 82:
getTextArea()?.blur()使用可选链- Line 146: 显式空值检查
if (textarea) { ... }- Line 169:
getTextArea()?.style.height使用可选链现有代码不存在运行时崩溃风险。建议改进 Line 60 的实现为最佳实践：
- const getTextArea = () => resizableTextAreaRef.current?.textArea!; + const getTextArea = () => resizableTextAreaRef.current?.textArea ?? null;这样可消除非空断言，代码意图更清晰。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/TextArea.tsx` at line 60, The getTextArea function currently uses a non-null assertion (resizableTextAreaRef.current?.textArea!)—remove the non-null assertion and return the possibly-undefined textArea directly (resizableTextAreaRef.current?.textArea) so the function's signature reflects that it may be undefined; callers (getTextArea()?.focus(), getTextArea()?.blur(), getTextArea()?.style.height and the explicit if (textarea) checks) already defensively handle undefined, so no further call-site changes are needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Around line 75-97: The README TextArea API table is missing common
passthrough/native textarea props (maxLength, value, defaultValue, onChange)
which the examples/tests use; update the TextArea docs (referencing
Input.TextArea and TextArea) to include at least these properties in the table
and/or add a note stating that all other native textarea attributes are
forwarded to the underlying element (e.g., "Other native textarea props such as
maxLength, value, defaultValue, onChange are supported and passed through").
Ensure the names match the component exports (TextArea / Input.TextArea) so
readers can find them.
In `@src/calculateNodeHeight.tsx`:
- Around line 100-108: Replace the nonstandard attribute name used when creating
the hidden textarea: change the setAttribute call that currently uses
'tab-index' to the standard 'tabindex' (or set the element's tabIndex property)
on the hiddenTextarea variable so the element uses the correct HTML global
attribute; keep the existing aria-hidden and name assignments and append to
document.body as before.
In `@src/hooks/useCountDisplay.ts`:
- Around line 23-34: The code treats mergedMax as falsy which misinterprets an
explicit 0 max as "unset"; update checks to test for null/undefined instead of
falsiness: compute mergedMax as before, then set isOutOfRange to (mergedMax !==
undefined && mergedMax !== null) && valueLength > Number(mergedMax), and set
hasMaxLength to mergedMax !== undefined && mergedMax !== null so the formatter
and fallback string (`${valueLength}${hasMaxLength ? \` / ${mergedMax}\` : ''}`)
will correctly show " / 0" and mark out-of-range when max is 0; keep using
countConfig.showFormatter when present (pass Number(mergedMax) or mergedMax
consistently if needed).
In `@src/hooks/useCountExceed.ts`:
- Line 2: 当前文件的导入 `CountConfig` 在 src/hooks/useCountExceed.ts 中未被使用，触发
no-unused-vars 导致 CI 失败；请在代码中移除该未使用的导入行或将其用于相应的类型注解（例如为函数 useCountExceed
的参数或返回值添加 CountConfig 类型），确保唯一标识符 CountConfig 不再未使用。
- Around line 21-25: The effect that restores the selection uses
getTarget()?.setSelectionRange(...selection) but never clears the saved
selection, so subsequent renders reapply the old range; after calling
setSelectionRange inside the React.useEffect (the effect in useCountExceed.ts
that depends on getTarget and selection), clear the stored selection state (for
example call the selection setter like setSelection(null) or an equivalent reset
function) so the selection is applied once and not reapplied on later renders.
In `@src/Input.tsx`:
- Around line 69-77: The input's effective maxLength from count config isn't
being applied to the native <input> because you compute mergedMax via
useCountDisplay but continue to pass the original maxLength through otherProps;
update the Input component to override/replace the maxLength prop with mergedMax
(or undefined when no limit) before spreading otherProps to the input, and
ensure any references like mergedMax, maxLength, useCountDisplay,
useCountExceed, inputRef are used/kept in that block so the unused-vars warning
on mergedMax disappears; apply the same fix to the other occurrence around lines
188-216 where mergedMax must be injected into the native input props instead of
the raw maxLength.
In `@src/ResizableTextArea.tsx`:
- Around line 39-50: The autoSize recalculation only listens to the external
value prop and thus doesn't update for uncontrolled usage where
internalValue/mergedValue changes; update the effect that measures/resizes
(inside ResizableTextArea) to depend on mergedValue (or internalValue) instead
of just value so that typing triggers a re-measure, and/or call the same
resize/measure routine from onInternalChange after setMergedValue; reference
useControlledState, mergedValue/internalValue, setMergedValue, onInternalChange
and the resize/measure effect in ResizableTextArea to locate where to add
mergedValue to the effect dependencies or invoke the resize function.
In `@src/TextArea.tsx`:
- Around line 64-68: The textarea's native maxLength is still using the original
maxLength instead of the resolved mergedMax from useCountDisplay, breaking
intended priority; update the TextArea component to pass mergedMax (not
maxLength) into the rendered <textarea> props so browser enforcement matches the
count logic (change the prop usage where the textarea is rendered—refer to
mergedMax and useCountDisplay in the TextArea component).
In `@tests/TextArea.allowClear.test.tsx`:
- Around line 15-17: Fix the typo in the visibility class used in the assertion:
change the selector '.rc-textarea-clear-icon-hidhen' to
'.rc-textarea-clear-icon-hidden' in the test (the expect call that uses
container.querySelector('.rc-textarea-clear-icon-hidhen')). This will ensure the
"icon should be visible after input" assertion actually checks the correct DOM
class.
In `@tests/TextArea.focus.test.tsx`:
- Around line 45-75: 当前两个用例分别调用 ref.current.focus() / ref.current.blur() 但对
onFocus / onBlur 的断言仍然依赖随后手动 fireEvent，这没有覆盖“imperative 调用触发回调”的路径。修改
tests/TextArea.focus.test.tsx 中相关用例（涉及 TextArea, TextAreaRef,
ref.current.focus(), ref.current.blur(), onFocus, onBlur 和
container.querySelector('textarea')）使断言直接验证在调用 ref.current.focus() /
ref.current.blur() 后回调被触发（并移除或合并随后单独的 fireEvent 调用），确保测试集中在同一路径上；如果需要保留 DOM
事件用例，把 imperative 调用的断言移到一个独立的测试中以避免相互掩盖。
In `@tests/TextArea.test.tsx`:
- Around line 103-158: The test creates an errorSpy via jest.spyOn(console,
'error').mockImplementation but never restores it, causing global test
pollution; update the test to ensure errorSpy.mockRestore() is called after the
assertions (or in a finally/afterEach cleanup) so console.error is returned to
its original implementation—locate the spy instantiation (errorSpy) near the top
of the test and add a mockRestore call after the final expect (or wrap the test
body to always restore errorSpy) to guarantee no leakage to other tests.
---
Nitpick comments:
In `@assets/index.less`:
- Around line 38-49: The stylesheet hardcodes the selector
.rc-textarea-affix-wrapper which can break when a custom prefixCls is used;
replace the hardcoded selector with the interpolated variable used elsewhere
(e.g. .@{textarea-prefix-cls}-affix-wrapper) so it matches the existing
.@{textarea-prefix-cls}-out-of-range usage and the dynamic affixWrapperPrefixCls
(`${prefixCls}-affix-wrapper`) computed in src/BaseInput.tsx; update the nested
textarea block accordingly to use the variable-based selector so styles apply
for custom prefixCls values.
In `@docs/examples/textarea-show-count.tsx`:
- Around line 40-54: The example for TextArea.count.exceedFormatter doesn't
demonstrate its behavior because defaultValue is not over the max; update the
TextArea props so defaultValue exceeds count.max (e.g., set defaultValue to a
string longer than 5 characters/emojis) so the exceedFormatter branch is
executed and readers can see the trimming behavior for the exceedFormatter
function on the TextArea component.
In `@src/calculateNodeHeight.tsx`:
- Around line 52-59: In calculateNodeStyling, nodeRef is currently asserted as a
string which can mask a real undefined when the element lacks
id/data-reactid/name; stop using the "as string" assertion, treat nodeRef as
possibly undefined, and change cache accesses to guarded checks (e.g., if
(useCache && nodeRef && computedStyleCache[nodeRef]) return ...), and likewise
only write into computedStyleCache[nodeRef] when nodeRef is present; update all
references in calculateNodeStyling and any caching logic to avoid using
undefined as a cache key.
In `@src/hooks/useMergedValue.ts`:
- Around line 5-17: The hook handles null at runtime but the generic constraint
ValueType extends MergedValue doesn't include null, so update the types to
reflect that defensive check: modify the generic or the MergedValue alias so it
explicitly allows null (e.g. ValueType extends MergedValue | null or add null to
MergedValue), and ensure useControlledState and downstream consumers like
useCountDisplay remain compatible with the widened type; update the type
signature in useMergedValue to reference the adjusted MergedValue so formatValue
logic and typings align.
In `@src/TextArea.tsx`:
- Line 60: The getTextArea function currently uses a non-null assertion
(resizableTextAreaRef.current?.textArea!)—remove the non-null assertion and
return the possibly-undefined textArea directly
(resizableTextAreaRef.current?.textArea) so the function's signature reflects
that it may be undefined; callers (getTextArea()?.focus(),
getTextArea()?.blur(), getTextArea()?.style.height and the explicit if
(textarea) checks) already defensively handle undefined, so no further call-site
changes are needed.
In `@tests/setupFilesAfterEnv.js`:
- Around line 3-4: The RAF shim assigns window.requestAnimationFrame and
window.cancelAnimationFrame to setTimeout/clearTimeout causing TypeScript
signature mismatch; update the assignments in tests/setupFilesAfterEnv.js to use
explicit type assertions so signatures match (e.g., cast window.setTimeout to
the FrameRequestCallback => number type for requestAnimationFrame and cast
window.clearTimeout to the (handle: number) => void type for
cancelAnimationFrame) so the runtime shim remains the same but TypeScript no
longer reports type errors for requestAnimationFrame and cancelAnimationFrame.
In `@tests/TextArea.focus.test.tsx`:
- Line 1: 删除文件顶部的 "@ts-nocheck" 并修复因其被忽略的类型错误：在测试中显式收窄可空类型（例如对 TextAreaRef
的引用、DOM 查询结果和任何可能为 null/undefined 的变量），使用适当的 type guards 或断言（例如 if (!node) throw
或 expect(node).not.toBeNull() 再访问属性，或使用 non-null assertion Только 在确定安全时），更新涉及
TextAreaRef 和所有 DOM 查询 的变量以满足 TypeScript（例如 getByRole/getByTestId 返回可能为 null
的情况），然后移除注释后运行整个仓库的 typecheck 确认没有回归。
In `@tests/TextArea.test.tsx`:
- Around line 160-179: The two tests duplicate verifying TextArea's onKeyDown
and onPressEnter behavior; replace them with a single parameterized test (e.g.,
using test.each or a small loop) that iterates over key events (e.g., keyCode 65
-> expect onKeyDown called only; keyCode 13 -> expect onKeyDown and onPressEnter
called) to cover both scenarios in one case; update the test name to reflect
parameterization, reuse the existing fakeHandleKeyDown and fakeHandlePressEnter
mocks and the TextArea render, and remove the redundant block around lines
297-315 so only the new parameterized test remains.
In `@tests/utils.ts`:
- Around line 25-31: The wait() helper currently loops 100 times with no
explanation; either document why 100 iterations are required or make it
configurable: update the wait function to accept an optional iterations
parameter (default 100) and add a concise comment above wait() explaining that
the loop ensures all microtasks and timers flush (and why 100 was chosen), or
adjust the default to a smaller, justified value; reference the wait() function
and the loop that calls act()/jest.runAllTimers()/Promise.resolve() so reviewers
can find and update the exact implementation.
- Around line 3-6: The file imports the same symbol twice (_rs) under two names
(onEsResize, onLibResize) causing redundant references; either import the
CommonJS and ESM variants from their respective module paths (use
'@rc-component/resize-observer/lib/utils/observerUtil' for the lib/CommonJS one
and the ESM entry for the ES variant) so onLibResize and onEsResize point to
different actual exports, or simplify by removing one alias and use a single
import (keep just onEsResize or onLibResize) referencing _rs; update the import
line(s) that mention _rs, onEsResize, and onLibResize in tests/utils.ts
accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7c84889c-9f92-4eda-827a-f1956fba416e
⛔ Files ignored due to path filters (2)
tests/__snapshots__/TextArea.allowClear.test.tsx.snapis excluded by!**/*.snaptests/__snapshots__/TextArea.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (27)
README.mdassets/index.lessdocs/demo/textarea.mddocs/examples/textarea-allow-clear.tsxdocs/examples/textarea-auto-size.tsxdocs/examples/textarea-basic.tsxdocs/examples/textarea-show-count.tsxpackage.jsonsrc/Input.tsxsrc/ResizableTextArea.tsxsrc/TextArea.tsxsrc/calculateNodeHeight.tsxsrc/hooks/useCount.tssrc/hooks/useCountDisplay.tssrc/hooks/useCountExceed.tssrc/hooks/useMergedValue.tssrc/index.tsxsrc/interface.tstests/ResizableTextArea.test.tsxtests/TextArea.allowClear.test.tsxtests/TextArea.count.test.tsxtests/TextArea.focus.test.tsxtests/TextArea.showCount.test.tsxtests/TextArea.test.tsxtests/count.test.tsxtests/setupFilesAfterEnv.jstests/utils.ts
There was a problem hiding this comment.
♻️ Duplicate comments (2)
src/hooks/useCountExceed.ts (1)
20-24:⚠️ Potential issue | 🟠 Major恢复选区后需要立即清空缓存。
这里在
setSelectionRange后没有重置selection。结合src/Input.tsx的 Line 50-77，getTarget目前是每次渲染都会新建的回调；只要后续发生重渲染，这个 effect 就会再次执行，把光标反复拉回旧位置。建议修改
React.useEffect(() => { - if (selection) { - getTarget()?.setSelectionRange(...selection); - } + if (!selection) { + return; + } + + getTarget()?.setSelectionRange(...selection); + setSelection(null); }, [getTarget, selection]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useCountExceed.ts` around lines 20 - 24, The effect in useCountExceed.ts calls getTarget()?.setSelectionRange(...selection) but never clears the saved selection, so on subsequent renders the effect re-applies the old range; after restoring the range you must reset the stored selection to avoid re-running — update the effect to call the setter that clears selection (e.g., setSelection(null) or setSelection(undefined)) immediately after getTarget()?.setSelectionRange(...selection), referencing the useCountExceed hook's selection and its setter so the cached selection is cleared once applied.src/TextArea.tsx (1)
64-68:⚠️ Potential issue | 🟠 Major不要绕过
count.max/maxLength的合并结果。当前版本只是把
mergedMax的解构去掉了，但 Line 215 仍然把原始maxLength传给ResizableTextArea。当两者同时存在且取值不一致时，浏览器原生限制会先于计数逻辑生效，导致截断优先级和dataCount展示不一致。建议修改
- const { isOutOfRange, dataCount } = useCountDisplay({ + const { mergedMax, isOutOfRange, dataCount } = useCountDisplay({ countConfig, value: formatValue, maxLength, }); @@ - maxLength={maxLength} + maxLength={mergedMax}Also applies to: 212-215
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/TextArea.tsx` around lines 64 - 68, The code currently ignores the merged max length from useCountDisplay and still forwards the original maxLength to ResizableTextArea, causing native browser truncation to disagree with dataCount; update the destructuring from useCountDisplay to include mergedMax (e.g., const { isOutOfRange, dataCount, mergedMax } = useCountDisplay({...})) and pass mergedMax into the ResizableTextArea prop instead of maxLength so the textarea's native limit matches the merged/count logic (ensure you keep passing countConfig, value/formatValue as before).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/hooks/useCountExceed.ts`:
- Around line 20-24: The effect in useCountExceed.ts calls
getTarget()?.setSelectionRange(...selection) but never clears the saved
selection, so on subsequent renders the effect re-applies the old range; after
restoring the range you must reset the stored selection to avoid re-running —
update the effect to call the setter that clears selection (e.g.,
setSelection(null) or setSelection(undefined)) immediately after
getTarget()?.setSelectionRange(...selection), referencing the useCountExceed
hook's selection and its setter so the cached selection is cleared once applied.
In `@src/TextArea.tsx`:
- Around line 64-68: The code currently ignores the merged max length from
useCountDisplay and still forwards the original maxLength to ResizableTextArea,
causing native browser truncation to disagree with dataCount; update the
destructuring from useCountDisplay to include mergedMax (e.g., const {
isOutOfRange, dataCount, mergedMax } = useCountDisplay({...})) and pass
mergedMax into the ResizableTextArea prop instead of maxLength so the textarea's
native limit matches the merged/count logic (ensure you keep passing
countConfig, value/formatValue as before).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ac370b85-a16c-48ea-8abe-30add1591b37
📒 Files selected for processing (3)
src/Input.tsxsrc/TextArea.tsxsrc/hooks/useCountExceed.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Input.tsx
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/hooks/useCountExceed.ts (1)
47-50: 可选优化：缓存目标元素，减少重复调用并提升可读性。Line 47-50 可以先取一次
target再读取selectionStart/selectionEnd，让意图更直接。建议修改
if (currentValue !== nextValue) { + const target = getTargetRef.current(); setSelection([ - getTargetRef.current()?.selectionStart || 0, - getTargetRef.current()?.selectionEnd || 0, + target?.selectionStart || 0, + target?.selectionEnd || 0, ]); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useCountExceed.ts` around lines 47 - 50, Cache the target element returned by getTargetRef.current() into a local variable before reading selectionStart/selectionEnd to avoid repeated calls and improve readability; e.g., assign const target = getTargetRef.current(), then call setSelection([target?.selectionStart || 0, target?.selectionEnd || 0]) so the logic still defaults to 0 when target is null while only accessing getTargetRef.current() once.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/hooks/useCountExceed.ts`:
- Around line 47-50: Cache the target element returned by getTargetRef.current()
into a local variable before reading selectionStart/selectionEnd to avoid
repeated calls and improve readability; e.g., assign const target =
getTargetRef.current(), then call setSelection([target?.selectionStart || 0,
target?.selectionEnd || 0]) so the logic still defaults to 0 when target is null
while only accessing getTargetRef.current() once.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 087742c5-b1ec-40b8-a4bc-18aaa49192b1
📒 Files selected for processing (3)
src/hooks/useCountExceed.tstests/TextArea.allowClear.test.tsxtests/count.test.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/count.test.tsx
- tests/TextArea.allowClear.test.tsx
Member
|
@gemini-code-assist，重新 CR 一下。这个 PR 是把 https://github.com/react-component/textarea 的逻辑迁移到这个项目中。所以最最要的是比较迁移时是否有 break change，而不是语法问题。 |
Contributor
There was a problem hiding this comment.
Code Review
This pull request successfully refactors the rc-input component to integrate textarea capabilities and deduplicate shared logic. The introduction of Input.TextArea and a named TextArea export provides a clean and consistent API. The refactoring into specialized hooks (useCountDisplay, useCountExceed, useMergedValue) significantly improves maintainability and separation of concerns. Comprehensive test coverage for the new TextArea component and its features ensures stability. Overall, this is a well-executed refactoring that enhances the component's architecture without altering existing external behavior.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
背景
本次 PR 包含两部分工作：
textarea能力正式并入rc-inputInput与TextArea之间的重复实现在迁移完成后，
rc-input已经成为input与textarea后续维护的主要承接位置。基于这个前提，这次进一步对内部实现做整理，目标是在不改变既有行为的前提下，降低重复代码和后续维护成本。主要改动
1. 将 textarea 能力并入 rc-input
将原先
textarea的核心实现迁移到rc-input，使rc-input能够直接承接 textarea 的相关能力，包括：TextArea主组件实现ResizableTextArea同时在
rc-input中提供：Input.TextAreaTextArea这意味着后续 textarea 相关能力可以直接在
rc-input中维护和演进。2. 收敛 Input 与 TextArea 的共享逻辑
在 textarea 合入后，
Input与TextArea在以下方面存在较多重复实现：showCount/count配置归并count.max超限裁剪exceedFormatter本次将这些可以稳定复用、且不会抹平组件差异的部分进行了收敛，主要抽离为几类共享能力：
这样可以避免
Input与TextArea长期各自维护一套相近实现。3. 保留组件之间原本存在的行为差异
这次重构没有强行把
Input与TextArea抽成一套完全统一的事件流程。像 composition / IME、事件触发时机、交互细节等存在已知差异的部分，仍然保留在各自组件内部处理，避免因为过度抽象而引入行为偏差。
也就是说，这次抽离的是“适合共享的纯逻辑”，而不是把所有实现都揉成一层通用逻辑。
行为影响说明
本次 PR 的原则是：
对上层使用方的影响
从上层接入方式看，这次改动风险较低。
以 antd 当前使用方式为例：
Input使用@rc-component/inputTextArea当前仍有自己的包装层因此，这次
rc-input的内部整理不会直接改变上层的 API 形状。对于未来逐步将 textarea 维护进一步收口到rc-input，这次改动也为后续继续演进打下了更稳定的内部基础。测试与验证
已完成以下验证：
npm test -- --runInBandnpx tsc -p tsconfig.json --noEmit结果均通过。
另外，针对这次重构涉及的关键路径，也补充和确认了回归覆盖，重点包括：
count.max与maxLength同时存在时的优先级Input/TextArea的 count 展示路径TextArea在未开启showCount时不会产生额外 count 展示副作用Summary by CodeRabbit
发布说明
新功能
文档
样式
测试
依赖更新