[EuiValidatableControl] Fixed ref not being handled properly when use…

…d with react-hook-form (#4001) * [EuiValidatableControl] Fixed ref not being handled properly when used with react-hook-form - Fix react-hook-form's reset() not working properly with EuiFieldText and more - See react-hook-form/react-hook-form#2637 * Updated changelog * [EuiValidatableControl] Reimplemented using react hooks - Utilize useMemo() to prevent stable ref from being called on every render * [EuiValidatableControl] Added more unit tests for ref management * [EuiValidatableControl] Replace useMemo with useCallback Co-authored-by: Chandler Prall <[email protected]>
elastic · Sep 9, 2020 · eee0b3b · eee0b3b
1 parent 6661ca9
commit eee0b3b
Show file tree

Hide file tree

Showing 3 changed files with 140 additions and 40 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,6 +1,8 @@
 ## [`master`](https://github.com/elastic/eui/tree/master)
 
-No public interface changes since `28.4.0`.
+**Bug fixes**
+
+- Fixed ref not being handled properly in `EuiValidatableControl` when used with [react-hook-form](https://react-hook-form.com/) ([#4001](https://github.com/elastic/eui/pull/4001))
 
 ## [`28.4.0`](https://github.com/elastic/eui/tree/v28.4.0)
 

diff --git a/src/components/form/validatable_control/validatable_control.test.tsx b/src/components/form/validatable_control/validatable_control.test.tsx
@@ -61,5 +61,107 @@ describe('EuiValidatableControl', () => {
       expect(ref.current).not.toBeNull();
       expect(ref.current!.getAttribute('id')).toBe('testInput');
     });
+
+    it('calls stable ref function only once on re-render', async () => {
+      const ref = jest.fn();
+
+      const Component = () => (
+        <EuiValidatableControl>
+          <input id="testInput" ref={ref} />
+        </EuiValidatableControl>
+      );
+
+      const wrapper = mount(<Component />);
+
+      expect(ref).toHaveBeenCalledTimes(1);
+      expect(ref.mock.calls[0][0].getAttribute('id')).toBe('testInput');
+
+      // Force re-render
+      wrapper.setProps({});
+
+      expect(ref).toHaveBeenCalledTimes(1);
+      expect(ref.mock.calls[0][0].getAttribute('id')).toBe('testInput');
+    });
+
+    it('calls unstable ref function again on re-render', async () => {
+      const ref = jest.fn();
+
+      const Component = () => (
+        <EuiValidatableControl>
+          <input id="testInput" ref={el => ref(el)} />
+        </EuiValidatableControl>
+      );
+
+      const wrapper = mount(<Component />);
+
+      expect(ref).toHaveBeenCalledTimes(1);
+      expect(ref.mock.calls[0][0].getAttribute('id')).toBe('testInput');
+
+      // Force re-render
+      wrapper.setProps({});
+
+      expect(ref).toHaveBeenCalledTimes(3);
+
+      expect(ref.mock.calls[1][0]).toBe(null);
+      expect(ref.mock.calls[2][0].getAttribute('id')).toBe('testInput');
+    });
+
+    it('calls a ref function again when the child element changes', () => {
+      const ref = jest.fn();
+
+      const Component = ({ change }: { change: boolean }) => (
+        <EuiValidatableControl>
+          {!change ? (
+            <input key="1" id="testInput" ref={ref} />
+          ) : (
+            <input key="2" id="testInput2" ref={ref} />
+          )}
+        </EuiValidatableControl>
+      );
+
+      const wrapper = mount(<Component change={false} />);
+
+      expect(ref).toHaveBeenCalledTimes(1);
+      expect(ref.mock.calls[0][0].getAttribute('id')).toBe('testInput');
+
+      wrapper.setProps({ change: true });
+
+      expect(ref).toHaveBeenCalledTimes(3);
+
+      expect(ref.mock.calls[1][0]).toBe(null);
+      expect(ref.mock.calls[2][0].getAttribute('id')).toBe('testInput2');
+
+      // Ensure that the child element has changed
+      expect(ref.mock.calls[0][0]).not.toBe(ref.mock.calls[2][0]);
+    });
+
+    it('sets a ref object\'s "current" property when the child element changes', () => {
+      const ref = React.createRef<HTMLInputElement>();
+
+      const Component = ({ change }: { change: boolean }) => (
+        <EuiValidatableControl>
+          {!change ? (
+            <input key="1" id="testInput" ref={ref} />
+          ) : (
+            <input key="2" id="testInput2" ref={ref} />
+          )}
+        </EuiValidatableControl>
+      );
+
+      const wrapper = mount(<Component change={false} />);
+
+      expect(ref.current).not.toBeNull();
+      expect(ref.current!.getAttribute('id')).toBe('testInput');
+
+      const prevRef = ref.current;
+
+      wrapper.setProps({ change: true });
+
+      expect(ref.current).not.toBeNull();
+      expect(ref.current!.getAttribute('id')).toBe('testInput2');
+
+      // Ensure that the child element has changed
+      expect(ref.current).not.toBe(prevRef);
+    });
   });
 });
