fix: double event fire in React wrappers (T1293563) (DevExpress#30138)

Author: VasilyStrelyaev

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

@@ -250,35 +250,37 @@ describe('option update', () => {
     expect(Widget.option.mock.calls.length).toBe(1);
     expect(Widget.option.mock.calls[0]).toEqual(['items[0].subItems[0].a', 234]);
   });
-});
 
-it('updates sub-nested collection item within a custom component', () => {
-  const MySetting = (props: any) => {
-    const { value } = props;
+  it('updates sub-nested collection item within a custom component', () => {
+    const MySetting = (props: any) => {
+      const { value } = props;
 
-    return (
-      <CollectionNestedComponent>
-        <CollectionSubNestedComponent a={value} />
-      </CollectionNestedComponent>
-    );
-  };
+      return (
+        <CollectionNestedComponent>
+          <CollectionSubNestedComponent a={value} />
+        </CollectionNestedComponent>
+      );
+    };
 
-  const TestContainer = (props: any) => {
-    const { value } = props;
-    return (
-      <TestComponentWithExpectation>
-        <MySetting value={value} />
-      </TestComponentWithExpectation>
-    );
-  };
-  const { rerender } = render(<TestContainer value={123} />);
-  rerender(<TestContainer value={234} />);
+    const TestContainer = (props: any) => {
+      const { value } = props;
+      return (
+        <TestComponentWithExpectation>
+          <MySetting value={value} />
+        </TestComponentWithExpectation>
+      );
+    };
+    const { rerender } = render(<TestContainer value={123} />);
+    rerender(<TestContainer value={234} />);
 
-  jest.runAllTimers();
-  expect(Widget.option.mock.calls.length).toBe(1);
-  expect(Widget.option.mock.calls[0]).toEqual(['items[0].subItems[0].a', 234]);
+    jest.runAllTimers();
+    expect(Widget.option.mock.calls.length).toBe(1);
+    expect(Widget.option.mock.calls[0]).toEqual(['items[0].subItems[0].a', 234]);
+  });
 });
 
