fix: avoid extra renders

* avoid setting state same as previous * fire effect by query state change * move problematic fetch code to react-query like hook * memoize SearchBox
adamborowski · Jan 16, 2023 · 1f91cf0 · 1f91cf0
1 parent 606c1e4
commit 1f91cf0
Show file tree

Hide file tree

Showing 9 changed files with 140 additions and 161 deletions.
diff --git a/src/common/api/clients/SearchClient.ts b/src/common/api/clients/SearchClient.ts
@@ -1,6 +1,3 @@
 export interface SearchClient<Entity> {
-  search: (
-    query: string,
-    abortController?: AbortController
-  ) => Promise<Entity[]>;
+  search: (query: string, signal?: AbortSignal) => Promise<Entity[]>;
 }
diff --git a/src/common/api/clients/fetchSearchClient.ts b/src/common/api/clients/fetchSearchClient.ts
@@ -8,14 +8,14 @@ export const createFetchSearchClient = <ServerType, ClientType>(
   adapter: Adapter<ServerType, ClientType>,
   fetchImpl: typeof fetch = fetch
 ): SearchClient<ClientType> => ({
-  search: async (query, abortController) => {
+  search: async (query, signal) => {
     const response = await fetchImpl(urlFactory(query), {
       method: "GET",
       headers: {
         Authorization: "Bearer " + token,
         "Content-Type": "application/json",
       },
-      signal: abortController?.signal,
+      signal,
     });
     const json = await response.json();
 

diff --git a/src/common/api/clients/graphqlSearchClient.ts b/src/common/api/clients/graphqlSearchClient.ts
@@ -12,7 +12,7 @@ export const createGraphqlSearchClient = <ServerType, ClientType>(
   token: string,
   adapter: Adapter<ServerType, ClientType>
 ): SearchClient<ClientType> => ({
-  search: async (query, abortController) => {
+  search: async (query, signal) => {
     const gql = {
       // TODO inject specific query from external
       query: `query ($query: String!) {
@@ -45,7 +45,7 @@ export const createGraphqlSearchClient = <ServerType, ClientType>(
         Authorization: "Bearer " + token,
         "Content-Type": "application/json",
       },
-      signal: abortController?.signal,
+      signal,
       body: JSON.stringify(gql),
     });
     const json = await response.json();

diff --git a/src/common/services/useQuery.ts b/src/common/services/useQuery.ts
@@ -0,0 +1,50 @@
+import { useCallback, useEffect, useRef, useState } from "react";
+import { PromiseState } from "../state/usePromiseState";
+import CancelablePromise from "cancelable-promise";
+import { cancelableWithAbortController } from "../state/cancelablePromise";
+
+export const useQuery = <Data>(
+  getData: (signal: AbortSignal) => Promise<Data>
+) => {
+  const [state, setState] = useState<PromiseState<Data>>({
+    type: "pending",
+  });
+  const lastPromise = useRef<CancelablePromise | null>(null);
+  const lastState = useRef(state);
+
+  const reload = useCallback(async () => {
+    const abortController = new AbortController();
+    lastPromise.current?.cancel();
+    lastState.current.type !== "pending" && setState({ type: "pending" }); // avoid extra render, read state by ref
+    try {
+      lastPromise.current = cancelableWithAbortController(
+        abortController,
+        getData(abortController.signal)
+      );
+
+      const fetchedResult = await lastPromise.current;
+      setState({ type: "loaded", data: fetchedResult });
+    } catch (e) {
+      if (e instanceof DOMException && e.name === "AbortError") {
+        // ignore this error, we are already in loading state due to the new fetch
+      } else {
+        setState({
+          type: "error",
+          message: String(e),
+        });
+      }
+    }
+  }, [getData]);
+
+  useEffect(() => {
+    void reload();
+
+    return () => {
+      lastPromise.current?.cancel();
+    };
+  }, [reload]);
+
+  lastState.current = state;
+
+  return { state, reload };
+};
diff --git a/src/common/services/useSearchClient.ts b/src/common/services/useSearchClient.ts
diff --git a/src/features/repositories/api/github-graphql/demo.stories.tsx b/src/features/repositories/api/github-graphql/demo.stories.tsx
@@ -1,9 +1,7 @@
-import { FC, useEffect, useState } from "react";
-import { PromiseState } from "../../../../common/state/usePromiseState";
-import { Repository } from "../../types";
+import { FC, useCallback } from "react";
 import { repositorySearchClient } from "./repositorySearchClient";
 import { ComponentStory } from "@storybook/react";
-import { useSearchClient } from "../../../../common/services/useSearchClient";
+import { useQuery } from "../../../../common/services/useQuery";
 
 export default {
   parameters: {
@@ -12,14 +10,11 @@ export default {
 };
 
 const Template: ComponentStory<FC<{ query: string }>> = ({ query }) => {
-  const [state, setState] = useState<PromiseState<Repository[]>>({
-    type: "pending",
-  });
-  const { search } = useSearchClient(state, setState, repositorySearchClient);
-
-  useEffect(() => {
-    void search(query);
-  }, [query, search]);
+  const getData = useCallback(
+    (signal: AbortSignal) => repositorySearchClient.search(query, signal),
+    [query]
+  );
+  const { state } = useQuery(getData);
 
   return (
     <div>

diff --git a/src/features/repositories/api/github-rest/demo.stories.tsx b/src/features/repositories/api/github-rest/demo.stories.tsx
@@ -1,9 +1,7 @@
-import { FC, useEffect, useState } from "react";
-import { PromiseState } from "../../../../common/state/usePromiseState";
-import { Repository } from "../../types";
+import { FC, useCallback } from "react";
 import { repositorySearchClient } from "./repositorySearchClient";
 import { ComponentStory } from "@storybook/react";
-import { useSearchClient } from "../../../../common/services/useSearchClient";
+import { useQuery } from "../../../../common/services/useQuery";
 
 export default {
   parameters: {
@@ -12,14 +10,11 @@ export default {
 };
 
 const Template: ComponentStory<FC<{ query: string }>> = ({ query }) => {
-  const [state, setState] = useState<PromiseState<Repository[]>>({
-    type: "pending",
-  });
-  const { search } = useSearchClient(state, setState, repositorySearchClient);
-
-  useEffect(() => {
-    void search(query);
-  }, [query, search]);
+  const getData = useCallback(
+    (signal: AbortSignal) => repositorySearchClient.search(query, signal),
+    [query]
+  );
+  const { state } = useQuery(getData);
 
   return (
     <div>

diff --git a/src/features/repositories/components/SearchBox.tsx b/src/features/repositories/components/SearchBox.tsx
@@ -1,4 +1,4 @@
-import React, { FC, FormEvent, useLayoutEffect, useState } from "react";
+import React, { FormEvent, memo, useEffect, useRef, useState } from "react";
 import { Button, Flex, Input, useColorModeValue } from "@chakra-ui/react";
 import { PromiseStateType } from "../../../common/state/usePromiseState";
 import { FormattedMessage } from "react-intl";
@@ -10,61 +10,59 @@ export interface SearchBoxProps {
   onSubmit: (query: string) => void;
 }
 
-export const SearchBox: FC<SearchBoxProps> = ({
-  query,
-  onSubmit,
-  searchStateType,
-  ...rest
-}) => {
-  const [currentQuery, setCurrentQuery] = useState(query);
+export const SearchBox = memo<SearchBoxProps>(
+  ({ query, onSubmit, searchStateType, ...rest }) => {
+    const [currentQuery, setCurrentQuery] = useState(query);
+    const lastOnSubmit = useRef(onSubmit);
+    lastOnSubmit.current = onSubmit;
+    useEffect(() => {
+      setCurrentQuery(query);
+    }, [query]);
 
-  useLayoutEffect(() => {
-    setCurrentQuery(query);
-  }, [query]);
+    const isSearchAvailable =
+      searchStateType === "error" || currentQuery !== query;
 
-  const isSearchAvailable =
-    searchStateType === "error" || currentQuery !== query;
+    const handleSearch = (event: FormEvent<HTMLFormElement>) => {
+      event.preventDefault();
+      isSearchAvailable && onSubmit(currentQuery);
+    };
 
-  const handleSearch = (event: FormEvent<HTMLFormElement>) => {
-    event.preventDefault();
-    isSearchAvailable && onSubmit(currentQuery);
-  };
+    const bgColor = useColorModeValue(
+      "rgba(237, 242, 247, 0.9)",
+      "rgba(45, 55, 72, 0.9)"
+    );
+    const border = useColorModeValue(
+      "1px solid var(--chakra-colors-chakra-border-color)",
+      "1px solid var(--chakra-colors-chakra-body-bg)"
+    );
 
-  const bgColor = useColorModeValue(
-    "rgba(237, 242, 247, 0.9)",
-    "rgba(45, 55, 72, 0.9)"
-  );
-  const border = useColorModeValue(
-    "1px solid var(--chakra-colors-chakra-border-color)",
-    "1px solid var(--chakra-colors-chakra-body-bg)"
-  );
-
-  return (
-    <form onSubmit={handleSearch} {...rest}>
-      <Flex
-        padding={4}
-        direction="row"
-        gap={4}
-        alignItems="start"
-        background={bgColor}
-        borderBottom={border}
-      >
-        <Input
-          background="chakra-body-bg"
-          id="query"
-          value={currentQuery}
-          maxWidth="xl"
-          onChange={(event) => setCurrentQuery(event.target.value)}
-        />
-        <Button
-          type="submit"
-          variant="solid"
-          colorScheme="blue"
-          disabled={!isSearchAvailable}
+    return (
+      <form onSubmit={handleSearch} {...rest}>
+        <Flex
+          padding={4}
+          direction="row"
+          gap={4}
+          alignItems="start"
+          background={bgColor}
+          borderBottom={border}
         >
-          <FormattedMessage {...messages.searchButtonLabel} />
-        </Button>
-      </Flex>
-    </form>
-  );
-};
+          <Input
+            background="chakra-body-bg"
+            id="query"
+            value={currentQuery}
+            maxWidth="xl"
+            onChange={(event) => setCurrentQuery(event.target.value)}
+          />
+          <Button
+            type="submit"
+            variant="solid"
+            colorScheme="blue"
+            disabled={!isSearchAvailable}
+          >
+            <FormattedMessage {...messages.searchButtonLabel} />
+          </Button>
+        </Flex>
+      </form>
+    );
+  }
+);
diff --git a/src/features/repositories/pages/search/useRepositorySearchProps.ts b/src/features/repositories/pages/search/useRepositorySearchProps.ts
@@ -1,34 +1,34 @@
-import { useEffect, useState } from "react";
-import { PromiseState } from "../../../../common/state/usePromiseState";
+import { useCallback, useState } from "react";
 import { Repository } from "../../types";
 import { SearchClient } from "../../../../common/api/clients/SearchClient";
-import { useSearchClient } from "../../../../common/services/useSearchClient";
+import { useQuery } from "../../../../common/services/useQuery";
 
 export const useRepositorySearchProps = (
   searchClient: SearchClient<Repository>
 ) => {
   const [query, setQuery] = useState("react");
-  const [repositories, setRepositories] = useState<PromiseState<Repository[]>>({
-    type: "pending",
-  });
 
-  const searchRepositories = useSearchClient(
-    repositories,
-    setRepositories,
-    searchClient
+  const getData = useCallback(
+    (signal: AbortSignal) => searchClient.search(query, signal),
+    [query, searchClient]
   );
 
-  useEffect(() => {
-    void searchRepositories.search(query);
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, []);
+  const repositories = useQuery(getData);
+
+  const onQueryChange = useCallback(
+    (newQuery: string) => {
+      if (query === newQuery) {
+        void repositories.reload();
+      } else {
+        setQuery(newQuery);
+      }
+    },
+    [query, repositories]
+  );
 
   return {
     query,
-    repositories,
-    onQueryChange: (query: string) => {
-      setQuery(query);
-      void searchRepositories.search(query);
-    },
+    repositories: repositories.state,
+    onQueryChange,
   };
 };