diff --git a/src/components/form/validatable_control/validatable_control.tsx b/src/components/form/validatable_control/validatable_control.tsx
@@ -20,10 +20,13 @@
 import {
   Children,
   cloneElement,
-  Component,
   MutableRefObject,
   ReactElement,
   Ref,
+  FunctionComponent,
+  useRef,
+  useEffect,
+  useCallback,
 } from 'react';
 import { CommonProps } from '../../common';
 
@@ -49,50 +52,43 @@ export interface EuiValidatableControlProps {
   children: ReactElementWithRef;
 }
 
-export class EuiValidatableControl extends Component<
-  CommonProps & EuiValidatableControlProps
-> {
-  private control?: HTMLConstraintValidityElement;
+export const EuiValidatableControl: FunctionComponent<CommonProps &
+  EuiValidatableControlProps> = ({ isInvalid, children }) => {
+  const control = useRef<HTMLConstraintValidityElement | null>(null);
 
-  updateValidity() {
+  const child = Children.only(children);
+  const childRef = child.ref;
+
+  const replacedRef = useCallback(
+    (element: HTMLConstraintValidityElement) => {
+      control.current = element;
+
+      // Call the original ref, if any
+      if (typeof childRef === 'function') {
+        childRef(element);
+      } else if (isMutableRef(childRef)) {
+        childRef.current = element;
+      }
+    },
+    [childRef]
+  );
+
+  useEffect(() => {
     if (
-      this.control == null ||
-      typeof this.control.setCustomValidity !== 'function'
+      control.current === null ||
+      typeof control.current.setCustomValidity !== 'function'
     ) {
       return; // jsdom doesn't polyfill this for the server-side
     }
 
-    if (this.props.isInvalid) {
-      this.control.setCustomValidity('Invalid');
+    if (isInvalid) {
+      control.current.setCustomValidity('Invalid');
     } else {
-      this.control.setCustomValidity('');
+      control.current.setCustomValidity('');
     }
-  }
-
-  componentDidMount() {
-    this.updateValidity();
-  }
-
-  componentDidUpdate() {
-    this.updateValidity();
-  }
+  });
 
-  setRef = (element: HTMLConstraintValidityElement) => {
-    this.control = element;
-
-    // Call the original ref, if any
-    const { ref } = this.props.children;
-    if (typeof ref === 'function') {
-      ref(element);
-    } else if (isMutableRef(ref)) {
-      ref.current = element;
-    }
-  };
-
-  render() {
-    const child = Children.only(this.props.children);
-    return cloneElement(child, {
-      ref: this.setRef,
-    });
-  }
-}
+  return cloneElement(child, {
+    ref: replacedRef,
+  });
+};