Fix useRequest to support query change

nchaulet · nchaulet · commit a3c837a0b243 · 2020-02-19T12:48:51.000-05:00
diff --git a/src/plugins/es_ui_shared/public/request/np_ready_request.ts b/src/plugins/es_ui_shared/public/request/np_ready_request.ts
@@ -17,7 +17,7 @@
  * under the License.
  */
 
-import { useEffect, useState, useRef } from 'react';
+import { useEffect, useState, useRef, useMemo } from 'react';
 
 import { HttpSetup, HttpFetchQuery } from '../../../../../src/core/public';
 
@@ -78,6 +78,7 @@ export const useRequest = <D = any>(
     deserializer = (data: any): any => data,
   }: UseRequestConfig
 ): UseRequestResponse<D> => {
+  const sendRequestRef = useRef<() => Promise<SendRequestResponse<D>>>();
   // Main states for tracking request status and data
   const [error, setError] = useState<null | any>(null);
   const [isLoading, setIsLoading] = useState<boolean>(true);
@@ -102,7 +103,10 @@ export const useRequest = <D = any>(
 
     // Set new interval
     if (pollInterval.current) {
-      pollIntervalId.current = setTimeout(_sendRequest, pollInterval.current);
+      pollIntervalId.current = setTimeout(
+        () => (sendRequestRef.current ?? _sendRequest)(),
+        pollInterval.current
+      );
     }
   };
 
@@ -145,11 +149,17 @@ export const useRequest = <D = any>(
   };
 
   useEffect(() => {
-    _sendRequest();
-    // To be functionally correct we'd send a new request if the method, path, or body changes.
+    sendRequestRef.current = _sendRequest;
+  }, [_sendRequest]);
+
+  const stringifiedQuery = useMemo(() => JSON.stringify(query), [query]);
+
+  useEffect(() => {
+    (sendRequestRef.current ?? _sendRequest)();
+    // To be functionally correct we'd send a new request if the method, path, query or body changes.
     // But it doesn't seem likely that the method will change and body is likely to be a new
-    // object even if its shape hasn't changed, so for now we're just watching the path.
-  }, [path]);
+    // object even if its shape hasn't changed, so for now we're just watching the path and the query.
+  }, [path, stringifiedQuery]);
 
   useEffect(() => {
     scheduleRequest();
@@ -168,6 +178,6 @@ export const useRequest = <D = any>(
     isLoading,
     error,
     data,
-    sendRequest: _sendRequest, // Gives the user the ability to manually request data
+    sendRequest: sendRequestRef.current ?? _sendRequest, // Gives the user the ability to manually request data
   };
 };
diff --git a/src/plugins/es_ui_shared/public/request/request.test.js b/src/plugins/es_ui_shared/public/request/request.test.js
@@ -71,7 +71,7 @@ describe.skip('request lib', () => {
     it('uses the provided path, method, and body to send the request', async () => {
       const response = await sendRequest({ ...successRequest });
       sinon.assert.calledOnce(sendPost);
-      expect(response).toEqual({ data: successResponse.data });
+      expect(response).toEqual({ data: successResponse.data, error: null });
     });
 
     it('surfaces errors', async () => {
@@ -182,11 +182,11 @@ describe.skip('request lib', () => {
           expect(hook.error).toBe(errorResponse);
         });
 
-        it('is undefined when the request is successful', async () => {
+        it('is null when the request is successful', async () => {
           initUseRequest({ ...successRequest });
           await wait(50);
           expect(hook.isLoading).toBe(false);
-          expect(hook.error).toBeUndefined();
+          expect(hook.error).toBeNull();
         });
       });
 
@@ -205,11 +205,11 @@ describe.skip('request lib', () => {
           expect(hook.data).toBe(successResponse.data);
         });
 
-        it('is undefined when the request fails', async () => {
+        it('is null when the request fails', async () => {
           initUseRequest({ ...errorRequest });
           await wait(50);
           expect(hook.isLoading).toBe(false);
-          expect(hook.data).toBeUndefined();
+          expect(hook.data).toBeNull();
         });
       });
     });
