3
\$\begingroup\$

I'm building a full stack exercise app and some of my objects include routines, exercises, exercise sets etc. Instead of writing out specific get, post, put and delete requests for each resource, I was wondering if there's a more dynamic way to make requests using a custom React hook. I've seen many examples of a useFetch hook, but not much for other request types. I've already built a custom hook, and created a strategy for making requests in my components. It seems to work, but I was hoping to get some feedback from some more experienced developers out there if they are willing, or if they can show some examples of what I'm trying to achieve. Thanks!

Type Defs

import { AxiosError, AxiosResponse } from 'axios'
import React from 'react'

export type StatusType = "fetching" | "creating" | "updating" | "deleting" | "done" 

export interface DataReturnType {
  response: AxiosResponse | undefined
  status: StatusType
  error: AxiosError | undefined
}

// Every time there's a new resource added to my back end, all I have to do is add a 
// new endpoint string here and I automatically get all the requests.

export type ResourceEndpointType = "routines" | "routines/routine" | "routines/weeks" | "set-groups" | "exercise-sets" | "exercises" | "users"

// Below essentially tells you how the request is built every time you need it. 
// TypeScript makes it easy to implement because you never have to remember what the endpoint is,
// it just pops up as you're typing. Furthermore, this approach imposes  more consistent structure to requests. 

export type RequestObjType = {
  resourceEndPoint: ResourceEndpointType
  parameter?: string
  query?: string
  request: "get" | "post" | "put" | "delete"
  body?: object
  trigger:boolean
  setTrigger: React.Dispatch<React.SetStateAction<boolean>>
}

The Hook

import { useEffect, useState } from "react";
import { AxiosError, AxiosResponse } from "axios";
import axiosWithAuth from "../utils/axiosWithAuth"; // an instance of axios with baseUrl and appropriate headers
import { DataReturnType, RequestObjType, StatusType } from "./apiTypes";

enum Statuses {
  get = "fetching",
  post = "creating",
  put = "updating",
  delete = "deleting",
  done = "done",
}

const useRequest = (requestObj: RequestObjType): DataReturnType => {
  const [response, responseSet] = useState<AxiosResponse | undefined>(
    undefined
  );
  const [status, statusSet] = useState<StatusType>(Statuses.done);
  const [error, errorSet] = useState<AxiosError | undefined>(undefined);

  let url = requestObj.resourceEndPoint;
  if (requestObj.parameter) url += "/" + requestObj.parameter;
  if (requestObj.query) url += "?" + requestObj.query;

  useEffect(() => {
    if (requestObj.trigger) { // trigger and setTrigger are passed in by the component
      console.log(`Requesting ${requestObj.request.toUpperCase()}...`);
      statusSet(Statuses[requestObj.request]);
      axiosWithAuth()
        [requestObj.request](url, requestObj.body)
        .then((response: AxiosResponse) => {
          requestObj.setTrigger(false);
          responseSet(response);
          statusSet("done");
        })
        .catch((error: AxiosError) => {
          console.log(error);
          errorSet(error);
          statusSet("done");
          requestObj.setTrigger(false);
        });
    }
    // resetting response and error every time will allow for conditional checks in the component
    responseSet(undefined);
    errorSet(undefined);
    requestObj.setTrigger(false);

  }, [url, requestObj]);

  return {
    response,
    status,
    error,
  };
};

export default useRequest;

Using the hook for GET in a search bar component

  const [search, setSearch] = useState("");
  const { loadExercises, paginationSet } = useExerciseContext(); // custom hook built from useContext
  const [trigger, shouldFetch] = useState(false);

  const { response, status } = useRequest({
    request: "get",
    query: `name=${search}`,
    resourceEndPoint: "exercises",
    trigger,
    setTrigger: shouldFetch,
  });
  // watching for changes in the response
  useEffect(() => {
    if (response) {
      loadExercises(response.data.data);
      paginationSet(response.data.pagination)
    }
  }, [loadExercises, response, status, paginationSet]);

 ... 
     <Button
        onClick={() => shouldFetch(true)} // triggering the request
        className={classes.submitBtn}
        variant="outlined"
      >
        Submit
      </Button>

