From a24b71166b203505ce909e9c909196be6a1dcbc9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Mierzwa?= Date: Sat, 25 Jul 2020 21:48:11 +0100 Subject: [PATCH] fix(ui): use atomic update in useFetchGet --- ui/src/Hooks/useFetchGet.test.tsx | 43 +++++++++---------- ui/src/Hooks/useFetchGet.ts | 70 ++++++++++++++++++++++--------- 2 files changed, 72 insertions(+), 41 deletions(-) diff --git a/ui/src/Hooks/useFetchGet.test.tsx b/ui/src/Hooks/useFetchGet.test.tsx index 0c137100d..7ab9d9491 100644 --- a/ui/src/Hooks/useFetchGet.test.tsx +++ b/ui/src/Hooks/useFetchGet.test.tsx @@ -1,6 +1,6 @@ import React from "react"; -import { renderHook } from "@testing-library/react-hooks"; +import { renderHook, act } from "@testing-library/react-hooks"; import { mount } from "enzyme"; @@ -31,7 +31,7 @@ describe("useFetchGet", () => { it("sends a GET request", async () => { const { waitForNextUpdate } = renderHook(() => - useFetchGet("http://localhost/ok") + useFetchGet("http://localhost/ok") ); await waitForNextUpdate(); @@ -45,7 +45,7 @@ describe("useFetchGet", () => { it("sends correct headers", async () => { const { waitForNextUpdate } = renderHook(() => - useFetchGet("http://localhost/ok") + useFetchGet("http://localhost/ok") ); await waitForNextUpdate(); @@ -61,12 +61,14 @@ describe("useFetchGet", () => { it("doesn't send any request if autorun=false", async () => { const { result, waitForNextUpdate } = renderHook(() => - useFetchGet("http://localhost/ok", { autorun: false }) + useFetchGet("http://localhost/ok", { autorun: false }) ); expect(fetchMock.calls()).toHaveLength(0); - result.current.get(); + act(() => { + result.current.get(); + }); await waitForNextUpdate(); expect(fetchMock.calls()).toHaveLength(1); @@ -75,7 +77,7 @@ describe("useFetchGet", () => { it("will retry failed requests", async () => { const { result, waitForNextUpdate } = renderHook(() => - useFetchGet("http://localhost/error") + useFetchGet("http://localhost/error") ); // initial state @@ -129,7 +131,7 @@ describe("useFetchGet", () => { it("response is updated after successful fetch", async () => { const { result, waitForNextUpdate } = renderHook(() => - useFetchGet("http://localhost/ok") + useFetchGet("http://localhost/ok") ); expect(result.current.response).toBe(null); @@ -153,7 +155,7 @@ describe("useFetchGet", () => { }); const { result, waitForNextUpdate } = renderHook(() => - useFetchGet("http://localhost/500/json") + useFetchGet("http://localhost/500/json") ); await waitForNextUpdate(); @@ -171,7 +173,7 @@ describe("useFetchGet", () => { }); const { result, waitForNextUpdate } = renderHook(() => - useFetchGet("http://localhost/500/text") + useFetchGet("http://localhost/500/text") ); await waitForNextUpdate(); @@ -184,7 +186,7 @@ describe("useFetchGet", () => { it("error is updated after failed fetch", async () => { const { result, waitForNextUpdate } = renderHook(() => - useFetchGet("http://localhost/error") + useFetchGet("http://localhost/error") ); expect(result.current.response).toBe(null); @@ -210,7 +212,7 @@ describe("useFetchGet", () => { }); const { result, waitForNextUpdate } = renderHook(() => - useFetchGet("http://localhost/json/invalid") + useFetchGet("http://localhost/json/invalid") ); expect(result.current.response).toBe(null); @@ -236,7 +238,7 @@ describe("useFetchGet", () => { }); const Component = () => { - const { response, error, isLoading } = useFetchGet( + const { response, error, isLoading } = useFetchGet( "http://localhost/slow/ok" ); return ( @@ -265,7 +267,7 @@ describe("useFetchGet", () => { }); const Component = () => { - const { response, error, isLoading } = useFetchGet( + const { response, error, isLoading } = useFetchGet( "http://localhost/slow/500" ); return ( @@ -293,7 +295,7 @@ describe("useFetchGet", () => { }); const Component = () => { - const { response, error, isLoading } = useFetchGet( + const { response, error, isLoading } = useFetchGet( "http://localhost/slow/error" ); return ( @@ -322,7 +324,7 @@ describe("useFetchGet", () => { }); const Component = () => { - const { response, error, isLoading } = useFetchGet( + const { response, error, isLoading } = useFetchGet( "http://localhost/slow/json/invalid" ); return ( @@ -348,7 +350,7 @@ describe("useFetchGet", () => { }); const Component = () => { - const { response, error, isLoading } = useFetchGet( + const { response, error, isLoading } = useFetchGet( "http://localhost/slow/text" ); return ( @@ -385,11 +387,10 @@ describe("useFetchGet", () => { jest.useRealTimers(); const Component = () => { - const { - response, - error, - isLoading, - } = useFetchGet("http://localhost/slow/body", { fetcher: fetcher }); + const { response, error, isLoading } = useFetchGet( + "http://localhost/slow/body", + { fetcher: fetcher } + ); return ( {response} diff --git a/ui/src/Hooks/useFetchGet.ts b/ui/src/Hooks/useFetchGet.ts index 2f5379d08..22b062a34 100644 --- a/ui/src/Hooks/useFetchGet.ts +++ b/ui/src/Hooks/useFetchGet.ts @@ -14,6 +14,14 @@ export interface FetchGetOptionsT { fetcher?: null | FetchFunctionT; } +interface ResponseState { + response: null | T; + error: null | string; + isLoading: boolean; + isRetrying: boolean; + retryCount: number; +} + const useFetchGet = ( uri: string, { autorun = true, deps = [], fetcher = null }: FetchGetOptionsT = {} @@ -26,11 +34,13 @@ const useFetchGet = ( get: () => void; cancelGet: () => void; } => { - const [response, setResponse] = useState(null); - const [error, setError] = useState(null); - const [isLoading, setIsLoading] = useState(true); - const [isRetrying, setIsRetrying] = useState(false); - const [retryCount, setRetryCount] = useState(0); + const [response, setResponse] = useState>({ + response: null, + error: null, + isLoading: autorun, + isRetrying: false, + retryCount: 0, + }); const isCanceled = useRef(false); const cancelGet = useCallback(() => { @@ -41,11 +51,15 @@ const useFetchGet = ( isCanceled.current = false; try { - setIsLoading(true); - setRetryCount(0); - setError(null); + setResponse((r) => ({ + ...r, + isLoading: true, + isRetrying: false, + retryCount: 0, + })); + const res = await promiseRetry( - (retry: (err: Error) => Promise, number: number) => + (retry: (err: Error) => Promise, n: number) => (fetcher || fetch)( uri, merge( @@ -55,13 +69,16 @@ const useFetchGet = ( }, CommonOptions, { - mode: number <= FetchRetryConfig.retries ? "cors" : "no-cors", + mode: n <= FetchRetryConfig.retries ? "cors" : "no-cors", } ) as RequestInit ).catch((err: Error) => { if (!isCanceled.current) { - setIsRetrying(true); - setRetryCount(number); + setResponse((r) => ({ + ...r, + isRetrying: true, + retryCount: n, + })); return retry(err); } }), @@ -79,18 +96,31 @@ const useFetchGet = ( if (!isCanceled.current) { if (res.ok) { - setResponse(body); + setResponse({ + response: body, + error: null, + isLoading: false, + isRetrying: false, + retryCount: 0, + }); } else { - setError(body); + setResponse({ + response: null, + error: body, + isLoading: false, + isRetrying: false, + retryCount: 0, + }); } - setIsLoading(false); - setIsRetrying(false); } } } catch (error) { - setError(error.message); - setIsLoading(false); - setIsRetrying(false); + setResponse((r) => ({ + ...r, + error: error.message, + isLoading: false, + isRetrying: false, + })); } }, [uri, fetcher]); @@ -100,7 +130,7 @@ const useFetchGet = ( return () => cancelGet(); }, [uri, get, cancelGet, autorun, ...deps]); // eslint-disable-line react-hooks/exhaustive-deps - return { response, error, isLoading, isRetrying, retryCount, get, cancelGet }; + return { get, cancelGet, ...response }; }; export { useFetchGet };