From 30cc81fc0d311b7b55fcd9746a7b43d360881b35 Mon Sep 17 00:00:00 2001
From: Nicolas Chaulet <nicolas.chaulet@elastic.co>
Date: Thu, 20 Feb 2020 12:09:04 -0500
Subject: [PATCH] Fix useRequest to support query change (#57723) (#58067)

---
 .../public/request/np_ready_request.ts        | 25 +++++++++++++------
 .../public/request/request.test.js            | 10 ++++----
 2 files changed, 23 insertions(+), 12 deletions(-)

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
index 790e29b6d3655..01144fa1da2e9 100644
--- 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,8 @@ export const useRequest = (
     deserializer = (data: any): any => data,
   }: UseRequestConfig
 ): UseRequestResponse => {
+  const sendRequestRef = useRef<() => Promise<SendRequestResponse>>();
+
   // Main states for tracking request status and data
   const [error, setError] = useState<null | any>(null);
   const [isLoading, setIsLoading] = useState<boolean>(true);
@@ -102,7 +104,10 @@ export const useRequest = (
 
     // Set new interval
     if (pollInterval.current) {
-      pollIntervalId.current = setTimeout(_sendRequest, pollInterval.current);
+      pollIntervalId.current = setTimeout(
+        () => (sendRequestRef.current ?? _sendRequest)(),
+        pollInterval.current
+      );
     }
   };
 
@@ -145,11 +150,17 @@ export const useRequest = (
   };
 
   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 +179,6 @@ export const useRequest = (
     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
index 8c7e2b6ddc72a..44bf149d5fd1e 100644
--- 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();
         });
       });
     });