T1106899 - Toolbar - An item's content is not updated according to the React state if it is nested in a DataGrid component instance in React 18 (#751) (#752)

* Fix T1106899
In new react 18 guards call early then a switch is rendered. Move schedule guard timeout after the switch is rendered.

Author: MikeVitik

.eslintrc

@@ -1,6 +1,7 @@
 {
     "extends": [
-        "devextreme/typescript"
+        "devextreme/typescript",
+        "devextreme/spell-check"
     ],
     "env": {
         "browser": true,
@@ -15,14 +16,30 @@
         "import/no-cycle": "warn",
         "no-param-reassign": "warn",
         "no-underscore-dangle": "warn",
-        "@typescript-eslint/no-explicit-any": "warn"
+        "@typescript-eslint/no-explicit-any": "warn",
+        "spellcheck/spell-checker": [1, {
+            "lang": "en_US",
+            "comments": false,
+            "strings": false,
+            "identifiers": true,
+            "templates": false,
+            "skipIfMatch": [ "^\\$?..$" ],
+            "skipWords": [
+               "unschedule",
+               "subscribable",
+               "renderer",
+               "rerender"
+            ]
+        }
+    ]
     },
     "overrides": [
         {
-            "files": [ "./packages/sandbox/*.ts", "./packages/sandbox/*.tsx" ],
+            "files": [ "./packages/sandbox/*.ts", "./packages/sandbox/*.tsx", "packages/devextreme-react/src/*.ts" ],
             "rules": {
-                "import/extensions": "warn"
+                "import/extensions": "warn",
             }
         }
     ]
 }
+

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

@@ -1,15 +1,18 @@
 /* eslint-disable max-classes-per-file */
 import { cleanup, render } from '@testing-library/react';
 import * as React from 'react';
+import * as CommonModule from 'devextreme/core/utils/common';
 import ConfigurationComponent from '../nested-option';
-import { OptionsManager } from '../options-manager';
+import * as OptionsManagerModule from '../options-manager';
 import {
   eventHandlers,
   fireOptionChange,
   TestComponent,
   Widget,
   WidgetClass,
 } from './test-component';
+import TemplatesManager from '../templates-manager';
+import { TemplatesStore } from '../templates-store';
 
 jest.useFakeTimers();
 
@@ -592,6 +595,69 @@ describe('cfg-component option control', () => {
     expect(Widget.option.mock.calls[1]).toEqual(['nestedOption.b', 'const']);
   });
 
