React - useImperativeHandle dependencies fix and useCallbacks' cleanup (T1308983) (#31870)

Author: ajivanyandev

packages/devextreme-react/src/core/__tests__/props-updating.test.tsx

@@ -1,5 +1,5 @@
 /* eslint-disable max-classes-per-file */
-import { cleanup, render } from '@testing-library/react';
+import { cleanup, render, fireEvent } from '@testing-library/react';
 import * as React from 'react';
 import { memo } from 'react';
 import { act } from 'react-dom/test-utils';
@@ -188,6 +188,80 @@ describe('option update', () => {
     expect(Widget.option.mock.calls[1][1]).toEqual(ref.current?.instance().element());
   });
 
+  it('keeps component ref defined when controlled selection triggers optionChanged', () => {
+    const ref = React.createRef<TestComponentRef>();
+    const clickInstances: Array<ReturnType<TestComponentRef['instance']> | undefined> = [];
+    const optionChangedInstances: Array<ReturnType<TestComponentRef['instance']> | undefined> = [];
+
+    const SelectionScenario = () => {
+      const [selectedRowKeys, setSelectedRowKeys] = React.useState<number[]>([]);
+
+      const handleClick = React.useCallback(() => {
+        clickInstances.push(ref.current?.instance());
+        setSelectedRowKeys((prev) => [...prev, prev.length + 1]);
+      }, []);
+
+      const handleOptionChanged = React.useCallback((e: { fullName?: string }) => {
+        if (e.fullName === 'selectedRowKeys') {
+          optionChangedInstances.push(ref.current?.instance());
+        }
+      }, []);
+
+      return (
+        <>
+          <button type="button" onClick={handleClick}>Test</button>
+          <TestComponent
+            ref={ref}
+            selectedRowKeys={selectedRowKeys}
+            selection={{ mode: 'multiple' }}
+            onOptionChanged={handleOptionChanged}
+            independentEvents={['onOptionChanged']}
+          />
+        </>
+      );
+    };
+
+    const { getByText } = render(<SelectionScenario />);
+
+    let currentOnOptionChanged = WidgetClass.mock.calls[0][1].onOptionChanged;
+
+    expect(typeof currentOnOptionChanged).toBe('function');
+
+    Widget.option.mockImplementation((name: string, value: unknown) => {
+      if (name === 'integrationOptions.useDeferUpdateForTemplates') {
+        return false;
+      }
+
+      if (name === 'onOptionChanged') {
+        currentOnOptionChanged = value as typeof currentOnOptionChanged;
+        return undefined;
+      }
+
+      if (name === 'selectedRowKeys' && typeof currentOnOptionChanged === 'function') {
+        currentOnOptionChanged({
+          component: Widget,
+          element: undefined,
+          fullName: name,
+          model: undefined,
+          name,
+          previousValue: undefined,
+          value,
+        });
+      }
+
+      return undefined;
+    });
+
+    act(() => {
+      fireEvent.click(getByText('Test'));
+    });
+
+    expect(clickInstances).toHaveLength(1);
+    expect(clickInstances[0]).toBeTruthy();
+    expect(optionChangedInstances).toHaveLength(1);
+    expect(optionChangedInstances[0]).toBeTruthy();
+  });
+
   it('updates nested collection item', () => {
     const TestContainer = (props: any) => {
       const { value } = props;

packages/devextreme-react/src/core/__tests__/test-component.tsx

@@ -8,6 +8,7 @@ import {
   useCallback,
   useContext,
   useRef,
+  useLayoutEffect,
   forwardRef,
   ReactElement,
 } from 'react';
@@ -65,20 +66,26 @@ const TestComponent = memo(forwardRef<TestComponentRef, any>(function TestCompon
     Widget.resetOption.mockReset();
   }, []);
 
+  const propsRef = useRef(props);
+
+  useLayoutEffect(() => {
+    propsRef.current = props;
+  }, [props]);
+
   useImperativeHandle(ref, () => {
     return {
       instance() {
         return {
           element() {
             return getElement();
           }
-        }
+        };
       },
       getProps() {
-        return props;
+        return propsRef.current;
       },
     };
-  }, [componentRef.current, getElement, props]);
+  }, []);
 
   return (
     <Component<P & IHtmlOptions>
@@ -104,12 +111,17 @@ const TestPortalComponent = memo(forwardRef<TestComponentRef, any>(function Test
 
 const TestRestoreTreeComponent = forwardRef((_, ref: React.ForwardedRef<{ restoreTree?: () => void }>) => {
   const restoreParentLink = useContext(RestoreTreeContext);
+  const restoreParentLinkRef = useRef<() => void>(() => {});
+
+  useLayoutEffect(() => {
+    restoreParentLinkRef.current = restoreParentLink ?? (() => {});
+  }, [restoreParentLink]);
 
   useImperativeHandle(ref, () => {
     return {
-      restoreTree: restoreParentLink
+      restoreTree: () => restoreParentLinkRef.current(),
     };
-  }, [restoreParentLink]);
+  }, []);
 
   return <div>Context Component</div>;
 });

packages/devextreme-react/src/core/component-base.tsx

@@ -80,6 +80,7 @@ const ComponentBase = forwardRef<ComponentBaseRef, any>(
     const [, setForceUpdateToken] = useState(Symbol('initial force update token'));
     const removalLocker = useContext(RemovalLockerContext);
     const restoreParentLink = useContext(RestoreTreeContext);
+    const restoreParentLinkRef = useRef(restoreParentLink);
     const instance = useRef<any>();
     const element = useRef<HTMLDivElement>();
     const portalContainer = useRef<HTMLElement | null>();
@@ -127,15 +128,10 @@ const ComponentBase = forwardRef<ComponentBaseRef, any>(
         childElementsDetached.current = false;
       }
 
-      if (restoreParentLink && element.current && !element.current.isConnected) {
-        restoreParentLink();
+      if (restoreParentLinkRef.current && element.current && !element.current.isConnected) {
+        restoreParentLinkRef.current();
       }
-    }, [
-      childNodes.current,
-      element.current,
-      childElementsDetached.current,
-      restoreParentLink,
-    ]);
+    }, []);
 
     const updateCssClasses = useCallback((prevProps: (P & ComponentBaseProps) | undefined, newProps: P & ComponentBaseProps) => {
       const prevClassName = prevProps ? getClassName(prevProps) : undefined;
@@ -156,7 +152,7 @@ const ComponentBase = forwardRef<ComponentBaseRef, any>(
           element.current?.classList.add(...classNames);
         }
       }
-    }, [element.current]);
+    }, []);
 
     const setInlineStyles = useCallback((styles) => {
       if (element.current) {
@@ -172,7 +168,7 @@ const ComponentBase = forwardRef<ComponentBaseRef, any>(
           },
         );
       }
-    }, [element.current]);
+    }, []);
 
     const setTemplateManagerHooks = useCallback(({
       createDXTemplates: createDXTemplatesFn,
@@ -182,11 +178,7 @@ const ComponentBase = forwardRef<ComponentBaseRef, any>(
       createDXTemplates.current = createDXTemplatesFn;
       clearInstantiationModels.current = clearInstantiationModelsFn;
       updateTemplates.current = updateTemplatesFn;
-    }, [
-      createDXTemplates.current,
-      clearInstantiationModels.current,
-      updateTemplates.current,
-    ]);
+    }, []);
 
     const getElementProps = useCallback(() => {
       const elementProps: Record<string, any> = {
@@ -203,7 +195,7 @@ const ComponentBase = forwardRef<ComponentBaseRef, any>(
         }
       });
       return elementProps;
-    }, [element.current, props]);
+    }, [props]);
 
     const scheduleTemplatesUpdate = useCallback(() => {
       if (guardsUpdateScheduled.current) {
@@ -221,11 +213,7 @@ const ComponentBase = forwardRef<ComponentBaseRef, any>(
       });
 
       unscheduleGuards();
-    }, [
-      guardsUpdateScheduled.current,
-      useDeferUpdateForTemplates.current,
-      updateTemplates.current,
-    ]);
+    }, []);
 
     const createWidget = useCallback((el?: Element) => {
       beforeCreateWidget();
@@ -266,14 +254,8 @@ const ComponentBase = forwardRef<ComponentBaseRef, any>(
     }, [
       beforeCreateWidget,
       afterCreateWidget,
-      element.current,
-      optionsManager.current,
-      createDXTemplates.current,
-      clearInstantiationModels.current,
       WidgetClass,
       useRequestAnimationFrameFlag,
-      useDeferUpdateForTemplates.current,
-      instance.current,
       subscribableOptions,
       independentEvents,
       widgetConfig,
@@ -284,7 +266,7 @@ const ComponentBase = forwardRef<ComponentBaseRef, any>(
         instance.current.focus();
         shouldRestoreFocus.current = false;
       }
-    }, [shouldRestoreFocus.current, instance.current]);
+    }, []);
 
     const onComponentUpdated = useCallback(() => {
       if (!optionsManager.current?.isInstanceSet) {
@@ -301,9 +283,6 @@ const ComponentBase = forwardRef<ComponentBaseRef, any>(
 
       prevPropsRef.current = props;
     }, [
-      optionsManager.current,
-      prevPropsRef.current,
-      createDXTemplates.current,
       scheduleTemplatesUpdate,
       updateCssClasses,
       props,
@@ -326,9 +305,6 @@ const ComponentBase = forwardRef<ComponentBaseRef, any>(
 
       prevPropsRef.current = props;
     }, [
-      childNodes.current,
-      element.current,
-      childElementsDetached.current,
       updateCssClasses,
       setInlineStyles,
       props,
@@ -358,15 +334,7 @@ const ComponentBase = forwardRef<ComponentBaseRef, any>(
       optionsManager.current.dispose();
 
       removalLocker?.unlock();
-    }, [
-      removalLocker,
-      instance.current,
-      childNodes.current,
-      element.current,
-      optionsManager.current,
-      childElementsDetached.current,
-      shouldRestoreFocus.current,
-    ]);
+    }, [removalLocker]);
 
     useLayoutEffect(() => {
       onComponentMounted();
@@ -376,10 +344,20 @@ const ComponentBase = forwardRef<ComponentBaseRef, any>(
       };
     }, []);
 
+    useLayoutEffect(() => {
+      restoreParentLinkRef.current = restoreParentLink;
+    }, [restoreParentLink]);
+
     useLayoutEffect(() => {
       onComponentUpdated();
     });
 
+    const createWidgetRef = useRef(createWidget);
+
+    useLayoutEffect(() => {
+      createWidgetRef.current = createWidget;
+    }, [createWidget]);
+
     useImperativeHandle(ref, () => (
       {
         getInstance() {
@@ -389,10 +367,10 @@ const ComponentBase = forwardRef<ComponentBaseRef, any>(
           return element.current;
         },
         createWidget(el) {
-          createWidget(el);
+          createWidgetRef.current?.(el);
         },
       }
-    ), [instance.current, element.current, createWidget]);
+    ), []);
 
     const _renderChildren = useCallback(() => {
       if (renderChildren) {
@@ -406,7 +384,7 @@ const ComponentBase = forwardRef<ComponentBaseRef, any>(
     const renderPortal = useCallback(() => portalContainer.current && createPortal(
       _renderChildren(),
       portalContainer.current,
-    ), [portalContainer.current, _renderChildren]);
+    ), [_renderChildren]);
 
     const renderContent = useCallback(() => {
       const { children } = props;
@@ -425,7 +403,6 @@ const ComponentBase = forwardRef<ComponentBaseRef, any>(
     }, [
       props,
       isPortalComponent,
-      portalContainer.current,
       _renderChildren,
     ]);