From cd86546313444470fc6af0a9157722be9d666164 Mon Sep 17 00:00:00 2001
From: Josh Hunt <joshhunt@users.noreply.github.com>
Date: Mon, 25 Nov 2024 14:44:56 +0000
Subject: [PATCH] GrafanaUI: Fix inconsistent controlled/uncontrolled state in
 AutoSizeInput (#96696)

* GrafanaUI: Fix delayed state in AutoSizeInput due to mixed controlled/uncontrolled state

* clean up state management into seperate hook

* update test

* Sync new prop value to state when uncontrolled, tests
---
 .../components/Input/AutoSizeInput.test.tsx   | 319 ++++++++++++------
 .../src/components/Input/AutoSizeInput.tsx    |  62 +++-
 2 files changed, 260 insertions(+), 121 deletions(-)

diff --git a/packages/grafana-ui/src/components/Input/AutoSizeInput.test.tsx b/packages/grafana-ui/src/components/Input/AutoSizeInput.test.tsx
index 148f35c6af3..fad2600de3e 100644
--- a/packages/grafana-ui/src/components/Input/AutoSizeInput.test.tsx
+++ b/packages/grafana-ui/src/components/Input/AutoSizeInput.test.tsx
@@ -1,4 +1,6 @@
 import { screen, render, fireEvent, waitFor } from '@testing-library/react';
+import userEvent from '@testing-library/user-event';
+import { useState } from 'react';
 
 import { measureText } from '../../utils/measureText';
 
@@ -13,15 +15,214 @@ jest.mock('../../utils/measureText', () => {
   return { measureText };
 });
 
+// The length calculation should be (text.length * 14) + 24
+const FIXTURES = {
+  'Initial value': '206px',
+  'Initial value with more text': '416px',
+  'A new value': '178px',
+  'Placeholder text': '248px',
+  _emptyString: '80px', // min width
+  foo: '80px', // min width
+} as const;
+
 describe('AutoSizeInput', () => {
-  it('should support default Input API', () => {
-    const onChange = jest.fn();
-    render(<AutoSizeInput onChange={onChange} value="" />);
+  it('all the test fixture strings should be a different length', () => {
+    const lengths = Object.keys(FIXTURES).map((key) => key.length);
 
-    const input: HTMLInputElement = screen.getByTestId('autosize-input');
-    fireEvent.change(input, { target: { value: 'foo' } });
+    // The unique number of lengths should be the same as the number of items in the object
+    const uniqueLengths = new Set(lengths);
+    expect(uniqueLengths.size).toBe(lengths.length);
+  });
 
-    expect(onChange).toHaveBeenCalled();
+  describe('as an uncontrolled component', () => {
+    it('renders an initial value prop with correct width', async () => {
+      render(<AutoSizeInput value="Initial value" />);
+
+      const input: HTMLInputElement = screen.getByTestId('autosize-input');
+      const inputWrapper: HTMLDivElement = screen.getByTestId('input-wrapper');
+
+      expect(input.value).toBe('Initial value');
+      expect(getComputedStyle(inputWrapper).width).toBe(FIXTURES['Initial value']);
+    });
+
+    it('renders an updated value prop with correct width', () => {
+      const { rerender } = render(<AutoSizeInput value="Initial value" />);
+
+      const input: HTMLInputElement = screen.getByTestId('autosize-input');
+      const inputWrapper: HTMLDivElement = screen.getByTestId('input-wrapper');
+
+      rerender(<AutoSizeInput value="A new value" />);
+
+      expect(input.value).toBe('A new value');
+      expect(getComputedStyle(inputWrapper).width).toBe(FIXTURES['A new value']);
+    });
+
+    it('renders the user typing in the input with correct width', async () => {
+      render(<AutoSizeInput value="" />);
+
+      const input: HTMLInputElement = screen.getByTestId('autosize-input');
+      const inputWrapper: HTMLDivElement = screen.getByTestId('input-wrapper');
+
+      await userEvent.type(input, 'Initial value with more text');
+
+      expect(input.value).toBe('Initial value with more text');
+      expect(getComputedStyle(inputWrapper).width).toBe(FIXTURES['Initial value with more text']);
+    });
+
+    it('renders correctly after the user clears the input', async () => {
+      render(<AutoSizeInput value="Initial value" />);
+
+      const input: HTMLInputElement = screen.getByTestId('autosize-input');
+      const inputWrapper: HTMLDivElement = screen.getByTestId('input-wrapper');
+
+      await userEvent.clear(input);
+
+      expect(input.value).toBe('');
+      expect(getComputedStyle(inputWrapper).width).toBe(FIXTURES._emptyString);
+    });
+
+    it('renders correctly with a placeholder after the user clears the input', async () => {
+      render(<AutoSizeInput value="Initial value" placeholder="Placeholder text" />);
+
+      const input: HTMLInputElement = screen.getByTestId('autosize-input');
+      const inputWrapper: HTMLDivElement = screen.getByTestId('input-wrapper');
+
+      await userEvent.clear(input);
+
+      expect(input.value).toBe('');
+      expect(getComputedStyle(inputWrapper).width).toBe(FIXTURES['Placeholder text']);
+    });
+
+    it('emits onCommitChange when you blur the input', () => {
+      const onCommitChange = jest.fn();
+      render(<AutoSizeInput value="Initial value" onCommitChange={onCommitChange} />);
+
+      const input: HTMLInputElement = screen.getByTestId('autosize-input');
+
+      fireEvent.blur(input);
+
+      expect(onCommitChange).toHaveBeenCalledTimes(1);
+    });
+
+    it('emits onBlur instead of onCommitChange when you blur the input', () => {
+      const onCommitChange = jest.fn();
+      const onBlur = jest.fn();
+      render(<AutoSizeInput value="Initial value" onCommitChange={onCommitChange} onBlur={onBlur} />);
+
+      const input: HTMLInputElement = screen.getByTestId('autosize-input');
+
+      fireEvent.blur(input);
+
+      expect(onCommitChange).not.toHaveBeenCalled();
+      expect(onBlur).toHaveBeenCalledTimes(1);
+    });
+
+    it('emits the value when you press enter', async () => {
+      const onCommitChange = jest.fn();
+      render(<AutoSizeInput value="Initial value" onCommitChange={onCommitChange} />);
+
+      const input: HTMLInputElement = screen.getByTestId('autosize-input');
+
+      await userEvent.type(input, '{enter}');
+
+      expect(onCommitChange).toHaveBeenCalledTimes(1);
+    });
+  });
+
+  describe('as a controlled component', () => {
+    function ControlledAutoSizeInputExample() {
+      const [value, setValue] = useState('');
+      return <AutoSizeInput value={value} onChange={(event) => setValue(event.currentTarget.value)} />;
+    }
+
+    // AutoSizeInput is considered controlled when it has both value and onChange props
+
+    it('renders a value prop with correct width', () => {
+      const onChange = jest.fn();
+      render(<AutoSizeInput value="Initial value" onChange={onChange} />);
+
+      const input: HTMLInputElement = screen.getByTestId('autosize-input');
+      const inputWrapper: HTMLDivElement = screen.getByTestId('input-wrapper');
+
+      expect(input.value).toBe('Initial value');
+      expect(getComputedStyle(inputWrapper).width).toBe(FIXTURES['Initial value']);
+    });
+
+    it('renders an updated value prop with correct width', () => {
+      const onChange = jest.fn();
+      const { rerender } = render(<AutoSizeInput value="Initial value" onChange={onChange} />);
+
+      const input: HTMLInputElement = screen.getByTestId('autosize-input');
+      const inputWrapper: HTMLDivElement = screen.getByTestId('input-wrapper');
+
+      rerender(<AutoSizeInput value="A new value" onChange={onChange} />);
+
+      expect(input.value).toBe('A new value');
+      expect(getComputedStyle(inputWrapper).width).toBe(FIXTURES['A new value']);
+    });
+
+    it('as a user types, the value is not updated because it is controlled', async () => {
+      const onChange = jest.fn();
+      render(<AutoSizeInput value="Initial value" onChange={onChange} />);
+
+      const input: HTMLInputElement = screen.getByTestId('autosize-input');
+      const inputWrapper: HTMLDivElement = screen.getByTestId('input-wrapper');
+
+      await userEvent.type(input, ' and more text');
+
+      expect(input.value).toBe('Initial value');
+      expect(getComputedStyle(inputWrapper).width).toBe(FIXTURES['Initial value']);
+    });
+
+    it('functions correctly as a controlled input', async () => {
+      render(<ControlledAutoSizeInputExample />);
+      const input: HTMLInputElement = screen.getByTestId('autosize-input');
+      const inputWrapper: HTMLDivElement = screen.getByTestId('input-wrapper');
+
+      await userEvent.type(input, 'Initial value with more text');
+      expect(input.value).toBe('Initial value with more text');
+      expect(getComputedStyle(inputWrapper).width).toBe(FIXTURES['Initial value with more text']);
+    });
+
+    it('emits onCommitChange when you blur the input', () => {
+      const onCommitChange = jest.fn();
+      const onChange = jest.fn();
+      render(<AutoSizeInput value="Initial value" onCommitChange={onCommitChange} onChange={onChange} />);
+
+      const input: HTMLInputElement = screen.getByTestId('autosize-input');
+
+      fireEvent.blur(input);
+
+      expect(onCommitChange).toHaveBeenCalledTimes(1);
+    });
+
+    it('emits onBlur instead of onCommitChange when you blur the input', () => {
+      const onCommitChange = jest.fn();
+      const onBlur = jest.fn();
+      const onChange = jest.fn();
+      render(
+        <AutoSizeInput value="Initial value" onCommitChange={onCommitChange} onBlur={onBlur} onChange={onChange} />
+      );
+
+      const input: HTMLInputElement = screen.getByTestId('autosize-input');
+
+      fireEvent.blur(input);
+
+      expect(onCommitChange).not.toHaveBeenCalled();
+      expect(onBlur).toHaveBeenCalledTimes(1);
+    });
+
+    it('emits the value when you press enter', async () => {
+      const onCommitChange = jest.fn();
+      const onChange = jest.fn();
+      render(<AutoSizeInput value="Initial value" onCommitChange={onCommitChange} onChange={onChange} />);
+
+      const input: HTMLInputElement = screen.getByTestId('autosize-input');
+
+      await userEvent.type(input, '{enter}');
+
+      expect(onCommitChange).toHaveBeenCalledTimes(1);
+    });
   });
 
   it('should have default minWidth when empty', () => {
@@ -30,104 +231,38 @@ describe('AutoSizeInput', () => {
     const input: HTMLInputElement = screen.getByTestId('autosize-input');
     const inputWrapper: HTMLDivElement = screen.getByTestId('input-wrapper');
 
-    fireEvent.change(input, { target: { value: '' } });
-
     expect(input.value).toBe('');
-    expect(getComputedStyle(inputWrapper).width).toBe('80px');
+    expect(getComputedStyle(inputWrapper).width).toBe(FIXTURES._emptyString);
   });
 
   it('should have default minWidth for short content', () => {
-    render(<AutoSizeInput />);
+    render(<AutoSizeInput value="foo" />);
 
     const input: HTMLInputElement = screen.getByTestId('autosize-input');
     const inputWrapper: HTMLDivElement = screen.getByTestId('input-wrapper');
 
-    fireEvent.change(input, { target: { value: 'foo' } });
-
     expect(input.value).toBe('foo');
-    expect(getComputedStyle(inputWrapper).width).toBe('80px');
-  });
-
-  it('should change width for long content', () => {
-    render(<AutoSizeInput />);
-
-    const input: HTMLInputElement = screen.getByTestId('autosize-input');
-    const inputWrapper: HTMLDivElement = screen.getByTestId('input-wrapper');
-
-    fireEvent.change(input, { target: { value: 'very very long value' } });
-    expect(getComputedStyle(inputWrapper).width).toBe('304px');
+    expect(getComputedStyle(inputWrapper).width).toBe(FIXTURES['foo']);
   });
 
   it('should use placeholder for width if input is empty', () => {
-    render(<AutoSizeInput placeholder="very very long value" />);
+    render(<AutoSizeInput value="" placeholder="Placeholder text" />);
 
     const inputWrapper: HTMLDivElement = screen.getByTestId('input-wrapper');
-    expect(getComputedStyle(inputWrapper).width).toBe('304px');
+    expect(getComputedStyle(inputWrapper).width).toBe(FIXTURES['Placeholder text']);
   });
 
   it('should use value for width even with a placeholder', () => {
-    render(<AutoSizeInput value="less long value" placeholder="very very long value" />);
+    render(<AutoSizeInput value="Initial value" placeholder="Placeholder text" />);
 
-    const input: HTMLInputElement = screen.getByTestId('autosize-input');
     const inputWrapper: HTMLDivElement = screen.getByTestId('input-wrapper');
 
-    fireEvent.change(input, { target: { value: 'very very long value' } });
-    expect(getComputedStyle(inputWrapper).width).toBe('304px');
-  });
-
-  it('should call onBlur if set when blurring', () => {
-    const onBlur = jest.fn();
-    const onCommitChange = jest.fn();
-    render(<AutoSizeInput onBlur={onBlur} onCommitChange={onCommitChange} />);
-
-    const input: HTMLInputElement = screen.getByTestId('autosize-input');
-
-    fireEvent.blur(input);
-
-    expect(onBlur).toHaveBeenCalled();
-    expect(onCommitChange).not.toHaveBeenCalled();
-  });
-
-  it('should call onCommitChange if not set when blurring', () => {
-    const onCommitChange = jest.fn();
-    render(<AutoSizeInput onCommitChange={onCommitChange} />);
-
-    const input: HTMLInputElement = screen.getByTestId('autosize-input');
-
-    fireEvent.blur(input);
-
-    expect(onCommitChange).toHaveBeenCalled();
-  });
-
-  it('should call onKeyDown if set when keydown', () => {
-    const onKeyDown = jest.fn();
-    const onCommitChange = jest.fn();
-    render(<AutoSizeInput onKeyDown={onKeyDown} onCommitChange={onCommitChange} />);
-
-    const input: HTMLInputElement = screen.getByTestId('autosize-input');
-
-    fireEvent.keyDown(input, { key: 'Enter' });
-
-    expect(onKeyDown).toHaveBeenCalled();
-    expect(onCommitChange).not.toHaveBeenCalled();
-  });
-
-  it('should call onCommitChange if not set when keydown', () => {
-    const onCommitChange = jest.fn();
-    render(<AutoSizeInput onCommitChange={onCommitChange} />);
-
-    const input: HTMLInputElement = screen.getByTestId('autosize-input');
-
-    fireEvent.keyDown(input, { key: 'Enter' });
-
-    expect(onCommitChange).toHaveBeenCalled();
+    expect(getComputedStyle(inputWrapper).width).toBe(FIXTURES['Initial value']);
   });
 
   it('should respect min width', async () => {
     render(<AutoSizeInput minWidth={4} defaultValue="" />);
 
-    await waitFor(() => expect(measureText).toHaveBeenCalled());
-
     expect(getComputedStyle(screen.getByTestId('input-wrapper')).width).toBe('32px');
   });
 
@@ -145,32 +280,6 @@ describe('AutoSizeInput', () => {
     expect(getComputedStyle(screen.getByTestId('input-wrapper')).width).toBe('32px');
   });
 
-  it('should update the input value if the value prop changes', () => {
-    const { rerender } = render(<AutoSizeInput value="Initial" />);
-
-    // Get a handle on the original input element (to catch if it's unmounted)
-    // And check the initial value
-    const input: HTMLInputElement = screen.getByTestId('autosize-input');
-    expect(input.value).toBe('Initial');
-
-    // Rerender and make sure it clears the input
-    rerender(<AutoSizeInput value="Updated" />);
-    expect(input.value).toBe('Updated');
-  });
-
-  it('should clear the input when the value is changed to an empty string', () => {
-    const { rerender } = render(<AutoSizeInput value="Initial" />);
-
-    // Get a handle on the original input element (to catch if it's unmounted)
-    // And check the initial value
-    const input: HTMLInputElement = screen.getByTestId('autosize-input');
-    expect(input.value).toBe('Initial');
-
-    // Rerender and make sure it clears the input
-    rerender(<AutoSizeInput value="" />);
-    expect(input.value).toBe('');
-  });
-
   it('should render string values as expected', () => {
     render(<AutoSizeInput value="foo" />);
 
diff --git a/packages/grafana-ui/src/components/Input/AutoSizeInput.tsx b/packages/grafana-ui/src/components/Input/AutoSizeInput.tsx
index 9e7b275c120..e7a89ab4fc8 100644
--- a/packages/grafana-ui/src/components/Input/AutoSizeInput.tsx
+++ b/packages/grafana-ui/src/components/Input/AutoSizeInput.tsx
@@ -1,4 +1,4 @@
-import { useEffect, useMemo } from 'react';
+import { useCallback, useEffect, useMemo, useRef } from 'react';
 import * as React from 'react';
 
 import { measureText } from '../../utils/measureText';
@@ -28,37 +28,32 @@ export const AutoSizeInput = React.forwardRef<HTMLInputElement, Props>((props, r
     placeholder,
     ...restProps
   } = props;
-  // Initialize internal state
-  const [value, setValue] = React.useState(controlledValue ?? defaultValue);
-
-  // Update internal state when controlled `value` prop changes
-  useEffect(() => {
-    setValue(controlledValue ?? defaultValue);
-  }, [controlledValue, defaultValue]);
+  const [inputState, setInputValue] = useControlledState(controlledValue, onChange);
+  const inputValue = inputState || defaultValue;
 
   // Update input width when `value`, `minWidth`, or `maxWidth` change
   const inputWidth = useMemo(() => {
-    const displayValue = value || placeholder || '';
+    const displayValue = inputValue || placeholder || '';
     const valueString = typeof displayValue === 'string' ? displayValue : displayValue.toString();
 
     return getWidthFor(valueString, minWidth, maxWidth);
-  }, [placeholder, value, minWidth, maxWidth]);
+  }, [placeholder, inputValue, minWidth, maxWidth]);
 
   return (
-    // Used to tell Input to increase the width properly of the input to fit the text.
-    // See comment in Input.tsx for more details
     <AutoSizeInputContext.Provider value={true}>
       <Input
         {...restProps}
         placeholder={placeholder}
         ref={ref}
-        value={value.toString()}
+        value={inputValue.toString()}
         onChange={(event) => {
           if (onChange) {
             onChange(event);
           }
-          setValue(event.currentTarget.value);
+
+          setInputValue(event.currentTarget.value);
         }}
+        width={inputWidth}
         onBlur={(event) => {
           if (onBlur) {
             onBlur(event);
@@ -73,8 +68,7 @@ export const AutoSizeInput = React.forwardRef<HTMLInputElement, Props>((props, r
             onCommitChange(event);
           }
         }}
-        width={inputWidth}
-        data-testid="autosize-input"
+        data-testid={'autosize-input'}
       />
     </AutoSizeInputContext.Provider>
   );
@@ -100,3 +94,39 @@ function getWidthFor(value: string, minWidth: number, maxWidth: number | undefin
 }
 
 AutoSizeInput.displayName = 'AutoSizeInput';
+
+/**
+ * Hook to abstract away state management for controlled and uncontrolled inputs.
+ * If the initial value is not undefined, then the value will be controlled by the parent
+ * for the lifetime of the component and calls to setState will be ignored.
+ */
+function useControlledState<T>(controlledValue: T, onChange: Function | undefined): [T, (newValue: T) => void] {
+  const isControlledNow = controlledValue !== undefined && onChange !== undefined;
+  const isControlledRef = useRef(isControlledNow); // set the initial value - we never change this
+
+  const hasLoggedControlledWarning = useRef(false);
+  if (isControlledNow !== isControlledRef.current && !hasLoggedControlledWarning.current) {
+    console.warn(
+      'An AutoSizeInput is changing from an uncontrolled to a controlled input. If you want to control the input, the empty value should be an empty string.'
+    );
+    hasLoggedControlledWarning.current = true;
+  }
+
+  const [internalValue, setInternalValue] = React.useState(controlledValue);
+
+  useEffect(() => {
+    if (!isControlledRef.current) {
+      setInternalValue(controlledValue);
+    }
+  }, [controlledValue]);
+
+  const handleChange = useCallback((newValue: T) => {
+    if (!isControlledRef.current) {
+      setInternalValue(newValue);
+    }
+  }, []);
+
+  const value = isControlledRef.current ? controlledValue : internalValue;
+
+  return [value, handleChange];
+}