Skip to content

Commit 428c817

Browse files
ORAZIO SPADAafc163
authored andcommitted
fix: resolve react-number-format compatibility (#46999) and prevent double-firing (#46587)
- Replace cloneEvent (which uses cloneNode creating detached DOM nodes) with createNormalizedEvent that sets target.value directly and points target/currentTarget at the real mounted input element. This fixes document.contains(e.target) returning false for third-party wrappers like react-number-format. - In triggerChange, use compositionEnd early-return with setValue to prevent double-firing of onChange. The browser's subsequent input event will handle the onChange call naturally. - Add regression tests for onChange event target being the real mounted element across clear, normal typing, and exceedFormatter scenarios.
1 parent 0215da5 commit 428c817

3 files changed

Lines changed: 86 additions & 53 deletions

File tree

src/Input.tsx

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -112,11 +112,15 @@ const Input = forwardRef<InputRef, InputProps>((props, ref) => {
112112
) => {
113113
const cutValue = getExceedValue(currentValue, compositionRef.current);
114114

115-
if (info.source === 'compositionEnd' && currentValue === cutValue) {
116-
// Avoid triggering twice
117-
// https://github.com/ant-design/ant-design/issues/46587
115+
if (info.source === 'compositionEnd') {
116+
// Always update internal state on compositionEnd, but skip onChange.
117+
// The browser fires an input/change event after compositionEnd, which
118+
// will call onInternalChange → triggerChange again and trigger onChange.
119+
// Skipping here prevents double-firing (#46587).
120+
setValue(cutValue);
118121
return;
119122
}
123+
120124
setValue(cutValue);
121125

122126
if (inputRef.current) {

src/utils/commonUtils.ts

Lines changed: 20 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -9,40 +9,19 @@ export function hasPrefixSuffix(props: BaseInputProps | InputProps) {
99
return !!(props.prefix || props.suffix || props.allowClear);
1010
}
1111

12-
// TODO: It's better to use `Proxy` replace the `element.value`. But we still need support IE11.
13-
function cloneEvent<
12+
// Create a normalized event that points target/currentTarget at the real mounted
13+
// element instead of a detached cloneNode. This keeps document.contains(e.target)
14+
// returning true, which third-party wrappers like react-number-format rely on.
15+
function createNormalizedEvent<
1416
EventType extends React.SyntheticEvent<any, any>,
1517
Element extends HTMLInputElement | HTMLTextAreaElement,
1618
>(event: EventType, target: Element, value: any): EventType {
17-
// A bug report filed on WebKit's Bugzilla tracker, dating back to 2009, specifically addresses the issue of cloneNode() not copying files of <input type="file"> elements.
18-
// As of the last update, this bug was still marked as "NEW," indicating that it might not have been resolved yet​​.
19-
// https://bugs.webkit.org/show_bug.cgi?id=28123
20-
const currentTarget = target.cloneNode(true) as Element;
19+
target.value = value;
2120

22-
// click clear icon
23-
const newEvent = Object.create(event, {
24-
target: { value: currentTarget },
25-
currentTarget: { value: currentTarget },
26-
});
27-
28-
// Fill data
29-
currentTarget.value = value;
30-
31-
// Fill selection. Some type like `email` not support selection
32-
// https://github.com/ant-design/ant-design/issues/47833
33-
if (
34-
typeof target.selectionStart === 'number' &&
35-
typeof target.selectionEnd === 'number'
36-
) {
37-
currentTarget.selectionStart = target.selectionStart;
38-
currentTarget.selectionEnd = target.selectionEnd;
39-
}
40-
41-
currentTarget.setSelectionRange = (...args) => {
42-
target.setSelectionRange(...args);
43-
};
44-
45-
return newEvent;
21+
return Object.create(event, {
22+
target: { value: target, enumerable: true, configurable: true },
23+
currentTarget: { value: target, enumerable: true, configurable: true },
24+
}) as EventType;
4625
}
4726

4827
export function resolveOnChange<
@@ -59,34 +38,25 @@ export function resolveOnChange<
5938
if (!onChange) {
6039
return;
6140
}
62-
let event = e;
6341

6442
if (e.type === 'click') {
65-
// Clone a new target for event.
66-
// Avoid the following usage, the setQuery method gets the original value.
67-
//
68-
// const [query, setQuery] = React.useState('');
69-
// <Input
70-
// allowClear
71-
// value={query}
72-
// onChange={(e)=> {
73-
// setQuery((prevStatus) => e.target.value);
74-
// }}
75-
// />
76-
77-
event = cloneEvent(e, target, '');
78-
79-
onChange(event as React.ChangeEvent<E>);
43+
// When clearing, the click event's native target is the clear icon, not the
44+
// input. We set target.value = '' so that e.target.value reads as the
45+
// cleared value, then redirect target/currentTarget back to the real input.
46+
onChange(createNormalizedEvent(e, target, '') as React.ChangeEvent<E>);
8047
return;
8148
}
8249

83-
// Trigger by composition event, this means we need force change the input value
50+
// Trigger by composition event or exceedFormatter, this means we need force
51+
// change the event target value
8452
// https://github.com/ant-design/ant-design/issues/45737
8553
// https://github.com/ant-design/ant-design/issues/46598
8654
if (target.type !== 'file' && targetValue !== undefined) {
87-
event = cloneEvent(e, target, targetValue);
88-
onChange(event as React.ChangeEvent<E>);
55+
onChange(
56+
createNormalizedEvent(e, target, targetValue) as React.ChangeEvent<E>,
57+
);
8958
return;
9059
}
91-
onChange(event as React.ChangeEvent<E>);
60+
61+
onChange(e as React.ChangeEvent<E>);
9262
}

tests/index.test.tsx

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -561,3 +561,62 @@ describe('Input IME behavior', () => {
561561
expect(onPressEnter).toHaveBeenCalled();
562562
});
563563
});
564+
565+
describe('onChange event target', () => {
566+
// https://github.com/ant-design/ant-design/issues/46999
567+
it('onChange target should be the real mounted input (not a detached clone) on clear', () => {
568+
const onChange = jest.fn();
569+
const { container } = render(
570+
<Input
571+
prefixCls="rc-input"
572+
allowClear
573+
defaultValue="hello"
574+
onChange={onChange}
575+
/>,
576+
);
577+
const input = container.querySelector('input')!;
578+
579+
fireEvent.click(container.querySelector('.rc-input-clear-icon')!);
580+
581+
expect(onChange).toHaveBeenCalled();
582+
const event = onChange.mock.calls[0][0];
583+
expect(event.target).toBe(input);
584+
expect(event.currentTarget).toBe(input);
585+
expect(document.contains(event.target)).toBe(true);
586+
expect(event.target.value).toBe('');
587+
});
588+
589+
it('onChange target should be the real mounted input for normal typing', () => {
590+
const onChange = jest.fn();
591+
const { container } = render(<Input onChange={onChange} />);
592+
const input = container.querySelector('input')!;
593+
594+
fireEvent.change(input, { target: { value: 'test' } });
595+
596+
expect(onChange).toHaveBeenCalled();
597+
const event = onChange.mock.calls[0][0];
598+
expect(event.target).toBe(input);
599+
expect(document.contains(event.target)).toBe(true);
600+
});
601+
602+
it('onChange target should be the real mounted input when exceedFormatter is active', () => {
603+
const onChange = jest.fn();
604+
const { container } = render(
605+
<Input
606+
count={{ max: 3, exceedFormatter: (val) => val.slice(0, 3) }}
607+
onChange={onChange}
608+
/>,
609+
);
610+
const input = container.querySelector('input')!;
611+
612+
fireEvent.change(input, { target: { value: 'abcdef' } });
613+
614+
expect(onChange).toHaveBeenCalled();
615+
const event = onChange.mock.calls[0][0];
616+
expect(event.target).toBe(input);
617+
expect(event.currentTarget).toBe(input);
618+
expect(document.contains(event.target)).toBe(true);
619+
// exceedFormatter should trim the value
620+
expect(event.target.value).toBe('abc');
621+
});
622+
});

0 commit comments

Comments
 (0)