[core] fix(InputGroup): async controlled updates (#4266)

InputGroup now uses a new private component called AsyncControllableInput which works around some problems in React dealing with async controlled updates to text inputs which arise during IME composition.

packages/core/src/components/forms/asyncControllableInput.tsx

@@ -0,0 +1,110 @@
+/* !
+ * Copyright 2020 Palantir Technologies, Inc. All rights reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import * as React from "react";
+import { polyfill } from "react-lifecycles-compat";
+
+export interface IAsyncControllableInputProps
+    extends React.DetailedHTMLProps<React.InputHTMLAttributes<HTMLInputElement>, HTMLInputElement> {
+    inputRef?: React.LegacyRef<HTMLInputElement>;
+}
+
+export interface IAsyncControllableInputState {
+    /**
+     * Whether we are in the middle of a composition event.
+     * @default false
+     */
+    isComposing: boolean;
+
+    /**
+     * The source of truth for the input value. This is not updated during IME composition.
+     * It may be updated by a parent component.
+     * @default ""
+     */
+    externalValue: IAsyncControllableInputProps["value"];
+
+    /**
+     * The latest input value, which updates during IME composition. If undefined, we use
+     * externalValue instead.
+     */
+    localValue: IAsyncControllableInputProps["value"];
+}
+
+/**
+ * A stateful wrapper around the low-level <input> component which works around a
+ * [React bug](https://github.com/facebook/react/issues/3926). This bug is reproduced when an input
+ * receives CompositionEvents (for example, through IME composition) and has its value prop updated
+ * asychronously. This might happen if a component chooses to do async validation of a value
+ * returned by the input's `onChange` callback.
+ *
+ * Implementation adapted from https://jsfiddle.net/m792qtys/ (linked in the above issue thread).
+ *
+ * Note: this component does not apply any Blueprint-specific styling.
+ */
+@polyfill
+export class AsyncControllableInput extends React.PureComponent<
+    IAsyncControllableInputProps,
+    IAsyncControllableInputState
+> {
+    public state: IAsyncControllableInputState = {
+        externalValue: this.props.value,
+        isComposing: false,
+        localValue: this.props.value,
+    };
+
+    public static getDerivedStateFromProps({ value }: IAsyncControllableInputProps) {
+        return {
+            externalValue: value,
+        };
+    }
+
+    public render() {
+        const { isComposing, externalValue, localValue } = this.state;
+        const { inputRef, ...restProps } = this.props;
+        return (
+            <input
+                {...restProps}
+                ref={inputRef}
+                value={isComposing ? localValue : externalValue}
+                onCompositionStart={this.handleCompositionStart}
+                onCompositionEnd={this.handleCompositionEnd}
+                onChange={this.handleChange}
+            />
+        );
+    }
+
+    private handleCompositionStart = (e: React.CompositionEvent<HTMLInputElement>) => {
+        this.setState({
+            isComposing: true,
+            // Make sure that localValue matches externalValue, in case externalValue
+            // has changed since the last onChange event.
+            localValue: this.state.externalValue,
+        });
+        this.props.onCompositionStart?.(e);
+    };
+
+    private handleCompositionEnd = (e: React.CompositionEvent<HTMLInputElement>) => {
+        this.setState({ isComposing: false });
+        this.props.onCompositionEnd?.(e);
+    };
+
+    private handleChange = (e: React.ChangeEvent<HTMLInputElement>) => {
+        const { value } = e.target;
+
+        this.setState({ localValue: value });
+        this.props.onChange?.(e);
+    };
+}

packages/core/src/components/forms/inputGroup.tsx

@@ -29,6 +29,7 @@ import {
     removeNonHTMLProps,
 } from "../../common/props";
 import { Icon, IconName } from "../icon/icon";
+import { AsyncControllableInput } from "./asyncControllableInput";
 
 // NOTE: This interface does not extend HTMLInputProps due to incompatiblity with `IControlledProps`.
 // Instead, we union the props in the component definition, which does work and properly disallows `string[]` values.
