From 11ccb2ef59ce4be383547abd08771a259f2c452b Mon Sep 17 00:00:00 2001 From: Denys Oblohin Date: Tue, 4 Apr 2023 19:25:59 +0300 Subject: [PATCH] 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 d516a435dd231dbc64b444ed2ebda81e332bcc77. * Revert "react 18 + strict" This reverts commit 6ec9facd3d7611fc866e815050332caa26a93fe4. * chlog * lint fix --- CHANGELOG.md | 1 + .../antd/modules/widgets/value/Slider.jsx | 17 +++++++++----- packages/core/modules/stores/tree.js | 6 ++++- packages/core/modules/utils/validation.js | 4 ++-- packages/ui/modules/components/Builder.jsx | 3 ++- .../ui/modules/components/QueryContainer.jsx | 23 +++++++++++++------ .../components/containers/GroupContainer.jsx | 3 ++- .../components/containers/RuleContainer.jsx | 3 ++- .../containers/SortableContainer.jsx | 3 ++- .../hooks/useListValuesAutocomplete.jsx | 16 ++++++------- packages/ui/modules/utils/validation.js | 8 +++++-- 11 files changed, 56 insertions(+), 31 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0a7d22333..448f947bd 100644 --- 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 index 78d7b3fe0..9cf026c52 100644 --- 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 index f88868c04..4ab1a15c0 100644 --- 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 index 48a0a2d3b..fdaf18025 100644 --- 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 index 123abe83f..628a5d9fa 100644 --- 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 index d1d32eca0..7764a67e8 100644 --- 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 index ad765ba5f..681c6ae57 100644 --- 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 index aa3be1f64..fffc2835f 100644 --- 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 index ce7082084..bc7f8bbc2 100644 --- 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 index 4e78c5b1f..2ddf9565a 100644 --- 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 index 7a8498073..59a7f17ab 100644 --- 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; } };