fix: move updateOpentKeysWhenSwitchMode to getDerivedStateFromProps hook (#346)

Author: kerm1it

src/Menu.tsx

@@ -68,6 +68,9 @@ export interface MenuProps
 
 export interface MenuState {
   switchingModeFromInline: boolean;
+  prevProps: MenuProps;
+  inlineOpenKeys: Array<string>;
+  store: MiniStore;
 }
 
 class Menu extends React.Component<MenuProps, MenuState> {
@@ -112,6 +115,9 @@ class Menu extends React.Component<MenuProps, MenuState> {
 
     this.state = {
       switchingModeFromInline: false,
+      prevProps: props,
+      inlineOpenKeys: [],
+      store: this.store,
     };
   }
 
@@ -121,64 +127,64 @@ class Menu extends React.Component<MenuProps, MenuState> {
 
   innerMenu: typeof SubPopupMenu;
 
-  inlineOpenKeys: string[] = [];
-
   prevOpenKeys: string[];
 
-  componentDidMount() {
-    this.updateMiniStore();
-    this.updateMenuDisplay();
-  }
-
-  componentDidUpdate(prevProps: MenuProps) {
-    this.updateOpentKeysWhenSwitchMode(prevProps);
-    this.updateMiniStore();
-    const { siderCollapsed, inlineCollapsed, onOpenChange } = this.props;
-    if (
-      (!prevProps.inlineCollapsed && inlineCollapsed) ||
-      (!prevProps.siderCollapsed && siderCollapsed)
-    ) {
-      onOpenChange([]);
-    }
-    this.updateMenuDisplay();
-  }
-
-  updateOpentKeysWhenSwitchMode(prevProps: MenuProps) {
-    const { props: nextProps, store, inlineOpenKeys } = this;
-    const prevState = store.getState();
-    const newState: any = {};
+  static getDerivedStateFromProps(nextProps: MenuProps, prevState: MenuState) {
+    const { prevProps, store } = prevState;
+    const prevStoreState = store.getState();
+    const newStoreState: any = {};
+    const newState: Partial<MenuState> = {
+      prevProps: nextProps,
+    };
     if (prevProps.mode === 'inline' && nextProps.mode !== 'inline') {
-      this.setState({
-        switchingModeFromInline: true,
-      });
+      newState.switchingModeFromInline = true;
     }
 
-    if (!('openKeys' in nextProps)) {
+    if ('openKeys' in nextProps) {
+      newStoreState.openKeys = nextProps.openKeys;
+    } else {
       // [Legacy] Old code will return after `openKeys` changed.
       // Not sure the reason, we should keep this logic still.
       if (
         (nextProps.inlineCollapsed && !prevProps.inlineCollapsed) ||
         (nextProps.siderCollapsed && !prevProps.siderCollapsed)
       ) {
-        this.setState({
-          switchingModeFromInline: true,
-        });
-        this.inlineOpenKeys = prevState.openKeys.concat();
-        newState.openKeys = [];
+        newState.switchingModeFromInline = true;
+        newState.inlineOpenKeys = prevStoreState.openKeys;
+        newStoreState.openKeys = [];
       }
 
       if (
         (!nextProps.inlineCollapsed && prevProps.inlineCollapsed) ||
         (!nextProps.siderCollapsed && prevProps.siderCollapsed)
       ) {
-        newState.openKeys = inlineOpenKeys;
-        this.inlineOpenKeys = [];
+        newStoreState.openKeys = prevState.inlineOpenKeys;
+        newState.inlineOpenKeys = [];
       }
     }
 
-    if (Object.keys(newState).length) {
-      store.setState(newState);
+    if (Object.keys(newStoreState).length) {
+      store.setState(newStoreState);
     }
+
+    return newState;
+  }
+
+  componentDidMount() {
+    this.updateMiniStore();
+    this.updateMenuDisplay();
+  }
+
+  componentDidUpdate(prevProps: MenuProps) {
+    const { siderCollapsed, inlineCollapsed, onOpenChange } = this.props;
+    if (
+      (!prevProps.inlineCollapsed && inlineCollapsed) ||
+      (!prevProps.siderCollapsed && siderCollapsed)
+    ) {
+      onOpenChange([]);
+    }
+    this.updateMiniStore();
+    this.updateMenuDisplay();
   }
 
   updateMenuDisplay() {

tests/Menu.spec.js

@@ -823,7 +823,7 @@ describe('Menu', () => {
           .at(0)
           .hasClass('rc-menu-vertical'),
       ).toBe(true);
-      expect(wrapper.find('ul.rc-menu-sub.rc-menu-hidden').length).toBe(1);
+      expect(wrapper.find('ul.rc-menu-sub').length).toBe(0);
 
       wrapper.setProps({ inlineCollapsed: false });
       jest.runAllTimers();