+  // T1106899
+  it('apply cfg-component option value if value has changes', () => {
+    const optionsManager = new OptionsManagerModule.OptionsManager(
+      new TemplatesManager(new TemplatesStore(() => {})),
+    );
+    const config = {
+      fullName: '',
+      predefinedOptions: {},
+      initialOptions: {},
+      options: { value: 1 },
+      templates: [],
+      configs: {},
+      configCollections: {},
+    };
+    optionsManager.setInstance({
+      skipOptionsRollBack: false,
+      option: jest.fn(),
+      on: jest.fn(),
+      off: jest.fn(),
+      beginUpdate: jest.fn(),
+      endUpdate: jest.fn(),
+    }, config, [], []);
+    jest.spyOn(optionsManager as any, 'addGuard');
+    jest.spyOn(optionsManager as any, 'setValue');
+    jest.spyOn(OptionsManagerModule, 'scheduleGuards');
+    jest.spyOn(OptionsManagerModule, 'unscheduleGuards');
+    let renderTemplate;
+    jest.spyOn(CommonModule, 'deferUpdate').mockImplementation((cb) => { renderTemplate = cb; });
+    const TestContainer = (props: any) => {
+      const { value } = props;
+      return (
+        <ControlledComponent>
+          <NestedComponent a={value} b="const" />
+        </ControlledComponent>
+      );
+    };
+
+    const { rerender } = render(<TestContainer value={2} />);
+    jest.runAllTimers();
+    // simulate option changing in jQuery control
+    optionsManager.onOptionChanged({ name: 'value', value: 2, fullName: 'value' });
+    // add guards for restore value
+    expect((optionsManager as any).addGuard).toBeCalled();
+    // but no call it
+    expect((optionsManager as any).setValue).not.toBeCalled();
+    // re-render container. Unschedule guards and wait template render for schedule it back
+    rerender(<TestContainer value={2} />);
+    expect(OptionsManagerModule.scheduleGuards).not.toBeCalled();
+    expect(OptionsManagerModule.unscheduleGuards).toBeCalled();
+    expect((optionsManager as any).setValue).not.toBeCalled();
+    jest.runAllTimers();
+    // simulate Request Animation Frame for template re-render
+    renderTemplate();
+    // guards are scheduled
+    expect(OptionsManagerModule.scheduleGuards).toBeCalled();
+    const updatedConfig = { ...config, options: { value: 2 } };
+    // value changed and options manager set value and remove scheduled guard
+    optionsManager.update(updatedConfig);
+    expect((optionsManager as any).setValue).toBeCalled();
+    jest.runAllTimers();
+    expect((optionsManager as any).setValue).toHaveBeenCalledTimes(1);
+  });
+
   it('apply cfg-component option change if value really change', () => {
     const TestContainer = (props: any) => {
       const { value } = props;
@@ -805,7 +871,8 @@ describe('onXXXChange', () => {
 
     beforeAll(() => {
       jest.spyOn(
-        OptionsManager.prototype as OptionsManager & { isOptionSubscribable: () => boolean; },
+        OptionsManagerModule.OptionsManager.prototype as
+        OptionsManagerModule.OptionsManager & { isOptionSubscribable: () => boolean; },
         'isOptionSubscribable',
       )
         .mockImplementation(() => true);
@@ -1040,7 +1107,8 @@ describe('onXXXChange', () => {
   describe('non-subscribable options', () => {
     beforeAll(() => {
       jest.spyOn(
-        OptionsManager.prototype as OptionsManager & { isOptionSubscribable: () => boolean; },
+        OptionsManagerModule.OptionsManager.prototype as
+        OptionsManagerModule.OptionsManager & { isOptionSubscribable: () => boolean; },
         'isOptionSubscribable',
       )
         .mockImplementation(() => false);
@@ -1080,7 +1148,8 @@ describe('onXXXChange', () => {
   describe('independent events', () => {
     beforeAll(() => {
       jest.spyOn(
-        OptionsManager.prototype as OptionsManager & { isIndependentEvent: () => boolean; },
+        OptionsManagerModule.OptionsManager.prototype as
+        OptionsManagerModule.OptionsManager & { isIndependentEvent: () => boolean; },
         'isIndependentEvent',
       )
         .mockImplementation(() => true);
@@ -1136,7 +1205,8 @@ describe('onXXXChange', () => {
   describe('dependent events', () => {
     beforeAll(() => {
       jest.spyOn(
-        OptionsManager.prototype as OptionsManager & { isIndependentEvent: () => boolean; },
+        OptionsManagerModule.OptionsManager.prototype as
+        OptionsManagerModule.OptionsManager & { isIndependentEvent: () => boolean; },
         'isIndependentEvent',
       )
         .mockImplementation(() => false);

packages/devextreme-react/src/core/__tests__/templates-renderer.test.tsx

@@ -90,7 +90,7 @@ jest.mock('devextreme/core/utils/common', () => ({
   });
 });
 
-fdescribe('option update', () => {
+describe('option update', () => {
   it('should call forceUpdateCallback and reset "updateScheduled" after forceUpdateCallback', async () => {
     const ref = React.createRef<TemplatesRenderer>();
     const templatesStore = new TemplatesStore(() => { });
@@ -100,19 +100,17 @@ fdescribe('option update', () => {
     });
 
     render(<TemplatesRenderer templatesStore={templatesStore} ref={ref} />);
-
     const current = ref.current!;
-    const actualForceUpdateCallback = current.forceUpdateCallback;
-    const spyForceUpdateCallback = jest.spyOn(current, 'forceUpdateCallback').mockImplementation(() => {
+    const onRendered = jest.fn();
+    const spyForceUpdateCallback = jest.spyOn(current, 'forceUpdate').mockImplementation((cb) => {
       expect((current as any).updateScheduled).toEqual(true);
-
-      actualForceUpdateCallback();
+      expect(onRendered).not.toHaveBeenCalled();
+      cb();
+      expect(onRendered).toHaveBeenCalled();
 
       expect((current as any).updateScheduled).toEqual(false);
     });
-
-    current.scheduleUpdate(true);
-
+    current.scheduleUpdate(true, onRendered);
     expect(spyForceUpdateCallback).toHaveBeenCalledTimes(1);
   });
 });

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

@@ -2,11 +2,12 @@ import * as events from 'devextreme/events';
 import * as React from 'react';
 import { createPortal } from 'react-dom';
 
-import { OptionsManager } from './options-manager';
-import { ITemplateMeta } from './template';
 import TemplatesManager from './templates-manager';
 import { TemplatesRenderer } from './templates-renderer';
 import { TemplatesStore } from './templates-store';
+
+import { OptionsManager, scheduleGuards, unscheduleGuards } from './options-manager';
+import { ITemplateMeta } from './template';
 import { elementPropNames, getClassName } from './widget-config';
 
 import { IConfigNode } from './configuration/config-node';
@@ -94,12 +95,12 @@ abstract class ComponentBase<P extends IHtmlOptions> extends React.PureComponent
 
   public componentDidUpdate(prevProps: P): void {
     this._updateCssClasses(prevProps, this.props);
-
     const config = this._getConfig();
     this._optionsManager.update(config);
     if (this._templatesRendererRef) {
-      this._templatesRendererRef.scheduleUpdate(this.useDeferUpdateForTemplates);
+      this._templatesRendererRef.scheduleUpdate(this.useDeferUpdateForTemplates, scheduleGuards);
     }
+    unscheduleGuards();
   }
 
   public componentWillUnmount(): void {

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

@@ -1,13 +1,27 @@
-import TemplatesManager from './templates-manager';
-
+/* eslint-disable no-restricted-globals */
 import { getChanges } from './configuration/comparer';
-import { IConfigNode } from './configuration/config-node';
 import { buildConfig, findValue, ValueType } from './configuration/tree';
 import { mergeNameParts, shallowEquals } from './configuration/utils';
 import { capitalizeFirstLetter } from './helpers';
 
+import type TemplatesManager from './templates-manager';
+import type { IConfigNode } from './configuration/config-node';
+
+const optionsManagers = new Set<OptionsManager>();
+let guardTimeoutHandler = -1;
+
+export function unscheduleGuards(): void {
+  clearTimeout(guardTimeoutHandler);
+}
+export function scheduleGuards(): void {
+  unscheduleGuards();
+  guardTimeoutHandler = window.setTimeout(() => {
+    optionsManagers.forEach((optionManager) => optionManager.execGuards());
+  });
+}
+
 class OptionsManager {
-  private readonly guards: Record<string, number> = {};
+  private readonly guards: Record<string, ()=> void> = {};
 
   private templatesManager: TemplatesManager;
 
@@ -23,6 +37,7 @@ class OptionsManager {
 
   constructor(templatesManager: TemplatesManager) {
     this.templatesManager = templatesManager;
+    optionsManagers.add(this);
 
     this.onOptionChanged = this.onOptionChanged.bind(this);
     this.wrapOptionValue = this.wrapOptionValue.bind(this);
@@ -62,7 +77,7 @@ class OptionsManager {
   }
 
   public update(config: IConfigNode): void {
-    const changedOptions:Array<[string, any]> = [];
+    const changedOptions:Array<[string, unknown]> = [];
     const optionChangedHandler = ({ value, fullName }) => {
       changedOptions.push([fullName, value]);
     };
@@ -103,7 +118,8 @@ class OptionsManager {
 
     changedOptions.forEach(([name, value]) => {
       const currentPropValue = config.options[name];
-      if (config.options.hasOwnProperty(name) && currentPropValue !== value) {
+      if (Object.prototype.hasOwnProperty.call(config.options, name)
+      && currentPropValue !== value) {
         this.setValue(name, currentPropValue);
       }
     });
@@ -129,7 +145,7 @@ class OptionsManager {
     if (value instanceof Array && type === ValueType.Array) {
       for (let i = 0; i < value.length; i += 1) {
         if (value[i] !== (e.value as Array<unknown>)?.[i]) {
-          this.setGuard(e.fullName, value);
+          this.addGuard(e.fullName, value);
           return;
         }
       }
@@ -138,7 +154,7 @@ class OptionsManager {
         if (value[key] === (e.value as Record<string, unknown>)?.[key]) {
           return;
         }
-        this.setGuard(mergeNameParts(e.fullName, key), value[key]);
+        this.addGuard(mergeNameParts(e.fullName, key), value[key]);
       });
     } else {
       const valuesAreEqual = value === e.value;
@@ -151,13 +167,13 @@ class OptionsManager {
         return;
       }
 
-      this.setGuard(e.fullName, value);
+      this.addGuard(e.fullName, value);
     }
   }
 
   public dispose(): void {
+    optionsManagers.delete(this);
     Object.keys(this.guards).forEach((optionName) => {
-      window.clearTimeout(this.guards[optionName]);
       delete this.guards[optionName];
     });
   }
@@ -211,18 +227,21 @@ class OptionsManager {
     return value;
   }
 
-  private setGuard(optionName: string, optionValue: unknown): void {
+  private addGuard(optionName: string, optionValue: unknown): void {
     if (this.guards[optionName] !== undefined) {
       return;
     }
-
-    const guardId = window.setTimeout(() => {
+    const handler = () => {
       this.setValue(optionName, optionValue);
-      window.clearTimeout(guardId);
       delete this.guards[optionName];
-    });
+    };
+    this.guards[optionName] = handler;
+    scheduleGuards();
+  }
 
-    this.guards[optionName] = guardId;
+  public execGuards(): void {
+    Object.values(this.guards)
+      .forEach((handler) => handler());
   }
 
   private resetOption(name: string) {
@@ -231,7 +250,6 @@ class OptionsManager {
 
   private setValue(name: string, value: unknown) {
     if (this.guards[name]) {
-      window.clearTimeout(this.guards[name]);
       delete this.guards[name];
     }