Fix bugs with React.StrictMode (#902)

* react 18 + strict * remove UNSAFE_componentWillUpdate * setLastTree . * this.pureShouldComponentUpdate * componentIsMounted is int to fix strict mode with unmount * demo #789 * fix slider shaking * Revert "demo #789" This reverts commit d516a43. * Revert "react 18 + strict" This reverts commit 6ec9fac. * chlog * lint fix
ukrbublik · Apr 4, 2023 · 11ccb2e · 11ccb2e · vercel · Apr 4, 2023
1 parent 19d9f92
commit 11ccb2e
Show file tree

Hide file tree

Showing 11 changed files with 56 additions and 31 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,6 +3,7 @@
   - Fix isNull roundtrip (PR #887) (issue #886)
   - Fix `BootstrapFieldSelect` for fields with 2+ level nesting (PR #898) (issue #868)
   - Fix `UNSAFE_componentWillReceiveProps` (PR #901) (issue #390)
+  - Fix bugs with `React.StrictMode` (PR #902) (issue #789)
 - 6.1.2
   - Fix typings for `children1`: tuple -> array (PR #885) (issue #881)
   - Fix compare fields for case when type != widget.type (PR #875) (issue #758)

diff --git a/packages/antd/modules/widgets/value/Slider.jsx b/packages/antd/modules/widgets/value/Slider.jsx
@@ -2,7 +2,7 @@ import React, { Component } from "react";
 import PropTypes from "prop-types";
 import { Slider, InputNumber, Col } from "antd";
 import { Utils } from "@react-awesome-query-builder/ui";
-const { useOnPropsChanged } = Utils.ReactUtils;
+const { useOnPropsChanged, pureShouldComponentUpdate } = Utils.ReactUtils;
 const __isInternal = true; //true to optimize render
 
 export default class SliderWidget extends Component {
@@ -34,6 +34,7 @@ export default class SliderWidget extends Component {
 
   constructor(props) {
     super(props);
+    this.pureShouldComponentUpdate = pureShouldComponentUpdate(this);
     useOnPropsChanged(this);
 
     this.state.internalValue = props.value;
@@ -53,12 +54,16 @@ export default class SliderWidget extends Component {
 
   tipFormatter = (val) => (val != undefined ? val.toString() : undefined);
 
-  UNSAFE_componentWillUpdate(nextProps, nextState) {
-    // RHL fix
-    if (this.props.cacheBusterProp && __isInternal) {
-      nextState.internalValue = this.state.internalValue;
+  shouldComponentUpdate = (nextProps, nextState) => {
+    const should = this.pureShouldComponentUpdate(nextProps, nextState);
+    if (should) {
+      // RHL fix
+      if (this.props.cacheBusterProp && __isInternal) {
+        nextState.internalValue = this.state.internalValue;
+      }
     }
-  }
+    return should;
+  };
 
   render() {
     const {config, placeholder, customProps, value,  min, max, step, marks, readonly, valueError} = this.props;

diff --git a/packages/core/modules/stores/tree.js b/packages/core/modules/stores/tree.js
@@ -704,7 +704,7 @@ const getActionMeta = (action, state) => {
  * @param {Immutable.Map} state
  * @param {object} action
  */
-export default (config, tree, getMemoizedTree) => {
+export default (config, tree, getMemoizedTree, setLastTree) => {
   const emptyTree = defaultRoot(config);
   const initTree = tree || emptyTree;
   const emptyState = {
@@ -825,6 +825,10 @@ export default (config, tree, getMemoizedTree) => {
     if (actionMeta) {
       set.__lastAction = actionMeta;
     }
+
+    if (setLastTree && set.tree && state.tree) {
+      setLastTree(state.tree);
+    }
 
     return {...state, ...unset, ...set};
   };

diff --git a/packages/core/modules/utils/validation.js b/packages/core/modules/utils/validation.js
@@ -25,8 +25,8 @@ const isTypeOf = (v, type) => {
   return false;
 };
 
-export const validateAndFixTree = (newTree, _oldTree, newConfig, oldConfig) => {
-  let tree = validateTree(newTree, _oldTree, newConfig, oldConfig);
+export const validateAndFixTree = (newTree, _oldTree, newConfig, oldConfig, removeEmptyGroups, removeIncompleteRules) => {
+  let tree = validateTree(newTree, _oldTree, newConfig, oldConfig, removeEmptyGroups, removeIncompleteRules);
   tree = fixPathsInTree(tree);
   return tree;
 };

diff --git a/packages/ui/modules/components/Builder.jsx b/packages/ui/modules/components/Builder.jsx
@@ -17,7 +17,7 @@ class Builder extends Component {
 
   shouldComponentUpdate(nextProps, nextState) {
     const prevProps = this.props;
-    let should = pureShouldComponentUpdate(this)(nextProps, nextState);
+    let should = this.pureShouldComponentUpdate(nextProps, nextState);
     if (should) {
       let chs = [];
       for (let k in nextProps) {
@@ -37,6 +37,7 @@ class Builder extends Component {
 
   constructor(props) {
     super(props);
+    this.pureShouldComponentUpdate = pureShouldComponentUpdate(this);
 
     this._updPath(props);
   }

diff --git a/packages/ui/modules/components/QueryContainer.jsx b/packages/ui/modules/components/QueryContainer.jsx
@@ -40,7 +40,7 @@ export default class QueryContainer extends Component {
     const tree = props.value;
     const validatedTree = this.getMemoizedTree(config, tree);
 
-    const reducer = treeStoreReducer(config, validatedTree, this.getMemoizedTree);
+    const reducer = treeStoreReducer(config, validatedTree, this.getMemoizedTree, this.setLastTree);
     const store = createStore(reducer);
 
     this.config = config;
@@ -49,6 +49,13 @@ export default class QueryContainer extends Component {
     };
   }
 
+  setLastTree = (lastTree) => {
+    if (this.prevTree) {
+      this.prevprevTree = this.prevTree;
+    }
+    this.prevTree = lastTree;
+  };
+
   shouldComponentUpdate = liteShouldComponentUpdate(this, {
     value: (nextValue, prevValue, state) => { return false; }
   });
@@ -63,18 +70,20 @@ export default class QueryContainer extends Component {
     const storeValue = this.state.store.getState().tree;
     const isTreeChanged = !immutableEqual(nextProps.value, this.props.value) && !immutableEqual(nextProps.value, storeValue);
     const currentTree = isTreeChanged ? nextProps.value || defaultRoot(nextProps) : storeValue;
+    const isTreeTrulyChanged = isTreeChanged && !immutableEqual(nextProps.value, this.prevTree) && !immutableEqual(nextProps.value, this.prevprevTree);
+    const sanitizeTree = isTreeTrulyChanged || isConfigChanged;
 
     if (isConfigChanged) {
       this.config = nextConfig;
     }
 
     if (isTreeChanged || isConfigChanged) {
-      const validatedTree = this.getMemoizedTree(nextConfig, currentTree, oldConfig);
-      return Promise.resolve().then(() => {
-        this.state.store.dispatch(
-          actions.tree.setTree(nextConfig, validatedTree)
-        );
-      });
+      const validatedTree = this.getMemoizedTree(nextConfig, currentTree, oldConfig, sanitizeTree);
+      //return Promise.resolve().then(() => {
+      this.state.store.dispatch(
+        actions.tree.setTree(nextConfig, validatedTree)
+      );
+      //});
     }
   }
 

diff --git a/packages/ui/modules/components/containers/GroupContainer.jsx b/packages/ui/modules/components/containers/GroupContainer.jsx
@@ -33,6 +33,7 @@ const createGroupContainer = (Group) =>
 
     constructor(props) {
       super(props);
+      this.pureShouldComponentUpdate = pureShouldComponentUpdate(this);
       useOnPropsChanged(this);
 
       this.selectedConjunction = this._selectedConjunction(props);
@@ -44,7 +45,7 @@ const createGroupContainer = (Group) =>
       let prevProps = this.props;
       let prevState = this.state;
 
-      let should = pureShouldComponentUpdate(this)(nextProps, nextState);
+      let should = this.pureShouldComponentUpdate(nextProps, nextState);
       if (should) {
         if (prevState == nextState && prevProps != nextProps) {
           const draggingId = (nextProps.dragging.id || prevProps.dragging.id);

diff --git a/packages/ui/modules/components/containers/RuleContainer.jsx b/packages/ui/modules/components/containers/RuleContainer.jsx
@@ -35,6 +35,7 @@ const createRuleContainer = (Rule) =>
 
     constructor(props) {
       super(props);
+      this.pureShouldComponentUpdate = pureShouldComponentUpdate(this);
 
       this.dummyFn.isDummyFn = true;
     }
@@ -73,7 +74,7 @@ const createRuleContainer = (Rule) =>
       let prevProps = this.props;
       let prevState = this.state;
 
-      let should = pureShouldComponentUpdate(this)(nextProps, nextState);
+      let should = this.pureShouldComponentUpdate(nextProps, nextState);
       if (should) {
         if (prevState == nextState && prevProps != nextProps) {
           const draggingId = (nextProps.dragging.id || prevProps.dragging.id);

diff --git a/packages/ui/modules/components/containers/SortableContainer.jsx b/packages/ui/modules/components/containers/SortableContainer.jsx
@@ -22,6 +22,7 @@ const createSortableContainer = (Builder, CanMoveFn = null) =>
 
     constructor(props) {
       super(props);
+      this.pureShouldComponentUpdate = pureShouldComponentUpdate(this);
       useOnPropsChanged(this);
 
       this.onPropsChanged(props);
@@ -35,7 +36,7 @@ const createSortableContainer = (Builder, CanMoveFn = null) =>
       let prevProps = this.props;
       let prevState = this.state;
 
-      let should = pureShouldComponentUpdate(this)(nextProps, nextState);
+      let should = this.pureShouldComponentUpdate(nextProps, nextState);
       if (should) {
         if (prevState == nextState && prevProps != nextProps) {
           let chs = [];

diff --git a/packages/ui/modules/hooks/useListValuesAutocomplete.jsx b/packages/ui/modules/hooks/useListValuesAutocomplete.jsx
@@ -29,7 +29,7 @@ const useListValuesAutocomplete = ({
 
   // ref
   const asyncFectchCnt = React.useRef(0);
-  const componentIsMounted = React.useRef(true);
+  const componentIsMounted = React.useRef(0);
   const isSelectedLoadMore = React.useRef(false);
 
   // compute
@@ -112,20 +112,18 @@ const useListValuesAutocomplete = ({
   };
   const loadListValuesDebounced = React.useCallback(debounce(loadListValues, debounceTimeout), []);
 
-  // Unmount
-  React.useEffect(() => {
-    return () => {
-      componentIsMounted.current = false;
-    };
-  }, []);
-
-  // Initial loading
   React.useEffect(() => {
+    componentIsMounted.current++;
+    // Initial loading
     if (canInitialLoad && loadingCnt == 0 && asyncFectchCnt.current == 0) {
       (async () => {
         await loadListValues();
       })();
     }
+    // Unmount
+    return () => {
+      componentIsMounted.current--;
+    };
   }, [canInitialLoad]);
 
   // Event handlers

diff --git a/packages/ui/modules/utils/validation.js b/packages/ui/modules/utils/validation.js
@@ -7,7 +7,7 @@ export const createValidationMemo = () => {
   let validatedTree;
   let configId;
 
-  return (config, tree, oldConfig) => {
+  return (config, tree, oldConfig = undefined, sanitizeTree = true) => {
     if (!tree) {
       return null;
     }
@@ -16,7 +16,11 @@ export const createValidationMemo = () => {
     } else {
       configId = config.__configId;
       originalTree = tree;
-      validatedTree = validateAndFixTree(tree, null, config, oldConfig || config);
+      if (sanitizeTree === false) {
+        validatedTree = validateAndFixTree(tree, null, config, oldConfig || config, false, false);
+      } else {
+        validatedTree = validateAndFixTree(tree, null, config, oldConfig || config);
+      }
       return validatedTree;
     }
   };