@@ -106,7 +107,7 @@ export class InputGroup extends AbstractPureComponent2<IInputGroupProps & HTMLIn
     };
 
     public render() {
-        const { className, disabled, fill, intent, large, small, round } = this.props;
+        const { className, disabled, fill, inputRef, intent, large, small, round } = this.props;
         const classes = classNames(
             Classes.INPUT_GROUP,
             Classes.intentClass(intent),
@@ -129,11 +130,11 @@ export class InputGroup extends AbstractPureComponent2<IInputGroupProps & HTMLIn
         return (
             <div className={classes}>
                 {this.maybeRenderLeftElement()}
-                <input
+                <AsyncControllableInput
                     type="text"
                     {...removeNonHTMLProps(this.props)}
                     className={Classes.INPUT}
-                    ref={this.props.inputRef}
+                    inputRef={inputRef}
                     style={style}
                 />
                 {this.maybeRenderRightElement()}

packages/core/test/controls/numericInputTests.tsx

@@ -752,7 +752,7 @@ describe("<NumericInput>", () => {
                 const inputField = component.find("input");
 
                 inputField.simulate("change", { target: { value: "-5" } });
-                inputField.simulate("blur");
+                inputField.simulate("blur", { target: { value: "-5" } });
                 expect(component.state().value).to.equal(MIN.toString());
             });
 
@@ -762,7 +762,7 @@ describe("<NumericInput>", () => {
                 const inputField = component.find("input");
 
                 inputField.simulate("change", { target: { value: "5" } });
-                inputField.simulate("blur");
+                inputField.simulate("blur", { target: { value: "5" } });
                 expect(component.state().value).to.equal(MAX.toString());
             });
 
@@ -775,7 +775,7 @@ describe("<NumericInput>", () => {
                 const inputField = component.find("input");
 
                 inputField.simulate("change", { target: { value: "-5" } });
-                inputField.simulate("blur");
+                inputField.simulate("blur", { target: { value: "-5" } });
 
                 const args = onValueChange.getCall(1).args;
                 expect(onValueChange.calledTwice).to.be.true;

packages/core/test/forms/asyncControllableInputTests.tsx

@@ -0,0 +1,82 @@
+/*
+ * Copyright 2020 Palantir Technologies, Inc. All rights reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { assert } from "chai";
+import { mount } from "enzyme";
+import * as React from "react";
+import { spy } from "sinon";
+
+// this component is not part of the public API, but we want to test its implementation in isolation
+import { AsyncControllableInput } from "../../src/components/forms/asyncControllableInput";
+
+describe("<AsyncControllableInput>", () => {
+    it("renders an input", () => {
+        const wrapper = mount(<AsyncControllableInput type="text" value="hi" />);
+        assert.strictEqual(wrapper.childAt(0).type(), "input");
+    });
+
+    it("accepts controlled updates", () => {
+        const wrapper = mount(<AsyncControllableInput type="text" value="hi" />);
+        assert.strictEqual(wrapper.find("input").prop("value"), "hi");
+        wrapper.setProps({ value: "bye" });
+        assert.strictEqual(wrapper.find("input").prop("value"), "bye");
+    });
+
+    it("triggers onChange events during composition", () => {
+        const handleChangeSpy = spy();
+        const wrapper = mount(<AsyncControllableInput type="text" value="hi" onChange={handleChangeSpy} />);
+        const input = wrapper.find("input");
+
+        input.simulate("compositionstart", { data: "" });
+        input.simulate("compositionupdate", { data: " " });
+        // some browsers trigger this change event during composition, so we test to ensure that our wrapper component does too
+        input.simulate("change", { target: { value: "hi " } });
+        input.simulate("compositionupdate", { data: " ." });
+        input.simulate("change", { target: { value: "hi ." } });
+        input.simulate("compositionend", { data: " ." });
+
+        assert.strictEqual(handleChangeSpy.callCount, 2);
+    });
+
+    it("external updates do not override in-progress composition", async () => {
+        const wrapper = mount(<AsyncControllableInput type="text" value="hi" />);
+        const input = wrapper.find("input");
+
+        input.simulate("compositionstart", { data: "" });
+        input.simulate("compositionupdate", { data: " " });
+        input.simulate("change", { target: { value: "hi " } });
+
+        await Promise.resolve();
+        wrapper.setProps({ value: "bye" }).update();
+
+        assert.strictEqual(wrapper.find("input").prop("value"), "hi ");
+    });
+
+    it("external updates flush after composition ends", async () => {
+        const wrapper = mount(<AsyncControllableInput type="text" value="hi" />);
+        const input = wrapper.find("input");
+
+        input.simulate("compositionstart", { data: "" });
+        input.simulate("compositionupdate", { data: " " });
+        input.simulate("change", { target: { value: "hi " } });
+        input.simulate("compositionend", { data: " " });
+
+        await Promise.resolve();
+        wrapper.setProps({ value: "bye" }).update();
+
+        assert.strictEqual(wrapper.find("input").prop("value"), "bye");
+    });
+});

packages/core/test/index.ts

@@ -34,6 +34,7 @@ import "./controls/radioGroupTests";
 import "./dialog/dialogTests";
 import "./drawer/drawerTests";
 import "./editable-text/editableTextTests";
+import "./forms/asyncControllableInputTests";
 import "./forms/fileInputTests";
 import "./forms/formGroupTests";
 import "./forms/textAreaTests";