fix(input): handle Chinese IME composition correctly #10967
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughEnhances the Input component with local controlled-like state management initialized from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 error, 2 warnings)
✅ Passed checks (4 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/frontend/src/components/ui/input.tsx (1)
35-41: Ref merging works but types andinputRefusage can be tightenedThe merged ref logic is functionally fine, but a couple of small improvements would make it more idiomatic and typesafe:
mergedRefshould acceptHTMLInputElement | null, because React calls ref callbacks withnullon unmount.inputRefcurrently isn’t used outside the merge; if you don’t need it yet, you can drop it to reduce noise. If you plan to use it, prefer typing it asHTMLInputElement | null.For example:
- const inputRef = React.useRef<HTMLInputElement>(null); + const inputRef = React.useRef<HTMLInputElement | null>(null); @@ - const mergedRef = (node: HTMLInputElement) => { - inputRef.current = node; + const mergedRef = (node: HTMLInputElement | null) => { + inputRef.current = node; if (typeof ref === "function") ref(node); else if (ref) (ref as any).current = node; };If
inputRefisn’t needed yet, you can also simplify<input ref={mergedRef}>to use a smallmergeRefshelper or inline callback when you introduce a second consumer.Also applies to: 88-88
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/frontend/src/components/ui/input.tsx(3 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
src/frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/frontend_development.mdc)
src/frontend/src/**/*.{ts,tsx}: Use React 18 with TypeScript for frontend development
Use Zustand for state management
Files:
src/frontend/src/components/ui/input.tsx
src/frontend/src/**/*.{tsx,jsx,css,scss}
📄 CodeRabbit inference engine (.cursor/rules/frontend_development.mdc)
Use Tailwind CSS for styling
Files:
src/frontend/src/components/ui/input.tsx
src/frontend/src/components/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/frontend_development.mdc)
src/frontend/src/components/**/*.{tsx,jsx}: Use React Flow for flow graph visualization with Node, Edge, Controls, and Background components
Use the cn() utility function for combining Tailwind CSS classes in components
Use TypeScript interfaces for defining component props in React components
Files:
src/frontend/src/components/ui/input.tsx
src/frontend/src/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/frontend_development.mdc)
src/frontend/src/**/*.{tsx,jsx}: Implement dark mode support using the useDarkMode hook and dark store
Use Lucide React for icon components in the application
Files:
src/frontend/src/components/ui/input.tsx
🧠 Learnings (4)
📚 Learning: 2025-11-24T19:46:57.920Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: .cursor/rules/icons.mdc:0-0
Timestamp: 2025-11-24T19:46:57.920Z
Learning: Applies to src/frontend/src/icons/*/index.tsx : Create an `index.tsx` for each icon that exports the icon using `forwardRef` and properly types it as `React.ForwardRefExoticComponent<React.PropsWithChildren<{}> & React.RefAttributes<SVGSVGElement>>`.
Applied to files:
src/frontend/src/components/ui/input.tsx
📚 Learning: 2025-06-16T11:14:04.200Z
Learnt from: dolfim-ibm
Repo: langflow-ai/langflow PR: 8394
File: src/frontend/src/icons/Docling/index.tsx:4-6
Timestamp: 2025-06-16T11:14:04.200Z
Learning: The Langflow codebase consistently uses `React.PropsWithChildren<{}>` as the prop type for all icon components using forwardRef, rather than `React.SVGProps<SVGSVGElement>`. This is an established pattern across hundreds of icon files in src/frontend/src/icons/.
Applied to files:
src/frontend/src/components/ui/input.tsx
📚 Learning: 2025-06-23T12:46:52.420Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: .cursor/rules/icons.mdc:0-0
Timestamp: 2025-06-23T12:46:52.420Z
Learning: Export custom icon components in React using React.forwardRef to ensure proper ref forwarding and compatibility with parent components.
Applied to files:
src/frontend/src/components/ui/input.tsx
📚 Learning: 2025-06-23T12:46:42.048Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: .cursor/rules/frontend_development.mdc:0-0
Timestamp: 2025-06-23T12:46:42.048Z
Learning: Custom React Flow node types should be implemented as memoized components, using Handle components for connection points and supporting optional icons and labels.
Applied to files:
src/frontend/src/components/ui/input.tsx
🧬 Code graph analysis (1)
src/frontend/src/components/ui/input.tsx (2)
src/frontend/src/utils/utils.ts (1)
cn(38-40)src/frontend/tailwind.config.mjs (1)
className(476-476)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 16/17
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 17/17
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 15/17
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 14/17
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 12/17
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 9/17
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 1/17
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 13/17
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 11/17
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 3/17
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 10/17
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 8/17
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 7/17
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 2/17
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 5/17
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 6/17
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 4/17
- GitHub Check: Test Docker Images / Test docker images
- GitHub Check: Test Starter Templates
| onChange, | ||
| value, | ||
| defaultValue, | ||
| ...props |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd /tmp && find . -name "input.tsx" -path "*/src/frontend/src/components/ui/*" 2>/dev/null | head -5Repository: langflow-ai/langflow
Length of output: 46
🏁 Script executed:
git ls-files | grep -E "src/frontend/src/components/ui/input\.tsx"Repository: langflow-ai/langflow
Length of output: 104
🏁 Script executed:
cat -n src/frontend/src/components/ui/input.tsx | head -120Repository: langflow-ai/langflow
Length of output: 4062
🏁 Script executed:
rg -A 3 -B 1 "<Input\s+" src/frontend/src --type tsx --type jsx | head -100Repository: langflow-ai/langflow
Length of output: 91
🏁 Script executed:
rg 'value\s*=' src/frontend/src --type tsx --type jsx -A 1 | grep -A 1 '<Input' | head -50Repository: langflow-ai/langflow
Length of output: 91
🏁 Script executed:
rg "import.*Input.*from.*input" src/frontend/src | head -20Repository: langflow-ai/langflow
Length of output: 2625
🏁 Script executed:
rg "<Input" src/frontend/src --type ts --type jsx | head -30Repository: langflow-ai/langflow
Length of output: 91
🏁 Script executed:
find src/frontend/src -name "*.tsx" -o -name "*.jsx" | xargs grep -l "<Input" | head -10Repository: langflow-ai/langflow
Length of output: 1663
🏁 Script executed:
grep -A 5 "<Input" src/frontend/src/modals/secretKeyModal/components/form-key-render.tsx | grep -E "(value|onChange)" | head -10Repository: langflow-ai/langflow
Length of output: 181
🏁 Script executed:
grep -B 2 -A 5 "<Input" src/frontend/src/pages/MainPage/components/header/index.tsx | head -30Repository: langflow-ai/langflow
Length of output: 405
🏁 Script executed:
grep -B 2 -A 5 "<Input" src/frontend/src/pages/MainPage/components/inputSearchComponent/index.tsx | head -30Repository: langflow-ai/langflow
Length of output: 351
🏁 Script executed:
cat -n src/frontend/src/modals/secretKeyModal/components/form-key-render.tsx | head -80Repository: langflow-ai/langflow
Length of output: 1435
🏁 Script executed:
rg "onCompositionStart|onCompositionEnd" src/frontend/src --no-heading | head -10Repository: langflow-ai/langflow
Length of output: 249
Controlled input behavior is broken by value/defaultValue handling
The destructuring of value and defaultValue (lines 24-27) removes them from ...props, and the <input> uses defaultValue={currentValue} instead of value={currentValue}. This makes the input effectively uncontrolled, breaking existing controlled usages:
- In
src/frontend/src/modals/secretKeyModal/components/form-key-render.tsx(and likely other places), the component is used as<Input value={apiKeyName} onChange={...} />, a controlled pattern. - When
apiKeyNamechanges after mount, the input will not update because thevalueprop is ignored after initialization. - Parent-driven resets or form state changes won't be reflected.
Fix: Use value={currentValue} instead of defaultValue={currentValue} and add a useEffect to sync value when not composing:
- defaultValue={currentValue}
+ value={currentValue}Also add after line 43:
+ React.useEffect(() => {
+ if (!isComposing && value !== undefined) {
+ setCurrentValue(String(value));
+ }
+ }, [isComposing, value]);This preserves uncontrolled usage via defaultValue (initialization only) and controlled usage via value (synced after mount), while IME behavior remains gated by isComposing.
Missing handler chaining for composition events (lines 89-90)
onCompositionStart and onCompositionEnd override any user-supplied handlers without calling them. Chain them:
- const handleCompositionStart = () => setIsComposing(true);
+ const handleCompositionStart = (e: React.CompositionEvent<HTMLInputElement>) => {
+ setIsComposing(true);
+ props.onCompositionStart?.(e);
+ };
- const handleCompositionEnd = (e: React.CompositionEvent<HTMLInputElement>) => {
+ const handleCompositionEnd = (e: React.CompositionEvent<HTMLInputElement>) => {
setIsComposing(false);
const newValue = e.currentTarget.value;
setCurrentValue(newValue);
+ props.onCompositionEnd?.(e);
onChange?.(e as any);
};Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/frontend/src/components/ui/input.tsx lines 24-27, 43, and 89-90: the
component currently destructures value/defaultValue out of props and renders
<input defaultValue={currentValue}>, which breaks controlled inputs; change the
input to use value={currentValue} so controlled props update, add a useEffect
after line 43 that syncs internal currentValue from the value prop when not
composing (so defaultValue still works for uncontrolled initialization), and
update onCompositionStart/onCompositionEnd handlers (lines 89-90) to call any
user-supplied handlers in props (chain them) before/after toggling composing
state to avoid swallowing consumers' handlers.
| const [isComposing, setIsComposing] = React.useState(false); | ||
|
|
||
| const handleCompositionStart = () => setIsComposing(true); | ||
|
|
||
| const handleCompositionEnd = ( | ||
| e: React.CompositionEvent<HTMLInputElement>, | ||
| ) => { | ||
| setIsComposing(false); | ||
| const newValue = e.currentTarget.value; | ||
| setCurrentValue(newValue); | ||
| onChange?.(e as any); | ||
| }; | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
wc -l src/frontend/src/components/ui/input.tsxRepository: langflow-ai/langflow
Length of output: 108
🏁 Script executed:
cat -n src/frontend/src/components/ui/input.tsxRepository: langflow-ai/langflow
Length of output: 4087
IME composition handlers double‑call onChange and swallow user onComposition* props
Two confirmed issues in the composition logic:
-
handleCompositionEnd(line 53) callsonChange?.(e as any)with aCompositionEvent, but React then fires a finalonChange(ChangeEvent) when composition commits. SinceisComposingis set tofalseat line 50,handleChangewill execute and callonChangeagain at line 60. This results inonChangebeing invoked twice per committed IME input. -
The
InputPropsinterface does not includeonCompositionStartoronCompositionEnd. The handlers at lines 45 and 47–54 are hardcoded and do not forward to any user-supplied composition callbacks fromprops, effectively swallowing them if a parent component attempts to pass them.
Fix by:
- Removing the
onChangecall fromhandleCompositionEnd(line 53). - Adding
onCompositionStartandonCompositionEndto the props destructuring and forwarding them in the composition handlers. - Guarding
handleChangewith bothisComposingstate and the nativee.nativeEvent.isComposingflag for cross-browser safety.
This ensures onChange fires exactly once per committed value and preserves any consumer-supplied composition handlers.
Also applies to: 56–62, 89–91
🤖 Prompt for AI Agents
In src/frontend/src/components/ui/input.tsx around lines 43 to 55 (also apply
same changes to ranges 56–62 and 89–91): the composition handlers currently call
onChange with a CompositionEvent and do not forward user-supplied
onCompositionStart/onCompositionEnd props, causing double onChange and swallowed
callbacks; remove the onChange call from handleCompositionEnd, add
onCompositionStart and onCompositionEnd to the component props destructuring,
forward those props from the internal handlers (call user callbacks if
provided), and guard the main handleChange so it returns early when isComposing
is true OR e.nativeEvent?.isComposing is true to avoid duplicate onChange firing
across browsers.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/frontend/src/components/ui/input.tsx (1)
35-41:inputRefis unused; consider removing or wiring it where needed
inputRefis set inmergedRef(Lines 35–41) but never read anywhere in the component. This adds indirection without benefit and slightly complicates the ref logic.Either:
- Remove
inputRefand just forwardref:- const inputRef = React.useRef<HTMLInputElement>(null); - - const mergedRef = (node: HTMLInputElement) => { - inputRef.current = node; - if (typeof ref === "function") ref(node); - else if (ref) (ref as any).current = node; - }; + const mergedRef = (node: HTMLInputElement | null) => { + if (typeof ref === "function") ref(node); + else if (ref) (ref as React.MutableRefObject<HTMLInputElement | null>).current = node; + };or:
- Actually use
inputReffor some behavior (e.g., selection management) so the extra ref has a clear purpose.Given this is a small UI component, I’d lean toward removing unused state to keep it simple.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/frontend/src/components/ui/input.tsx(3 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
src/frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/frontend_development.mdc)
src/frontend/src/**/*.{ts,tsx}: Use React 18 with TypeScript for frontend development
Use Zustand for state management
Files:
src/frontend/src/components/ui/input.tsx
src/frontend/src/**/*.{tsx,jsx,css,scss}
📄 CodeRabbit inference engine (.cursor/rules/frontend_development.mdc)
Use Tailwind CSS for styling
Files:
src/frontend/src/components/ui/input.tsx
src/frontend/src/components/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/frontend_development.mdc)
src/frontend/src/components/**/*.{tsx,jsx}: Use React Flow for flow graph visualization with Node, Edge, Controls, and Background components
Use the cn() utility function for combining Tailwind CSS classes in components
Use TypeScript interfaces for defining component props in React components
Files:
src/frontend/src/components/ui/input.tsx
src/frontend/src/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/frontend_development.mdc)
src/frontend/src/**/*.{tsx,jsx}: Implement dark mode support using the useDarkMode hook and dark store
Use Lucide React for icon components in the application
Files:
src/frontend/src/components/ui/input.tsx
🧠 Learnings (5)
📚 Learning: 2025-11-24T19:46:45.790Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: .cursor/rules/frontend_development.mdc:0-0
Timestamp: 2025-11-24T19:46:45.790Z
Learning: Applies to src/frontend/src/components/**/*.{tsx,jsx} : Use TypeScript interfaces for defining component props in React components
Applied to files:
src/frontend/src/components/ui/input.tsx
📚 Learning: 2025-11-24T19:46:57.920Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: .cursor/rules/icons.mdc:0-0
Timestamp: 2025-11-24T19:46:57.920Z
Learning: Applies to src/frontend/src/icons/*/index.tsx : Create an `index.tsx` for each icon that exports the icon using `forwardRef` and properly types it as `React.ForwardRefExoticComponent<React.PropsWithChildren<{}> & React.RefAttributes<SVGSVGElement>>`.
Applied to files:
src/frontend/src/components/ui/input.tsx
📚 Learning: 2025-06-16T11:14:04.200Z
Learnt from: dolfim-ibm
Repo: langflow-ai/langflow PR: 8394
File: src/frontend/src/icons/Docling/index.tsx:4-6
Timestamp: 2025-06-16T11:14:04.200Z
Learning: The Langflow codebase consistently uses `React.PropsWithChildren<{}>` as the prop type for all icon components using forwardRef, rather than `React.SVGProps<SVGSVGElement>`. This is an established pattern across hundreds of icon files in src/frontend/src/icons/.
Applied to files:
src/frontend/src/components/ui/input.tsx
📚 Learning: 2025-06-23T12:46:52.420Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: .cursor/rules/icons.mdc:0-0
Timestamp: 2025-06-23T12:46:52.420Z
Learning: Export custom icon components in React using React.forwardRef to ensure proper ref forwarding and compatibility with parent components.
Applied to files:
src/frontend/src/components/ui/input.tsx
📚 Learning: 2025-06-23T12:46:42.048Z
Learnt from: CR
Repo: langflow-ai/langflow PR: 0
File: .cursor/rules/frontend_development.mdc:0-0
Timestamp: 2025-06-23T12:46:42.048Z
Learning: Custom React Flow node types should be implemented as memoized components, using Handle components for connection points and supporting optional icons and labels.
Applied to files:
src/frontend/src/components/ui/input.tsx
🧬 Code graph analysis (1)
src/frontend/src/components/ui/input.tsx (2)
src/frontend/src/utils/utils.ts (1)
cn(38-40)src/frontend/tailwind.config.mjs (1)
className(476-476)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 16/17
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 12/17
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 1/17
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 13/17
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 11/17
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 3/17
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 10/17
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 8/17
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 7/17
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 2/17
- GitHub Check: Run Frontend Tests / Playwright Tests - Shard 5/17
- GitHub Check: Test Docker Images / Test docker images
🔇 Additional comments (3)
src/frontend/src/components/ui/input.tsx (3)
95-100: Placeholder visibility tied tocurrentValueis fine but depends on correct value syncingThe updated placeholder logic (Lines 95–100) using
currentValue ? "hidden" : ""is consistent with the “floating placeholder” pattern and the Tailwind classes look aligned with existing styles.However, its correctness depends on
currentValueaccurately reflecting the actual input value (see earlier comment on syncing fromvalue/ usingvalue={currentValue}). Once that is addressed, this part looks good.
118-119: AddingInput.displayNameis a good improvement for debugging
Input.displayName = "Input";(Lines 118–119) helps with React DevTools readability and is aligned with common forwardRef patterns. No issues here.
5-12: This comment is incorrect—InputProps does not redeclarevalue,defaultValue, oronChangeLines 24–27 are function parameter destructuring, not interface property redeclaration.
InputPropsextendsReact.InputHTMLAttributes<HTMLInputElement>without redefining these properties. The component's handling ofonChange,value, anddefaultValue(extracting them and managing internal state with composition event handling) is intentional custom behavior, not a problematic override.Likely an incorrect or invalid review comment.
| onChange, | ||
| value, | ||
| defaultValue, | ||
| ...props |
There was a problem hiding this comment.
Controlled vs uncontrolled behavior and value syncing look inconsistent and may break existing usages
Right now the component:
- Initializes
currentValuefromdefaultValue ?? value(Line 31–33), - Uses
defaultValue={currentValue}on the<input>(Line 81), - Updates
currentValueonly fromhandleChange/handleCompositionEnd(Lines 56–62), - Never re-syncs
currentValuewhen thevalueprop changes after mount.
As a result:
- Existing controlled usages like
<Input value={state} onChange={...} />will no longer behave as controlled: updates tovaluefrom the parent are ignored after the first render. - The visible text in the input can get out of sync with
currentValueand the externalvalueprop, while the placeholder visibility (Lines 95–100) is driven bycurrentValue, not the actual DOM value. - Consumers passing
defaultValuealso can’t programmatically clear/reset via props.
To keep IME fixes while preserving expected semantics, consider:
- Using
value={currentValue}instead ofdefaultValue={currentValue}, and - Adding an effect that syncs
currentValuefrom the externalvalueprop when it’s defined and not currently composing, e.g.:
- const [currentValue, setCurrentValue] = React.useState(() =>
- String(defaultValue ?? value ?? ""),
- );
+ const [currentValue, setCurrentValue] = React.useState(() =>
+ String(defaultValue ?? value ?? ""),
+ );
+
+ React.useEffect(() => {
+ if (value !== undefined) {
+ setCurrentValue(String(value));
+ }
+ }, [value]);and then:
- <input
- ...
- defaultValue={currentValue}
+ <input
+ ...
+ value={currentValue}You can still gate parent onChange calls during composition (see next comment) without sacrificing controlled behavior.
Also applies to: 31-33, 56-62, 78-82, 95-100
🤖 Prompt for AI Agents
In src/frontend/src/components/ui/input.tsx around lines 24–27, 31–33, 56–62,
78–82 and 95–100: the component currently initializes internal currentValue from
defaultValue ?? value, writes that to defaultValue on the <input>, and only
updates currentValue from internal handlers, so external controlled value
updates are ignored after mount. Change to use value={currentValue} on the input
(not defaultValue), initialize currentValue from value ?? defaultValue, and add
a useEffect that re-syncs currentValue whenever the external value prop changes
(but only when not composing) so controlled usages remain in sync; continue to
gate calling parent onChange during composition but still update internal state
on composition-aware changes.
| const [isComposing, setIsComposing] = React.useState(false); | ||
|
|
||
| const handleCompositionStart = () => setIsComposing(true); | ||
|
|
||
| const handleCompositionEnd = ( | ||
| e: React.CompositionEvent<HTMLInputElement>, | ||
| ) => { | ||
| setIsComposing(false); | ||
| const newValue = e.currentTarget.value; | ||
| setCurrentValue(newValue); | ||
| onChange?.(e as any); | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/frontend/src/components/ui/input.tsx | head -100Repository: langflow-ai/langflow
Length of output: 3510
🌐 Web query:
React 18 compositionend onChange event sequence IME input handling best practices
💡 Result:
Short best-practices for handling IME (composition) + onChange in React 18
-
Understand event sequencing: IME uses compositionstart/compositionupdate/compositionend; browsers differ in whether input/onChange fires during composition — handle composition events instead of assuming onChange means “final” text. [1][2]
-
Track composition state and ignore intermediate onChange updates:
- Set a boolean isComposing on compositionstart -> true, compositionend -> false.
- In onChange, if isComposing === true, skip heavy work (validation, filtering, API calls, state that would reset the input caret) and only store transient value if needed.
- On compositionend, apply final update/validation using event.target.value. This avoids premature resets/extra renders. [1][3]
-
Keep inputs consistently controlled (never flip value between undefined/null and string):
- Initialize value with '' and never set undefined/null; switching controlled↔uncontrolled causes weird behaviors with composition. [4]
-
Edge cases & platform differences:
- Some IMEs (mobile keyboards, certain OS IMEs) emit input/onInput before/without composition events; combine checks (composition flag + final input/compositionend) for robust handling.
- For numeric/filtered inputs, validate on compositionend or debounce validation to avoid discarding composing text. [1][5]
-
Implementation patterns (concise code sketch)
- useRef for isComposing; attach onCompositionStart/Update/End and onChange.
- In onChange: if (isComposingRef.current) { /* optionally cache */ return; } else setState(...)
- In onCompositionEnd: setState(e.target.value) and run validation/side-effects. [1][3]
-
Alternatives when composition problems persist:
- Use uncontrolled input (defaultValue) and read value on blur/submit.
- Use libraries/components built to handle composition (or small wrappers that buffer changes until compositionend). [6]
References
- React discussion / issues on composition & controlled inputs (recommendation to use composition events). [1]
- MDN docs for compositionstart/compositionend/compositionupdate. [2]
- Recent React issue reporting IME-controlled-input resets in React 18 (example of platform/implementation differences). [3]
- Controlled vs uncontrolled best-practices (always initialize to ''). [4]
- Article/notes about cross-IME differences and strategies (debounce, validate on compositionend). [5]
- Example npm packages / wrappers that buffer on compositionend. [6]
If you want, I can paste a minimal React 18 code example implementing the isComposing pattern (with TypeScript or plain JS).
🏁 Script executed:
rg "onCompositionStart|onCompositionEnd" src/frontend/src/components --type tsx --type ts -A 5 -B 5Repository: langflow-ai/langflow
Length of output: 91
🌐 Web query:
React 18 double onChange firing IME composition event browser behavior canonical pattern
💡 Result:
Short answer
When typing with an IME the browser emits composition events plus input events (and different browsers/OSes emit them in different orders). React’s onChange is a wrapper around the native input/onInput behavior, so you can see “double” updates (one during composition/update and one when composition finishes). The canonical solution is to track composition state with onCompositionStart/onCompositionEnd and ignore onChange while composing, then apply the final value at compositionend. (developer.mozilla.org)
Minimal example (canonical pattern)
- maintain an isComposing flag (ref/state)
- ignore onChange updates while composing; handle final update in onCompositionEnd
Example:
const isComposing = useRef(false);
const [value, setValue] = useState('');
<input
value={value}
onCompositionStart={() => (isComposing.current = true)}
onCompositionEnd={(e) => {
isComposing.current = false;
setValue(e.currentTarget.value); // apply final composed text
}}
onChange={(e) => {
if (!isComposing.current) setValue(e.currentTarget.value);
}}
/>
Notes and gotchas
- Browser behavior varies (some emit input/keyup after compositionend, some don’t) — testing across Chrome/Safari/Firefox/iOS is needed. (stum.de)
- Alternatives: use onBeforeInput/onInput carefully, or use uncontrolled inputs and read value on blur/submit if re-renders are the issue. (reactspectrum.blob.core.windows.net)
Sources
- MDN: compositionstart / compositionend / CompositionEvent (composition event docs). [MDN]. (developer.mozilla.org)
- React events (composition events listed; onChange maps to input behavior). [React docs]. (gaearon.github.io)
- Practical notes on cross-browser IME differences (examples & quirks). (stum.de)
If you want, I can paste a ready-to-drop hook (useComposition-aware onChange) you can copy into your app.
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/API/Element/compositionend_event?utm_source=openai
- 2: https://www.stum.de/2016/06/24/handling-ime-events-in-javascript/?utm_source=openai
- 3: https://reactspectrum.blob.core.windows.net/reactspectrum/7b8c56678028193bb1bcf6e621aade9851289351/docs/react-spectrum/TextArea.html?utm_source=openai
- 4: https://developer.mozilla.org/en-US/docs/Web/API/Element/compositionstart_event?utm_source=openai
- 5: https://gaearon.github.io/react/docs/events.html?utm_source=openai
- 6: https://www.stum.de/2016/06/24/handling-ime-events-in-javascript/?utm_source=openai
🏁 Script executed:
rg "onCompositionStart|onCompositionEnd" src/frontend/src/components -A 5 -B 5 -t tsRepository: langflow-ai/langflow
Length of output: 885
Remove manual onChange call from handleCompositionEnd to prevent double-firing
The composition event handler fires onChange manually (line 53), but React will emit a natural onChange event after compositionend. This results in two onChange invocations for the final committed text, which can trigger duplicate validation, API calls, or other side-effects.
Per React 18 canonical patterns for IME handling: update state in onCompositionEnd but let the browser's natural onChange propagate through handleChange (which now correctly checks !isComposing). Remove the manual onChange?.(e as any) call at line 53.
Additionally, onCompositionStart and onCompositionEnd are not destructured from props (line 24–27), so if a caller passes these handlers via props, they will override your IME logic. Consider destructuring and merging them:
const Input = React.forwardRef<HTMLInputElement, InputProps>(
({
className,
inputClassName,
icon = "",
endIcon = "",
endIconClassName = "",
type,
placeholder,
onChange,
+ onCompositionStart: userOnCompositionStart,
+ onCompositionEnd: userOnCompositionEnd,
value,
defaultValue,
...props
}, ref) => {
...
- const handleCompositionStart = () => setIsComposing(true);
+ const handleCompositionStart: React.CompositionEventHandler<HTMLInputElement> = (e) => {
+ setIsComposing(true);
+ userOnCompositionStart?.(e);
+ };
const handleCompositionEnd = (
e: React.CompositionEvent<HTMLInputElement>,
) => {
setIsComposing(false);
const newValue = e.currentTarget.value;
setCurrentValue(newValue);
- onChange?.(e as any);
+ userOnCompositionEnd?.(e);
};Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/frontend/src/components/ui/input.tsx around lines 43 to 54 (and note
props area around lines 24–27), remove the manual call to onChange in
handleCompositionEnd so the browser/React natural onChange after compositionend
is the only change event; keep setting isComposing to false and updating local
state there. Also destructure onCompositionStart and onCompositionEnd from props
and, in your internal handlers, invoke the caller-provided handlers (if any)
after performing your isComposing state updates so external handlers can run
without overriding the IME logic.
748561321
force-pushed
the
fix/chinese-ime-input
branch
from
d7249ae to
2db2176
Compare
748561321
changed the title
github-actions
bot
added
bug
|
This PR only touches the Input component (IME / composition handling) and does not modify canvas interactions, flow logic, selectors, or code paths covered by Playwright component tests. The failing job appears to be: I retriggered CI with an empty commit, and the same timeout still occurred. If it makes sense from your side, could you please try rerunning this job? |
github-actions
bot
added
bug
1 participant
Add basic IME support for Input component
This PR tries to improve Chinese (IME) input in Langflow's Input component. The main issue was that typing Chinese caused the cursor to jump or input to be lost, because React updates the input value on every
onChangeevent without handling composition.What was changed
onCompositionStartandonCompositionEndevent handlers.isComposingflag to track if the user is currently composing text.onChangeupdates are temporarily ignored, preventing cursor jumps.This is a quick, initial fix. Tested mostly in Chrome; may need further testing in other browsers or with different input methods.
Open to feedback and improvements.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.