+
+
 describe('option control', () => {
   afterEach(() => {
     jest.clearAllMocks();
@@ -359,26 +361,36 @@ describe('option control', () => {
     });
 
     it('should not rollback option if optionChanged is fired in endUpdate on props updating', () => {
-      const { rerender } = render(
+      const obj = { a: 'changed' };
+
+      let { rerender } = render(
         <ControlledComponent
           controlledOption="controlled"
+          complexOption={{ a: "controlled" }}
         />,
       );
 
-      Widget.endUpdate.mockImplementation(
-        () => {
-          fireOptionChange('controlledOption', 'changed');
-        },
-      );
+      try {
+        Widget.endUpdate.mockImplementation(
+          () => {
+            fireOptionChange('controlledOption', 'changed');
+            fireOptionChange('complexOption', obj);
+          },
+        );
 
-      rerender(<ControlledComponent
-        controlledOption="changed"
-      />);
+        rerender(<ControlledComponent
+          controlledOption="changed"
+          complexOption={obj}
+        />);
 
-      jest.runAllTimers(); // it is necessary to test that setGuard is not called
+        jest.runAllTimers(); // it is necessary to test that setGuard is not called
 
-      expect(Widget.option).toHaveBeenCalledTimes(1);
-      expect(Widget.option).toHaveBeenCalledWith('controlledOption', 'changed');
+        expect(Widget.option).toHaveBeenCalledTimes(2);
+        expect(Widget.option).toHaveBeenCalledWith('controlledOption', 'changed');
+        expect(Widget.option).toHaveBeenCalledWith('complexOption', obj);
+      } finally {
+        Widget.endUpdate.mockRestore();
+      }
     });
 
     it('is not updated on other prop updating', () => {
@@ -542,8 +554,42 @@ describe('option control', () => {
     );
 
     jest.runAllTimers();
+    expect(Widget.option.mock.calls.length).toBe(0);
+  });
+
+  it('applies option changed twice', () => {
+    const { rerender } = render(
+      <ControlledComponent controlledOption='a' />,
+    );
+
+    fireOptionChange('controlledOption', 'b');
+    fireOptionChange('controlledOption', 'c');
+
+    rerender(
+      <ControlledComponent controlledOption='c' />,
+    );
+
+    jest.runAllTimers();
+
+    expect(Widget.option.mock.calls.length).toBe(0);
+  });
+
+  it('rolls back an option changed one extra time', () => {
+    const { rerender } = render(
+      <ControlledComponent controlledOption='a' />,
+    );
+
+    fireOptionChange('controlledOption', 'b');
+    fireOptionChange('controlledOption', 'c');
+
+    rerender(
+      <ControlledComponent controlledOption='b' />,
+    );
+
+    jest.runAllTimers();
+
     expect(Widget.option.mock.calls.length).toBe(1);
-    expect(Widget.option.mock.calls[0]).toEqual(['everyOption', 234]);
+    expect(Widget.option.mock.calls[0]).toEqual(['controlledOption', 'b']);
   });
 
   it('applies option change with async React 18+ update', () => {
@@ -561,8 +607,7 @@ describe('option control', () => {
     );
 
     jest.runAllTimers();
-    expect(Widget.option.mock.calls.length).toBe(1);
-    expect(Widget.option.mock.calls[0]).toEqual(['everyOption', 234]);
+    expect(Widget.option.mock.calls.length).toBe(0);
   });
 
   it('applies complex option change', () => {
@@ -939,8 +984,7 @@ describe('cfg-component option control', () => {
     rerender(<TestContainer value={234} />);
 
     jest.runAllTimers();
-    expect(Widget.option.mock.calls.length).toBe(1);
-    expect(Widget.option.mock.calls[0]).toEqual(['nestedOption.a', 234]);
+    expect(Widget.option.mock.calls.length).toBe(0);
   });
 
   it('apply cfg-component option change if value really change (option in custom configuration component)', () => {
@@ -961,8 +1005,7 @@ describe('cfg-component option control', () => {
     rerender(<TestContainer value={234} />);
 
     jest.runAllTimers();
-    expect(Widget.option.mock.calls.length).toBe(1);
-    expect(Widget.option.mock.calls[0]).toEqual(['nestedOption.a', 234]);
+    expect(Widget.option.mock.calls.length).toBe(0);
   });
 
   it('does not control not specified cfg-component option', () => {

packages/devextreme-react/src/core/configuration/comparer.ts

@@ -10,6 +10,13 @@ interface IConfigChanges {
     currentOptions: Record<string, any>, prevOptions: Record<string, any>, path: string) => void;
 }
 
+function compareValues(
+  value1: unknown,
+  value2: unknown,
+): boolean {
+  return value1 === value2;
+}
+
 function compareTemplates(
   current: IConfigNode,
   currentFullName: string,
@@ -29,7 +36,7 @@ function compareTemplates(
   // appendRemovedValues(currentTemplates, prevTemplates, "", changesAccum.templates);
 
   Object.keys(currentTemplatesOptions).forEach((key) => {
-    if (currentTemplatesOptions[key] === prevTemplatesOptions[key]) {
+    if (compareValues(currentTemplatesOptions[key], prevTemplatesOptions[key])) {
       return;
     }
 
@@ -39,7 +46,7 @@ function compareTemplates(
   Object.keys(currentTemplates).forEach((key) => {
     const currentTemplate = currentTemplates[key];
     const prevTemplate = prevTemplates[key];
-    if (prevTemplate && currentTemplate.content === prevTemplate.content) {
+    if (prevTemplate && compareValues(currentTemplate.content, prevTemplate.content)) {
       return;
     }
 
@@ -75,7 +82,7 @@ function compare(current: IConfigNode, prev: IConfigNode, changesAccum: IConfigC
   });
 
   Object.keys(current.options).forEach((key) => {
-    if (current.options[key] === prev.options[key]) {
+    if (compareValues(current.options[key], prev.options[key])) {
       return;
     }
 
@@ -142,4 +149,5 @@ function compareCollections(
 
 export {
   getChanges,
+  compareValues,
 };

packages/devextreme-react/src/core/options-manager.ts

@@ -1,10 +1,10 @@
 /* eslint-disable no-restricted-globals */
-import { getChanges } from './configuration/comparer';
+import { compareValues, getChanges } from './configuration/comparer';
 import { buildConfig, findValue, ValueType } from './configuration/tree';
 import { mergeNameParts, shallowEquals } from './configuration/utils';
 import { capitalizeFirstLetter } from './helpers';
 import type { IConfigNode, ITemplate } from './configuration/config-node';
-import { DXTemplateCollection } from './types';
+import { DXTemplateCollection, GuardObject } from './types';
 
 const optionsManagers = new Set<OptionsManager>();
 let guardTimeoutHandler = -1;
@@ -24,7 +24,7 @@ export function scheduleGuards(): void {
 }
 
 class OptionsManager {
-  private readonly guards: Record<string, () => void> = {};
+  private readonly guards: Record<string, GuardObject> = {};
 
   private instance: any;
 
@@ -137,17 +137,22 @@ class OptionsManager {
     const { value, type } = valueDescriptor;
     if (value instanceof Array && type === ValueType.Array) {
       for (let i = 0; i < value.length; i += 1) {
-        if (value[i] !== (e.value as unknown[])?.[i]) {
-          this.addGuard(e.fullName, value);
+        const newValue = (e.value as unknown[])?.[i];
+
+        if (value[i] !== newValue) {
+          this.addGuard(e.fullName, value, newValue);
           return;
         }
       }
     } else if (type === ValueType.Complex && value instanceof Object) {
       Object.keys(value).forEach((key) => {
-        if (value[key] === (e.value as Record<string, unknown>)?.[key]) {
+        const newValue = (e.value as Record<string, unknown>)?.[key];
+
+        if (value[key] === newValue) {
           return;
         }
-        this.addGuard(mergeNameParts(e.fullName, key), value[key]);
+
+        this.addGuard(mergeNameParts(e.fullName, key), value[key], newValue);
       });
     } else {
       const valuesAreEqual = value === e.value;
@@ -160,7 +165,7 @@ class OptionsManager {
         return;
       }
 
-      this.addGuard(e.fullName, value);
+      this.addGuard(e.fullName, value, e.value);
     }
   }
 
@@ -225,21 +230,25 @@ class OptionsManager {
     return value;
   }
 
-  private addGuard(optionName: string, optionValue: unknown): void {
+  private addGuard(optionName: string, initialValue: unknown, latestValue: unknown): void {
     if (this.guards[optionName] !== undefined) {
+      this.guards[optionName].latestValue = latestValue;
       return;
     }
     const handler = () => {
-      this.setValue(optionName, optionValue);
+      this.setValue(optionName, initialValue);
       delete this.guards[optionName];
     };
-    this.guards[optionName] = handler;
+    this.guards[optionName] = {
+      handler,
+      latestValue,
+    };
     scheduleGuards();
   }
 
   public execGuards(): void {
     Object.values(this.guards)
-      .forEach((handler) => handler());
+      .forEach((guard) => guard.handler());
   }
 
   private resetOption(name: string) {
@@ -257,7 +266,13 @@ class OptionsManager {
 
   private setValue(name: string, value: unknown) {
     if (this.guards[name]) {
-      delete this.guards[name];
+      try {
+        if (compareValues(this.guards[name].latestValue, value)) {
+          return;
+        }
+      } finally {
+        delete this.guards[name];
+      }
     }
 
     this.instance.option(

packages/devextreme-react/src/core/types.ts

@@ -26,6 +26,11 @@ export interface RenderArgs {
 
 export type DXTemplateCollection = Record<string, DXTemplate>;
 
+export interface GuardObject {
+  handler: () => void;
+  latestValue: unknown;
+}
+
 export interface TemplateWrapperProps {
   templateFactory: TemplateFunc;
   data: any;