fix: use committed count value during IME composition#179
Conversation
概览该 PR 为 Input 和 TextArea 组件的计数显示（showCount）添加了 IME（输入法编辑器）感知能力。引入独立的 更改IME 感知的计数值状态管理
🎯 3 (Moderate) | ⏱️ ~25 minutes建议审阅人
诗
🚥 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)
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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the Input and TextArea components to ensure that the character count display reflects the committed value during IME composition instead of intermediate phonetic text, and adds corresponding unit tests. The reviewer suggests a refactoring to simplify state management: instead of maintaining a separate countValue state synced via useEffect (which can cause unnecessary double-renders), the components should track only the value at the start of composition (compositionStartValue) and derive the display count value directly during render.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| // Count-related state should reflect the committed value during IME composition | ||
| // instead of the intermediate phonetic text. | ||
| const [countValue, setCountValue] = useState(formatValue); |
There was a problem hiding this comment.
Instead of maintaining a separate countValue state and syncing it with formatValue via useEffect, we can derive countValue directly during render. By tracking only the compositionStartValue (the value when IME composition starts), we can simplify the state logic, eliminate the useEffect entirely, and avoid potential double-renders or out-of-sync issues.
| // Count-related state should reflect the committed value during IME composition | |
| // instead of the intermediate phonetic text. | |
| const [countValue, setCountValue] = useState(formatValue); | |
| // Track the value at the start of IME composition to reflect the committed value | |
| // during composition instead of the intermediate phonetic text. | |
| const [compositionStartValue, setCompositionStartValue] = useState<string | null>(null); | |
| const countValue = compositionStartValue !== null ? compositionStartValue : formatValue; |
| useEffect(() => { | ||
| if (!compositionRef.current) { | ||
| setCountValue(formatValue); | ||
| } | ||
| }, [formatValue]); |
There was a problem hiding this comment.
With the derived countValue approach, this useEffect is no longer needed, which prevents unnecessary double-renders when formatValue changes.
| return; | ||
| } | ||
| setValue(cutValue); | ||
| if (!compositionRef.current) { | ||
| setCountValue(cutValue); | ||
| } |
There was a problem hiding this comment.
Update triggerChange to reset compositionStartValue to null when composition ends. Note that we also reset it before the early return to ensure the count display is correctly updated even if the change event is skipped to avoid duplicate triggers.
| return; | |
| } | |
| setValue(cutValue); | |
| if (!compositionRef.current) { | |
| setCountValue(cutValue); | |
| } | |
| setCompositionStartValue(null); | |
| return; | |
| } | |
| setValue(cutValue); | |
| if (!compositionRef.current) { | |
| setCompositionStartValue(null); | |
| } |
| onCompositionStart={(e) => { | ||
| compositionRef.current = true; | ||
| setCountValue(formatValue); | ||
| onCompositionStart?.(e); | ||
| }} |
There was a problem hiding this comment.
Set compositionStartValue to the current formatValue when composition starts.
| onCompositionStart={(e) => { | |
| compositionRef.current = true; | |
| setCountValue(formatValue); | |
| onCompositionStart?.(e); | |
| }} | |
| onCompositionStart={(e) => { | |
| compositionRef.current = true; | |
| setCompositionStartValue(formatValue); | |
| onCompositionStart?.(e); | |
| }} |
| // Count-related state should reflect the committed value during IME composition | ||
| // instead of the intermediate phonetic text. | ||
| const [countValue, setCountValue] = React.useState(formatValue); |
There was a problem hiding this comment.
Similarly to Input.tsx, we can derive countValue directly during render by tracking only the compositionStartValue (the value when IME composition starts). This simplifies the state logic, eliminates the useEffect entirely, and avoids potential double-renders or out-of-sync issues.
| // Count-related state should reflect the committed value during IME composition | |
| // instead of the intermediate phonetic text. | |
| const [countValue, setCountValue] = React.useState(formatValue); | |
| // Track the value at the start of IME composition to reflect the committed value | |
| // during composition instead of the intermediate phonetic text. | |
| const [compositionStartValue, setCompositionStartValue] = React.useState<string | null>(null); | |
| const countValue = compositionStartValue !== null ? compositionStartValue : formatValue; |
| useEffect(() => { | ||
| if (!compositionRef.current) { | ||
| setCountValue(formatValue); | ||
| } | ||
| }, [formatValue]); |
There was a problem hiding this comment.
With the derived countValue approach, this useEffect is no longer needed, which prevents unnecessary double-renders when formatValue changes.
| if (!compositionRef.current) { | ||
| setCountValue(cutValue); | ||
| } |
There was a problem hiding this comment.
Update triggerChange to reset compositionStartValue to null when composition ends.
| if (!compositionRef.current) { | |
| setCountValue(cutValue); | |
| } | |
| if (!compositionRef.current) { | |
| setCompositionStartValue(null); | |
| } |
| compositionRef.current = true; | ||
| setCountValue(formatValue); | ||
| onCompositionStart?.(e); |
There was a problem hiding this comment.
Set compositionStartValue to the current formatValue when composition starts.
| compositionRef.current = true; | |
| setCountValue(formatValue); | |
| onCompositionStart?.(e); | |
| compositionRef.current = true; | |
| setCompositionStartValue(formatValue); | |
| onCompositionStart?.(e); |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR updates Input and TextArea character counting so the displayed count reflects the last committed value during IME composition (instead of counting the intermediate phonetic composition text), and adds regression tests for the behavior.
Changes:
- Add a dedicated
countValuestate for count calculations during composition inInputandTextArea. - Update count display hooks to use
countValuerather than the live formatted value. - Add IME composition tests for both
InputandTextArea.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tests/count.test.tsx | Adds regression test ensuring Input count is based on committed value during IME composition. |
| tests/TextArea.count.test.tsx | Adds regression test ensuring TextArea count is based on committed value during IME composition. |
| src/TextArea.tsx | Introduces countValue and updates count calculation to ignore intermediate IME composition text. |
| src/Input.tsx | Introduces countValue and updates count calculation to ignore intermediate IME composition text. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| fireEvent.compositionEnd(input, { currentTarget: { value: '竹' } }); | ||
| fireEvent.input(input, { target: { value: '竹' } }); |
| fireEvent.compositionEnd(textarea, { currentTarget: { value: '竹' } }); | ||
| fireEvent.input(textarea, { target: { value: '竹' } }); |
| useEffect(() => { | ||
| if (!compositionRef.current) { | ||
| setCountValue(formatValue); | ||
| } | ||
| }, [formatValue]); |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/Input.tsx (2)
105-109: ⚡ Quick win建议为 useEffect 添加注释说明其用途。
此 effect 的作用是在非组合输入期间将
countValue同步到formatValue。建议添加简短注释说明为何需要检查compositionRef.current，以便后续维护者理解同步时机。另外，注意到
triggerChange中已经在非组合状态时设置了countValue（第 127 行），此 effect 会在formatValue更新后再次设置相同的值。虽然 React 会批处理并跳过不必要的重渲染，这种冗余的 setState 在功能上无害，但如果未来考虑优化性能，可以考虑去除这种重复。💡 可选的注释示例
+ // Sync countValue with formatValue when not composing. + // During composition, countValue remains frozen at the pre-composition value. useEffect(() => { if (!compositionRef.current) { setCountValue(formatValue); } }, [formatValue]);🤖 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/Input.tsx` around lines 105 - 109, Add a short inline comment above the useEffect that synchronizes countValue to formatValue explaining that this effect ensures countValue follows formatValue when the input is not in composition mode (check compositionRef.current) and therefore avoids clobbering in-progress IME compositions; reference the useEffect block, compositionRef.current, setCountValue, and formatValue so reviewers can find it easily, and optionally note that triggerChange already updates countValue in non-composition cases (see triggerChange) so the effect is mainly a safety sync and could be removed if you want to avoid redundant setState in future optimizations.
63-109: ⚖️ Poor tradeoff可选重构建议：考虑提取共享的计数值管理逻辑。
Input和TextArea中的countValue状态管理逻辑几乎完全相同（状态声明、useEffect 同步、triggerChange 中的条件更新、onCompositionStart 中的冻结）。如果未来需要修改此逻辑（例如添加新的边缘情况处理），需要在两个地方同步修改。建议将此逻辑提取为自定义 Hook（例如
useCommittedCountValue），接受formatValue和compositionRef作为参数，返回countValue和setCountValue。这样可以：
- 消除代码重复
- 确保两个组件的行为保持一致
- 简化未来的维护和测试
不过，这个重构可以在后续 PR 中进行，当前 PR 的实现在功能上是正确的。
🤖 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/Input.tsx` around lines 63 - 109, The countValue state management is duplicated between Input and TextArea; extract the shared logic into a custom hook (e.g., useCommittedCountValue) that accepts formatValue and compositionRef and returns [countValue, setCountValue]; move the useEffect that syncs countValue when compositionRef.current is false, and any updates currently done in triggerChange/onCompositionStart into this hook so both Input and TextArea call useCommittedCountValue(formatValue, compositionRef) to eliminate duplication and keep behavior consistent.src/TextArea.tsx (1)
94-98: ⚡ Quick win建议为 useEffect 添加注释说明其用途。
与
Input.tsx中的相同 effect（第 105-109 行）一样，建议添加注释说明此 effect 的同步逻辑以及为何需要检查组合状态。同样存在triggerChange与 effect 之间的冗余 setState 调用，虽然无害但可作为未来优化点。💡 可选的注释示例
+ // Sync countValue with formatValue when not composing. + // During composition, countValue remains frozen at the pre-composition value. useEffect(() => { if (!compositionRef.current) { setCountValue(formatValue); } }, [formatValue]);🤖 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/TextArea.tsx` around lines 94 - 98, 在位于 TextArea.tsx 中的 useEffect（引用 useEffect、compositionRef.current、setCountValue、formatValue）上方添加一行注释，说明该 effect 的目的：在非组合输入期间将受控的格式化值同步到计数/显示状态（即为什么要检查 compositionRef.current），并指出它与 Input.tsx 中相同逻辑的对应关系；同时在注释中标注 triggerChange 与该 effect 之间存在冗余的 setCountValue 调用并添加一个 TODO/优化提示，方便未来合并或去重该冗余逻辑。
🤖 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.
Nitpick comments:
In `@src/Input.tsx`:
- Around line 105-109: Add a short inline comment above the useEffect that
synchronizes countValue to formatValue explaining that this effect ensures
countValue follows formatValue when the input is not in composition mode (check
compositionRef.current) and therefore avoids clobbering in-progress IME
compositions; reference the useEffect block, compositionRef.current,
setCountValue, and formatValue so reviewers can find it easily, and optionally
note that triggerChange already updates countValue in non-composition cases (see
triggerChange) so the effect is mainly a safety sync and could be removed if you
want to avoid redundant setState in future optimizations.
- Around line 63-109: The countValue state management is duplicated between
Input and TextArea; extract the shared logic into a custom hook (e.g.,
useCommittedCountValue) that accepts formatValue and compositionRef and returns
[countValue, setCountValue]; move the useEffect that syncs countValue when
compositionRef.current is false, and any updates currently done in
triggerChange/onCompositionStart into this hook so both Input and TextArea call
useCommittedCountValue(formatValue, compositionRef) to eliminate duplication and
keep behavior consistent.
In `@src/TextArea.tsx`:
- Around line 94-98: 在位于 TextArea.tsx 中的 useEffect（引用
useEffect、compositionRef.current、setCountValue、formatValue）上方添加一行注释，说明该 effect
的目的：在非组合输入期间将受控的格式化值同步到计数/显示状态（即为什么要检查 compositionRef.current），并指出它与 Input.tsx
中相同逻辑的对应关系；同时在注释中标注 triggerChange 与该 effect 之间存在冗余的 setCountValue 调用并添加一个
TODO/优化提示，方便未来合并或去重该冗余逻辑。
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8d4dca2f-acd0-45ff-943a-097eddd7a014
📒 Files selected for processing (4)
src/Input.tsxsrc/TextArea.tsxtests/TextArea.count.test.tsxtests/count.test.tsx
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #179 +/- ##
==========================================
+ Coverage 98.34% 98.39% +0.05%
==========================================
Files 11 11
Lines 423 437 +14
Branches 136 135 -1
==========================================
+ Hits 416 430 +14
Misses 7 7 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| ); | ||
| // Count-related state should reflect the committed value during IME composition | ||
| // instead of the intermediate phonetic text. | ||
| const [countValue, setCountValue] = useState(formatValue); |
There was a problem hiding this comment.
看了一下原 issue，感觉这并不是 bug。IME 状态下也是要统计字数的，原生的 validity.tooLong 也是按照输入长度来的。
我把结论 ref 过去，这个不用修。
There was a problem hiding this comment.
附 Codesandbox 原生的 validity： https://codesandbox.io/p/sandbox/55fv42?file=%2Findex.html%3A276%2C35-276%2C51
Summary
This change makes
showCountderive from a committed count value during IME composition instead of the intermediate phonetic text.Related Issue
Root Cause
InputandTextAreaupdated their count display from the live input value on every change event. During CJK IME composition, that meantshowCountcounted intermediate pinyin text likezhuinstead of the committed character like竹.Changes
InputTextAreashowCountand count-based out-of-range state aligned with the committed value during IME compositionInputandTextAreaVerification
pnpm test -- --runInBand tests/count.test.tsx tests/TextArea.count.test.tsxpnpm test -- --runInBandSummary by CodeRabbit
发布说明
Bug Fixes
Tests