[core] fix(InputGroup): make async control optional

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

@@ -109,7 +109,7 @@ export class AsyncControllableInput extends React.PureComponent<
     }
 
     public render() {
-        const { isComposing, hasPendingUpdate: pendingUpdate, value, nextValue } = this.state;
+        const { isComposing, hasPendingUpdate, value, nextValue } = this.state;
         const { inputRef, ...restProps } = this.props;
         return (
             <input
@@ -118,7 +118,7 @@ export class AsyncControllableInput extends React.PureComponent<
                 // render the pending value even if it is not confirmed by a parent's async controlled update
                 // so that the cursor does not jump to the end of input as reported in
                 // https://github.com/palantir/blueprint/issues/4298
-                value={isComposing || pendingUpdate ? nextValue : value}
+                value={isComposing || hasPendingUpdate ? nextValue : value}
                 onCompositionStart={this.handleCompositionStart}
                 onCompositionEnd={this.handleCompositionEnd}
                 onChange={this.handleChange}

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

@@ -34,6 +34,14 @@ 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.
 export interface IInputGroupProps extends IControlledProps, IIntentProps, IProps {
+    /**
+     * Set this to `true` if you will be controlling the `value` of this input with asynchronous updates.
+     * These may occur if you do not immediately call setState in a parent component with the value from
+     * the `onChange` handler, or if working with certain libraries like __redux-form__.
+     * @default false
+     */
+    asyncControl?: boolean;
+
     /**
      * Whether the input is non-interactive.
      * Note that `rightElement` must be disabled separately; this prop will not affect it.
@@ -107,8 +115,8 @@ export class InputGroup extends AbstractPureComponent2<IInputGroupProps & HTMLIn
     };
 
     public render() {
-        const { className, disabled, fill, inputRef, intent, large, small, round } = this.props;
-        const classes = classNames(
+        const { asyncControl = false, className, disabled, fill, inputRef, intent, large, small, round } = this.props;
+        const inputGroupClasses = classNames(
             Classes.INPUT_GROUP,
             Classes.intentClass(intent),
             {
@@ -120,23 +128,26 @@ export class InputGroup extends AbstractPureComponent2<IInputGroupProps & HTMLIn
             },
             className,
         );
-
         const style: React.CSSProperties = {
             ...this.props.style,
             paddingLeft: this.state.leftElementWidth,
             paddingRight: this.state.rightElementWidth,
         };
+        const inputProps = {
+            type: "text",
+            ...removeNonHTMLProps(this.props),
+            className: Classes.INPUT,
+            style,
+        };
 
         return (
-            <div className={classes}>
+            <div className={inputGroupClasses}>
                 {this.maybeRenderLeftElement()}
-                <AsyncControllableInput
-                    type="text"
-                    {...removeNonHTMLProps(this.props)}
-                    className={Classes.INPUT}
-                    inputRef={inputRef}
-                    style={style}
-                />
+                {asyncControl ? (
+                    <AsyncControllableInput {...inputProps} inputRef={inputRef} />
+                ) : (
+                    <input {...inputProps} ref={inputRef} />
+                )}
                 {this.maybeRenderRightElement()}
             </div>
         );

packages/core/src/components/forms/text-inputs.md

@@ -30,10 +30,11 @@ all valid props for HTML `<input>` elements and proxies them to that element in
 the DOM; the most common ones are detailed below.
 
 If controlled with the `value` prop, `InputGroup` has support for _asynchronous updates_, which may
-occur with some form handling libraries like `redux-form`. This is not generally encouraged (a value
-returned from `onChange` should generally be sent back to the component as a controlled `value` synchronously),
-but there is basic support for it. Note that the input cursor may jump to the end of the input if the speed
-of text entry (time between change events) is faster than the speed of the async update.
+occur with some form handling libraries like `redux-form`. This is not broadly encouraged (a value
+returned from `onChange` should be sent back to the component as a controlled `value` synchronously),
+but there is basic support for it using the `asyncControl` prop. Note that the input cursor may jump
+to the end of the input if the speed of text entry (time between change events) is faster than the
+speed of the async update.
 
 @interface IInputGroupProps
 

packages/core/test/controls/inputGroupTests.tsx

@@ -67,4 +67,39 @@ describe("<InputGroup>", () => {
         mount(<InputGroup inputRef={ref => (input = ref)} />);
         assert.instanceOf(input, HTMLInputElement);
     });
+
+    // this test was added to validate a regression introduced by AsyncControllableInput,
+    // see https://github.com/palantir/blueprint/issues/4375
+    it("accepts controlled update truncating the input value", () => {
+        class TestComponent extends React.PureComponent<
+            { initialValue: string; transformInput: (value: string) => string },
+            { value: string }
+        > {
+            public state = { value: this.props.initialValue };
+            public render() {
+                return <InputGroup type="text" value={this.state.value} onChange={this.handleChange} />;
+            }
+            private handleChange = (e: React.ChangeEvent<HTMLInputElement>) => {
+                this.setState({
+                    value: this.props.transformInput(e.target.value),
+                });
+            };
+        }
+
+        const wrapper = mount(
+            <TestComponent
+                initialValue="abc"
+                // tslint:disable-next-line:jsx-no-lambda
+                transformInput={(value: string) => value.substr(0, 3)}
+            />,
+        );
+
+        let input = wrapper.find("input");
+        assert.strictEqual(input.prop("value"), "abc");
+
+        input.simulate("change", { target: { value: "abcd" } });
+        input = wrapper.find("input");
+        // value should not change because our change handler prevents it from being longer than characters
+        assert.strictEqual(input.prop("value"), "abc");
+    });
 });

packages/core/test/forms/asyncControllableInputTests.tsx

@@ -14,11 +14,15 @@
  * limitations under the License.
  */
 
+/* eslint-disable max-classes-per-file */
+
 import { assert } from "chai";
 import { mount } from "enzyme";
 import * as React from "react";
 import { spy } from "sinon";
 
+import { sleep } from "../utils";
+
 // 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";
 
@@ -46,7 +50,7 @@ describe("<AsyncControllableInput>", () => {
             assert.strictEqual(wrapper.childAt(0).type(), "input");
         });
 
-        it("accepts controlled updates", () => {
+        it("accepts controlled update 'hi' -> 'bye'", () => {
             const wrapper = mount(<AsyncControllableInput type="text" value="hi" />);
             assert.strictEqual(wrapper.find("input").prop("value"), "hi");
             wrapper.setProps({ value: "bye" });
@@ -98,5 +102,50 @@ describe("<AsyncControllableInput>", () => {
 
             assert.strictEqual(wrapper.find("input").prop("value"), "bye");
         });
+
+        // this test only seems to work in React 16, where we don't rely on the react-lifecycles-compat polyfill
+        if (React.version.startsWith("16")) {
+            it("accepts async controlled update, optimistically rendering new value while waiting for update", async () => {
+                class TestComponent extends React.PureComponent<{ initialValue: string }, { value: string }> {
+                    public state = { value: this.props.initialValue };
+                    public render() {
+                        return (
+                            <AsyncControllableInput type="text" value={this.state.value} onChange={this.handleChange} />
+                        );
+                    }
+                    private handleChange = (e: React.ChangeEvent<HTMLInputElement>) => {
+                        const newValue = e.target.value;
+                        window.setTimeout(() => this.setState({ value: newValue }), 10);
+                    };
+                }
+
+                const wrapper = mount(<TestComponent initialValue="hi" />);
+                assert.strictEqual(wrapper.find("input").prop("value"), "hi");
+
+                wrapper.find("input").simulate("change", { target: { value: "hi " } });
+                wrapper.update();
+
+                assert.strictEqual(
+                    wrapper.find(AsyncControllableInput).prop("value"),
+                    "hi",
+                    "local state should still have initial value",
+                );
+                // but rendered input should optimistically show new value
+                assert.strictEqual(
+                    wrapper.find("input").prop("value"),
+                    "hi ",
+                    "rendered <input> should optimistically show new value",
+                );
+
+                // after async delay, confirm the update
+                await sleep(20);
+                assert.strictEqual(
+                    wrapper.find("input").prop("value"),
+                    "hi ",
+                    "rendered <input> should still show new value",
+                );
+                return;
+            });
+        }
     });
 });

packages/core/test/utils.ts

@@ -32,3 +32,7 @@ export function findInPortal<P>(overlay: ReactWrapper<P>, selector: string) {
     }
     return portalChildren.find(selector);
 }
+
+export async function sleep(timeout?: number) {
+    return new Promise(resolve => window.setTimeout(resolve, timeout));
+}

packages/docs-app/src/examples/core-examples/inputGroupExample.tsx

@@ -100,6 +100,7 @@ export class InputGroupExample extends React.PureComponent<IExampleProps, IInput
             <Example options={this.renderOptions()} {...this.props}>
                 <Tooltip content="My input value state is updated asynchronously with a 10ms delay">
                     <InputGroup
+                        asyncControl={true}
                         disabled={disabled}
                         large={large}
                         leftIcon="filter"