Prevent setState call if Popover is unmounted

youzan · May 26, 2017 · b2e36a6 · b2e36a6
1 parent c52048a
commit b2e36a6
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 17 deletions.
diff --git a/.gitignore b/.gitignore
@@ -4,6 +4,7 @@ lib/
 node_modules/
 node_modules.tar
 *.log
+npm-debug.log*
 coverage
 packages/*/coverage
 .vscode/

diff --git a/packages/zent/__tests__/popover.js b/packages/zent/__tests__/popover.js
@@ -66,11 +66,6 @@ describe('Popover', () => {
     expect(
       document.querySelectorAll('.zent-popover-content div')[1].textContent
     ).toBe('line two');
-    simulateWithTimers(wrapper.find('button'), 'click');
-    expect(wrapper.find('Portal').length).toBe(1);
-    // NOTE: 只能直接调用close method，无法mock。
-    wrapper.getNode().close();
-    expect(wrapper.find('Portal').length).toBe(0);
 
     // HACK: branch window.resize (throttle)
     wrapper.find('PopoverContent').getNode().onWindowResize(
@@ -80,17 +75,19 @@ describe('Popover', () => {
         deltaY: 0
       }
     );
-    // HACK: throttle
-    setTimeout(() => {
-      wrapper.find('PopoverContent').getNode().onWindowResize(
-        {},
-        {
-          deltaX: 10,
-          deltaY: 10
-        }
-      );
-    }, 160);
-    jest.runAllTimers();
+    wrapper.find('PopoverContent').getNode().onWindowResize(
+      {},
+      {
+        deltaX: 10,
+        deltaY: 10
+      }
+    );
+
+    simulateWithTimers(wrapper.find('button'), 'click');
+    expect(wrapper.find('Portal').length).toBe(1);
+
+    wrapper.getNode().close();
+    expect(wrapper.find('Portal').length).toBe(0);
     wrapper.unmount();
 
     wrapper = mount(

diff --git a/packages/zent/src/popover/Popover.js b/packages/zent/src/popover/Popover.js
@@ -152,6 +152,8 @@ export default class Popover extends Component {
         visible: false
       };
     }
+
+    this.isUnmounted = false;
   }
 
   isVisibilityControlled(props) {
@@ -205,7 +207,7 @@ export default class Popover extends Component {
       }
 
       handleBeforeHook(onBefore, beforeHook.length, () => {
-        this.setState({ visible });
+        this.safeSetState({ visible });
         this.pendingOnBeforeHook = false;
       });
     }
@@ -286,6 +288,12 @@ export default class Popover extends Component {
     return { trigger, content };
   }
 
+  safeSetState(updater, callback) {
+    if (!this.isUnmounted) {
+      return this.setState(updater, callback);
+    }
+  }
+
   componentDidMount() {
     const { _zentPopover: popover } = this.context || {};
     if (popover && popover.registerDescendant) {
@@ -310,6 +318,8 @@ export default class Popover extends Component {
     if (popover && popover.unregisterDescendant) {
       popover.unregisterDescendant(this);
     }
+
+    this.isUnmounted = true;
   }
 
   render() {