fix(@cubejs-client/react): prevent state updates on unmounted compone…

…nts (#1684)
cube-js · Dec 25, 2020 · 4f3796c · 4f3796c
1 parent 452ad44
commit 4f3796c
Show file tree

Hide file tree

Showing 3 changed files with 67 additions and 51 deletions.
diff --git a/packages/cubejs-client-react/.eslintrc.js b/packages/cubejs-client-react/.eslintrc.js
@@ -2,6 +2,7 @@ module.exports = {
   extends: 'airbnb',
   plugins: ['react', 'jsx-a11y', 'import'],
   rules: {
+    'object-curly-newline': 0,
     'react/jsx-no-bind': 0,
     'react/jsx-first-prop-new-line': 0,
     'react/jsx-indent-props': 0,

diff --git a/packages/cubejs-client-react/src/hooks/cube-query.js b/packages/cubejs-client-react/src/hooks/cube-query.js
@@ -1,6 +1,4 @@
-import {
-  useContext, useEffect, useState, useRef
-} from 'react';
+import { useContext, useEffect, useState, useRef } from 'react';
 import { equals } from 'ramda';
 import CubeContext from '../CubeContext';
 import isQueryPresent from '../isQueryPresent';
@@ -20,86 +18,97 @@ export default (query, options = {}) => {
   const progressCallback = ({ progressResponse }) => setProgress(progressResponse);
 
   useEffect(() => {
+    let isMounted = true;
     const { skip = false, resetResultSetOnChange } = options;
-    
-    const cubejsApi = options.cubejsApi || context && context.cubejsApi;
-          
+
+    const cubejsApi = options.cubejsApi || (context && context.cubejsApi);
+
     if (!cubejsApi) {
       throw new Error('Cube.js API client is not provided');
     }
 
     async function loadQuery() {
       if (!skip && query && isQueryPresent(query)) {
         const hasOrderChanged = !equals(
-          Object.keys(currentQuery && currentQuery.order || {}),
+          Object.keys((currentQuery && currentQuery.order) || {}),
           Object.keys(query.order || {})
         );
-        
+
         if (hasOrderChanged || !equals(currentQuery, query)) {
           if (resetResultSetOnChange == null || resetResultSetOnChange) {
             setResultSet(null);
           }
           setCurrentQuery(query);
         }
-        
+
         setError(null);
         setLoading(true);
-        
+
         try {
           if (subscribeRequest) {
             await subscribeRequest.unsubscribe();
             subscribeRequest = null;
           }
-          
+
           if (options.subscribe) {
-            subscribeRequest = cubejsApi.subscribe(query, {
-              mutexObj: mutexRef.current,
-              mutexKey: 'query',
-              progressCallback
-            }, (e, result) => {
-              if (e) {
-                setError(e);
-              } else {
-                setResultSet(result);
+            subscribeRequest = cubejsApi.subscribe(
+              query,
+              {
+                mutexObj: mutexRef.current,
+                mutexKey: 'query',
+                progressCallback,
+              },
+              (e, result) => {
+                if (isMounted) {
+                  if (e) {
+                    setError(e);
+                  } else {
+                    setResultSet(result);
+                  }
+                  setLoading(false);
+                  setProgress(null);
+                }
               }
-              setLoading(false);
-              setProgress(null);
-            });
+            );
           } else {
-            setResultSet(await cubejsApi.load(query, {
+            const response = await cubejsApi.load(query, {
               mutexObj: mutexRef.current,
               mutexKey: 'query',
-              progressCallback
-            }));
+              progressCallback,
+            });
+
+            if (isMounted) {
+              setResultSet(response);
+              setLoading(false);
+              setProgress(null);
+            }
+          }
+        } catch (e) {
+          if (isMounted) {
+            setError(e);
             setLoading(false);
             setProgress(null);
           }
-        } catch (e) {
-          setError(e);
-          setLoading(false);
-          setProgress(null);
         }
       }
     }
 
     loadQuery();
+
     return () => {
+      isMounted = false;
+
       if (subscribeRequest) {
         subscribeRequest.unsubscribe();
         subscribeRequest = null;
       }
     };
-  }, useDeepCompareMemoize([
-    query,
-    Object.keys(query && query.order || {}),
-    options,
-    context
-  ]));
+  }, useDeepCompareMemoize([query, Object.keys((query && query.order) || {}), options, context]));
 
   return {
     isLoading,
     resultSet,
     error,
-    progress
+    progress,
   };
 };
diff --git a/packages/cubejs-client-react/src/hooks/dry-run.js b/packages/cubejs-client-react/src/hooks/dry-run.js
@@ -1,6 +1,4 @@
-import {
-  useContext, useEffect, useState, useRef
-} from 'react';
+import { useContext, useEffect, useState, useRef } from 'react';
 import CubeContext from '../CubeContext';
 import isQueryPresent from '../isQueryPresent';
 import useDeepCompareMemoize from './deep-compare-memoize';
@@ -13,39 +11,47 @@ export default function useDryRun(query, options = {}) {
   const [error, setError] = useState(null);
 
   useEffect(() => {
+    let isMounted = true;
     const { skip = false } = options;
 
     const cubejsApi = options.cubejsApi || (context && context.cubejsApi);
     if (!cubejsApi) {
       throw new Error('Cube.js API client is not provided');
     }
-    
+
     async function loadQuery() {
       if (!skip && query && isQueryPresent(query)) {
         setError(null);
         setLoading(true);
 
         try {
-          setResponse(
-            await cubejsApi.dryRun(query, {
-              mutexObj: mutexRef.current,
-              mutexKey: 'dry-run',
-            })
-          );
-          setLoading(false);
+          const dryRunResponse = await cubejsApi.dryRun(query, {
+            mutexObj: mutexRef.current,
+            mutexKey: 'dry-run',
+          });
+          if (isMounted) {
+            setResponse(dryRunResponse);
+            setLoading(false);
+          }
         } catch (err) {
-          setError(err);
-          setLoading(false);
+          if (isMounted) {
+            setError(err);
+            setLoading(false);
+          }
         }
       }
     }
 
     loadQuery();
+
+    return () => {
+      isMounted = false;
+    };
   }, useDeepCompareMemoize([query, Object.keys((query && query.order) || {}), options, context]));
 
   return {
     isLoading,
     error,
-    response
+    response,
   };
 }