Using the hook for PUT in another component

  const { changeRoutineColor, routineColorMap, replaceRoutine } =
    useRoutinesContext();
  const routineColor = routineColorMap[routine._id];
  const classes = useItemStyles({ routineColor });
  const [shouldUpdate, setShouldUpdate] = useState(false); // you can make the triggers more semantic
  const { response, status, error } = useUpdate({ // being more semantic with the useResource import
    resourceEndPoint: "routines/routine",
    parameter: routine._id,
    query: "select=color", // I don't need the entire object back from the backend (MongoDB/Node/Mongoose) so I can add a select to the query
    request: "put",
    trigger: shouldUpdate,
    setTrigger: setShouldUpdate,
    body: { color: routineColor },
  });
  
  // handling the response
  useEffect(() => {
    if (response && response.data) {
      replaceRoutine({ ...routine, color: response.data.data.color });
    }
  }, [replaceRoutine, response, routine, error]);

  const onChange = (newColor: string) => {
    changeRoutineColor(routine._id, newColor);
  };
  // 
  const handleClose = () => {
    popupState.close();
    setShouldUpdate(true); // triggering the request
  };

\$\endgroup\$
3
\$\begingroup\$

The biggest part is that the code works and it looks okay.

I only have a few concerns about unnecessary rerenders and inconsistent UI because of inaccurate status state.

Below are a few minor things I'd recommend and consider.

Naming

Code naming should be consistent and verbose.

  • I'd recommend changing the name of trigger to refetch and setTrigger to something more descriptive to what's the code is doing - I'd recommend using the same name for the component - shouldFetch.

  • The set state variable assignments are not named consistently - i.e. paginationSet and setSearch. I'd recommend following the standard of the word set followed by a descriptive state word.

Below are two examples:

  • rename pagninationSet to setPagination
  • rename statusSet to setStatus

Status enums are not consistently used (you manually type 'done') - but I think this can be removed all together - see state.

The promise code can be simplified by using finally.


State

Objectively speaking you only have four states - No data, loading, error, and data.

Since you are using the hook in the components that are fetching the data you know what the UI would show in the place you're using the hook - the actual status strings of deleting, creating etc. are unnecessary for the universal fetch hook.

Here are some things I'd do

  • Remove the added complexity with the setStatus and change it to loading.

  • I would replace status with loading and remove the status enum and status logic as it is not necessary to achieve the goal of the hook and UI.
    This would also solve the issue of inaccurate default status of done;

  • Set the error state to either Boolean or the message.

  • I don't like undefined values for state, use either string (or empty one), null, a Boolean - something more representative of the actual set state.

  • I prefer string literals over string concatenation since I think they are more concise, so I'd change how the URL is generated.

  • I prefer destructuring too.

  • Move the setState into the if condition because you only set state values after when fetching. It will make sure you always reflect the current state to the components using the hook

  • Move the url inside the effect hook to reduce the unnecessary generation and effect hook dependency of url if the condition is not met.

Small refactor of the main hook

const useRequest = (request)=> {
  const [response, setResponse] = useState(null);
  const [loading, setLoading] = useState(false);
  const [error, setError] = useState(false);

  useEffect(() => {
    const { 
     body, resourceEndPoint, parameter, query, refetch, request, shouldFetch
    } = request;
    if (refetch) {
      const url = `${resourceEndPoint}${parameter ?`/${parameter}` :''}${query ?`?${query}` :''}`;
      setLoading(true);
      setError(false)
      axiosWithAuth()
        [request](url, body)
        .then(setResponse)
        .catch(()=> {
           setError(true)
         })
        .finally(() => setLoading(false));
    }
    shouldFetch(false); // can only be false - don't need to place in `if` statement
  }, [request]);

  return { error, loading, response };
};

Lastly, make the other components states match the names here.

Note: I wrote this in the stack editor so there might be a typo in the names. I also removed the types to make it easier to follow on here and I can't verify they are correct - they need to updated to reflect the changes.

\$\endgroup\$
1
  • 1
    \$\begingroup\$ Man, thanks so much! That's really helpful feedback. \$\endgroup\$
    – gCoder
    Aug 9 '21 at 10:55

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.