fix(unmount): attempt to fix setState after unmount

Ref: #390
react-grid-layout · Apr 10, 2020 · b4f2e73 · b4f2e73
1 parent cdf8bee
commit b4f2e73
Show file tree

Hide file tree

Showing 4 changed files with 52 additions and 7 deletions.
diff --git a/karma.conf.js b/karma.conf.js
@@ -28,6 +28,7 @@ module.exports = function(config) {
         mode: 'production',
         // Remove source maps: *speeeeeed*
         devtool: 'none',
+        cache: true,
         module: {
           // Suppress power-assert warning
           exprContextCritical: false,

diff --git a/lib/Draggable.js b/lib/Draggable.js
@@ -193,6 +193,8 @@ class Draggable extends React.Component<DraggableProps, DraggableState> {
     return null;
   }
 
+  mounted = false;
+
   constructor(props: DraggableProps) {
     super(props);
 
@@ -225,13 +227,15 @@ class Draggable extends React.Component<DraggableProps, DraggableState> {
   }
 
   componentDidMount() {
+    this.mounted = true;
     // Check to see if the element passed is an instanceof SVGElement
     if(typeof window.SVGElement !== 'undefined' && ReactDOM.findDOMNode(this) instanceof window.SVGElement) {
       this.setState({isElementSVG: true});
     }
   }
 
   componentWillUnmount() {
+    this.mounted = false;
     this.setState({dragging: false}); // prevents invariant if unmounted while dragging
   }
 
@@ -295,8 +299,8 @@ class Draggable extends React.Component<DraggableProps, DraggableState> {
     if (!this.state.dragging) return false;
 
     // Short-circuit if user's callback killed it.
-    const shouldStop = this.props.onStop(e, createDraggableData(this, coreData));
-    if (shouldStop === false) return false;
+    const shouldContinue = this.props.onStop(e, createDraggableData(this, coreData));
+    if (shouldContinue === false) return false;
 
     log('Draggable: onDragStop: %j', coreData);
 

diff --git a/lib/DraggableCore.js b/lib/DraggableCore.js
@@ -222,7 +222,14 @@ export default class DraggableCore extends React.Component<DraggableCoreProps, D
     touchIdentifier: null
   };
 
+  mounted = false;
+
+  componentDidMount() {
+    this.mounted = true;
+  }
+
   componentWillUnmount() {
+    this.mounted = false;
     // Remove any leftover event handlers. Remove both touch and mouse handlers in case
     // some browser quirk caused a touch event to fire during a mouse move, or vice versa.
     const thisNode = ReactDOM.findDOMNode(this);
@@ -277,7 +284,7 @@ export default class DraggableCore extends React.Component<DraggableCoreProps, D
     // Call event handler. If it returns explicit false, cancel.
     log('calling', this.props.onStart);
     const shouldUpdate = this.props.onStart(e, coreEvent);
-    if (shouldUpdate === false) return;
+    if (shouldUpdate === false || this.mounted === false) return;
 
     // Add a style to the body to disable user-select. This prevents text from
     // being selected all over the page.
@@ -324,7 +331,7 @@ export default class DraggableCore extends React.Component<DraggableCoreProps, D
 
     // Call event handler. If it returns explicit false, trigger end.
     const shouldUpdate = this.props.onDrag(e, coreEvent);
-    if (shouldUpdate === false) {
+    if (shouldUpdate === false || this.mounted === false) {
       try {
         // $FlowIgnore
         this.handleDragStop(new MouseEvent('mouseup'));
@@ -353,6 +360,10 @@ export default class DraggableCore extends React.Component<DraggableCoreProps, D
     const {x, y} = position;
     const coreEvent = createCoreData(this, x, y);
 
+    // Call event handler
+    const shouldContinue = this.props.onStop(e, coreEvent);
+    if (shouldContinue === false || this.mounted === false) return false;
+
     const thisNode = ReactDOM.findDOMNode(this);
     if (thisNode) {
       // Remove user-select hack
@@ -368,9 +379,6 @@ export default class DraggableCore extends React.Component<DraggableCoreProps, D
       lastY: NaN
     });
 
-    // Call event handler
-    this.props.onStop(e, coreEvent);
-
     if (thisNode) {
       // Remove event handlers
       log('DraggableCore: Removing handlers');

diff --git a/specs/draggable.spec.jsx b/specs/draggable.spec.jsx
@@ -721,6 +721,38 @@ describe('react-draggable', function () {
       // (element, fromX, fromY, toX, toY)
       simulateMovementFromTo(drag, 0, 0, 100, 100);
     });
+
+    it('should not throw an error if unmounted during a callback', function () {
+      function App(props) {
+        const [firstVisible, setFirstVisible] = React.useState(true);
+        // Callback with ref to draggable
+        const dragRef = React.useRef(null);
+        props.draggableRefCb(dragRef);
+        return (
+          <div className="App">
+            <button onClick={() => setFirstVisible(true)}>Show draggables</button>
+
+            {firstVisible && (
+              <Draggable onStop={() => setFirstVisible(false)} ref={dragRef}>
+                <h2>1. Drag me!</h2>
+              </Draggable>
+            )}
+          </div>
+        );
+      }
+      let dragRef;
+      const appContainer = TestUtils.renderIntoDocument(
+        <App draggableRefCb={(_ref) => {dragRef = _ref;}}/>
+      );
+
+      // (element, fromX, fromY, toX, toY)
+      simulateMovementFromTo(dragRef.current, 0, 0, 100, 100);
+
+      // ok, was a setstate warning thrown?
+      // Assert unmounted
+      assert(dragRef.current === null);
+    });
+
   });
 
   describe('DraggableCore callbacks', function () {