[React] Todo App 리팩토링 (1)

외부 요소에 강결합된 코드

Before

• Axios 객체/API 인스턴스가 Fetcher 함수에서 바로 사용된다.

  • 이 경우 auth.ts에서는 base url처럼 API 인스턴스에서 관리가 가능한 부분도 각 함수에 의존하게 된다. 해당 파일에서 Fetcher가 2개뿐이기 때문에 별 생각 없이 초반에 작성했던대로 코드가 유지되었다. 그러나 이후 todo.ts에서는 더 많은 Fetcher를 효율적으로 관리하기 위해 API 인스턴스를 분리했고, 결국 두 파일에서 API 요청을 위해 같은 base url을 사용함에도 관심사가 적절히 분리되지 않았다.

• todo.ts에서 API 인스턴스의 헤더 토큰을 설정한다.

  • 토큰 설정 코드가 api 코드와 함께 있어 관심사가 적절히 분리되지 않았다는 생각이 들었다. 또한 이 경우 해당 파일을 처음 읽을 때를 제외하고 토큰을 설정하는 부분이 실행되지 않는 문제가 있었다.

  // src/api/auth.ts
  import axios, { AxiosRequestConfig } from 'axios'
  
  export const login = async (email: string, password: string) => {
    try {
      const response: AxiosResponse = await axios.post('http://127.0.0.1:8080/users/login', { email, password })
      return response.data
    } catch (error) {
      return error
    }
  }
  
  export const signUp = async (email: string, password: string) => {
    try {
      const response: AxiosResponse = await axios.post('http://127.0.0.1:8080/users/create', { email, password })
      return response.data
    } catch (error) {
      return error
    }
  }

  // src/api/todo.ts
  import axios, { AxiosRequestConfig } from 'axios'
  
  export interface ResponseData {
    title: string;
    content: string;
    id: string;
    createdAt: string;
    updatedAt: string;
  }
  
  interface AxiosResponse<T = never> {
    data: T;
    status: number;
    statusText: string;
    headers: Record<string, string>;
    config: AxiosRequestConfig<T>;
    request?: any;
  }
  
  const token = localStorage.getItem('token')
  const axiosInstance = axios.create({
    baseURL: 'http://127.0.0.1:8080/todos',
    headers: {
      Authorization: token
    }
  })
  
  export const getTodos = async () => {
    try {
      const response: AxiosResponse = await axiosInstance.get('')
      return response.data
    } catch (error) {
      return error
    }
  }  

After

• auth.ts와 todo.ts에서 공통되는 API 인스턴스를 추출하여 파일을 분리했다.

• 헤더 토큰 설정은 API 인스턴스에서 가능한 작업이므로 해당 파일에 작성했다.

  // src/api/axios.ts
  import axios from 'axios'
  
  const apiInstance = axios.create({
    baseURL: 'http://127.0.0.1:8080',
  })
  
  apiInstance.defaults.headers.common.Authorization = localStorage.getItem('token') || ''
  
  export default apiInstance

  // src/api/auth.ts
  import apiInstance from './axios';
  
  export interface authData {
    message: string;
    token: string;
  }
  
  export const login = async (email: string, password: string): Promise<authData> => {
    const { data } = await apiInstance.post('/users/login', { email, password })
    return data
  }
  
  export const signUp = async (email: string, password: string): Promise<authData> => {
    const { data } = await apiInstance.post('/users/create', { email, password })
    return data
  }

불필요한 코드

Before

• api 요청에 대한 응답이 제대로 올 경우 내부에 있는 data만 사용함에도 전체 응답에 대한 타입을 작성했다.

• 이 과정에서 불필요하게 AxiosRequestConfig를 import 했다.

  // src/api/auth.ts
  import axios, { AxiosRequestConfig } from 'axios'
  
  export interface ResponseData {
    message: string;
    token: string;
  }
  
  interface AxiosResponse<T = never>  {
    data: ResponseData;
    status: number;
    statusText: string;
    headers: Record<string, string>;
    config: AxiosRequestConfig<T>;
    request?: any;
  }
  
  export const login = async (email: string, password: string) => {
    try {
      const response: AxiosResponse = await axios.post('http://127.0.0.1:8080/users/login', { email, password })
      return response.data
    } catch (error) {
      return error
    }
  }

After

• 구조 분해 할당을 통해 data 속성만 가져오고, 이 data에 대한 타입만 작성해주었다.

  // src/api/auth.ts
  import apiInstance from './axios';
  
  export interface authData {
    message: string;
    token: string;
  }
  
  export const login = async (email: string, password: string): Promise<authData> => {
    const { data } = await apiInstance.post('/users/login', { email, password })
    return data
  }

함수 추상화

Before

• 명령형으로 작성된 코드가 많았다. 따라서 코드를 읽어야만 무슨 일을 하는지 알 수 있었다.

  // src/pages/Login.tsx
  useEffect(() => {
    // 사용자 입력이 조건을 만족했는지 여부에 따라 제출 버튼 disabled 토글
    if (emailRule.test(email) === false || pw.length < 8) {
      setIsDisabled(true)
      return
    }
    setIsDisabled(false)
  }, [email, pw])
  
  function handleLogin() {
    // handleLogin 함수에서 토큰 검사와 로그인 처리 -> 이름이 불분명
    if (token) {
      navigate('/')
      return
    }
    login(email, pw)
      .then(data => {
        const { message, token } = data as ResponseData
        localStorage.setItem('token', token)
        axiosInstance.defaults.headers.common.Authorization = token
        getTodos()
        .then(res => {
          const resValue = res as TodoResponseData
          setTodos(resValue.data)
        })
        alert(message)
        navigate('/')
      })
  }

  // src/pages/Intro.tsx
  return (
    <ButtonsContainer>
      <LoginButton onClick={() => {
        if (isValidToken) {
          navigate('/')
        } else {
          navigate('/auth/login')
        }
      }}>로그인</LoginButton>
      // ...
    </ButtonsContainer>
  )

After

• 함수로 분리했을 때 더 파악하기 쉽다고 생각되는 부분을 분리했다.

  • Login: useEffect 부분에 작성한 코드를 함수로 분리한 게 과연 나은 방법이 맞는지에 대해 고민 중이다.

  // src/pages/Login.tsx
  function validateUserInput() {
    if (emailRule.test(email) === false || pw.length < 8) {
      setIsDisabled(true)
      return
    }
    setIsDisabled(false)
  }
  
  useEffect(() => {
    validateUserInput()
  }, [email, pw])
  
  async function handleLogin() {
    try {
      const { message, token } = await login(email, pw)
      localStorage.setItem('token', token)
      apiInstance.defaults.headers.common.Authorization = token
      alert(message)
      navigate('/')
    } catch (error) {
      alert('로그인에 실패했습니다.')
    }
  }
  
  function handleFormSubmit() {
    if (isValidToken) {
      navigate('/')
      return
    }
    handleLogin()
  }

  // src/pages/Intro.tsx
  function navigateToLogin() {
    if (isValidToken) {
      navigate('/')
    } else {
      navigate('/auth/login')
    }
  }
  
  return (
    <ButtonsContainer>
      <LoginButton onClick={() => navigateToLogin()}>로그인</LoginButton>
      // ...
    </ButtonsContainer>
  )

컴포넌트 추상화

Before

• Detail 페이지에서 Home 페이지의 UI를 포함하고 있어 코드가 중복되었다. 그래서 css를 수정할 경우 두 파일을 모두 수정해야 하고 todos도 두 파일에서 import 해야 했기 때문에 코드를 작성할 당시에도 좋지 않은 방법이라고 생각하고 있었으나, 방법을 찾지 못해 아래와 같이 작성했다.

  // src/pages/Detail.tsx
  export default function Detail() {
    // ...
    return (
      <Page>
        {/* 여기부터 */}
        <H1>Todo App</H1>
        <ListDetailContainer>    
          <div>
            <TodoForm />
            <Ul>
              {todos && todos.length > 0 && todos.map((todo: Todo) => (
                <TodoItem
                  key={todo.id}
                  currentTodo={todo}
                />
              ))}
            </Ul>
          </div>
          {/* 여기까지가 중복되는 부분 */}
          <DetailItem>
            <p>{currentTodo.title}</p>
            <p>{currentTodo.content}</p>
            <p>작성일: {currentTodo.createdAt.split('T')[0]}</p>
            <p>수정일: {currentTodo.updatedAt.split('T')[0]}</p>
          </DetailItem>
        </ListDetailContainer>
      </Page>
    )
  }

After

• 처음에는 Home 컴포넌트를 Detail에서 import 하는 방법을 고려했었다.
  그러던 중 전에 비슷한 작업을 했던 게 생각나서 velog를 뒤지다가 이전에 썼던 이 글을 발견했다. 부모 라우트 요소에서 자식 라우트 요소를 렌더할 때 Outlet을 사용한다고 적었던 것에서 힌트를 얻어 react-router-dom Outlet을 찾아 아래와 같이 코드를 수정했다.

  // src/App.tsx
  <Route path="/" element={<Home />}>
    <Route path=":todoId" element={<Detail />} />
  </Route>

  // src/pages/Home.tsx
  import { useNavigate, Outlet, Link } from 'react-router-dom'
  
  export default function Home() {
    // ...
    return (
      <Page>
        <HeaderWrapper>
          <StyledLink to="/">
            <H1>Todo App</H1>
          </StyledLink>
          <LogoutButton />
        </HeaderWrapper>
        <OutletContainer>
          <div>
            <TodoForm />
            <Ul>
              {todos && todos.length > 0 && todos.map((todo: Todo) => (
                <TodoItem
                  key={todo.id}
                  currentTodo={todo}
                />
              ))}
            </Ul>
          </div>
          {/* Here */}
          <Outlet />
        </OutletContainer>
      </Page>
    )
  }

비동기 처리 & Promise Type

Before

• api 요청 함수에서는 async & await를 사용하고 api 요청 함수를 호출하는 부분에서는 .then()을 사용했기 때문에 코드의 통일성이 부족했다.

• Promise chaining이 있는 부분의 가독성이 좋지 않았다.

• api 요청 함수에서 처리 여부에 따라 data 또는 error를 반환하므로 해당 함수를 호출하는 부분에서 어떤 응답이 올지 확실하게 알 수 없었다.

• 따라서 해당 함수를 호출하는 부분에서 타입을 작성하고자 했고, 처리 방법을 정확히 알지 못해 as를 사용했다.

  // src/api/todo.ts
  export const getTodos = async () => {
    try {
      const response: AxiosResponse = await axiosInstance.get('')
      return response.data
    } catch (error) {
      return error
    }
  }

  // src/components/TodoForm.tsx
  function handleCreateTodo() {
    tokenCheck()
    createTodo(title, content)
      .then(() => {
        setTitle('')
        setContent('')
        getTodos()
          .then(res => {
            const resValue = res as ResponseData
            setTodos(resValue.data)
          })
      })
  }

After

• api 요청 함수에서는 확실한 한 타입의 리턴값만 갖기 위해 api 요청에 대해 응답 안에 있는 data만 리턴해주도록 수정했다.

• try/catch문을 통해 에러가 발생했을 경우에 대한 처리는 함수 호출 부분에서 파악&처리가 가능해야 한다고 생각하여 노출시켰다.

  // src/api/todo.ts
  export const getTodos = async (): Promise<TodosData> => {
    const { data } = await apiInstance.get('/todos')
    return data
  }

  // src/pages/Home.tsx
  async function handleGetTodos() {
    try {
      const { data } = await getTodos()
      setTodos(data)
    } catch (error) {
      alert('할 일 목록을 가져올 수 없습니다.')
    }
  }
  
  useEffect(() => {
    // ...
    handleGetTodos()
  }, [])

고민되었던 부분

deleteTodo 함수 자체는 async await로 구현되어 있다. 그런데 todo를 삭제할 때는 응답에 관계 없이 요청이 성공했다면 바로 todos.filter()를 수행해도 된다. 그렇다면 아래와 같은 형태로 작성해도 되지 않을까 하는 고민이 있었다. 그런데 아래와 같이 작성하면 try catch 구문에서 deleteTodo의 에러를 캐치할 수 있는지에 대해 의문이 생겼다.

// src/components/TodoItem.tsx
async function handleDeleteTodo() {
  try {
    deleteTodo(currentTodo.id)
    const newTodos = todos.filter(todo => todo.id !== currentTodo.id)
    setTodos(newTodos)
  } catch (error) {
    alert('할 일을 삭제할 수 없습니다.')
  }
}

그래서 deleteTodo(currentTodo.id + 1)로 코드를 수정해서 실행해보았는데 에러를 캐치하지 못했다. (에러 확인 방법을 고민하면서 테스트 프레임워크를 배우고 싶다고 생각했다..) 응답을 받아오는 부분이 없다면 deleteTodo() 함수 호출 자체에서는 에러가 발생하지 않기 때문이 아닐까라고 추측하고 있다. 이 부분은 조금 더 알아봐야 할 것 같다. 수정한 코드는 아래와 같다.

// src/components/TodoItem.tsx
async function handleDeleteTodo() {
  try {
    const responseData = await deleteTodo(currentTodo.id)
    const newTodos = todos.filter(todo => todo.id !== currentTodo.id)
    setTodos(newTodos)
  } catch (error) {
    alert('할 일을 삭제할 수 없습니다.')
  }
}

Promise 에러 처리

Before

• 모든 api 요청에서 리턴한 promise 객체에 대해 에러 처리를 하지 않았다.

  // src/pages/Home.tsx
  useEffect(() => {
    getTodos()
      .then(res => {
        const resValue = res as Response
        setTodos(resValue.data)
      })
  }, [])

After

• try catch 구문으로 수정 후 에러 케이스를 처리해주었다.
  (얼마나 별 생각 없이 코드를 짰는지 깨달았다. 언제나 예외 케이스를 고민하고 단단한 코드를 짜야겠다고 생각했다.)

• 우선은alert()로 처리했는데 이후 더 적절한 UI로 수정하려고 계획 중이다.

  // src/pages/Home.tsx
  async function handleGetTodos() {
    try {
      const { data } = await getTodos()
      setTodos(data)
    } catch (error) {
      alert('할 일 목록을 가져올 수 없습니다.')
    }
  }

TypeScript | 타입 단언(as)

Before

• 리턴할 promise 객체의 타입을 명시하지 않았기 때문에 response.data를 사용하는 부분에서 promise 객체의 타입을 알 수 없었다.

• 따라서 응답을 받는 부분에서 타입을 작성하고자 했고, 이때 타입 단언을 사용했다.

  // src/api/todo.ts
  export const getTodos = async () => {
    try {
      const response: AxiosResponse = await axiosInstance.get('')
      return response.data
    } catch (error) {
      return error
    }
  }

  // src/pages/Home.tsx
  useEffect(() => {
    // ...
    getTodos()
      .then(res => {
        const resValue = res as Response
        setTodos(resValue.data)
      })
  }, [])

After

• api 요청 함수를 호출하는 부분보다는 함수 자체에서 리턴값의 타입을 명시하는 게 옳다고 생각하여 수정했다.

• 타입 단언은 자연스레 필요없게 되었다.
  찾아본 결과 타입 단언은 실제로 해당 타입의 인스턴스가 아닐 수 있는데 코드를 작성하는 입장에서 '이게 맞아!'라고 말하는 것이라고 이해했다. 타입 단언을 사용할 경우 타입스크립트를 사용하는 의미가 없어질 수 있으니 사용을 자제해야겠다.

  // src/api/todo.ts
  export const getTodos = async (): Promise<TodosData> => {
    const { data } = await apiInstance.get('/todos')
    return data
  }

  // src/pages/Home.tsx
  async function handleGetTodos() {
    try {
      const { data } = await getTodos()
      setTodos(data)
    } catch (error) {
      alert('할 일 목록을 가져올 수 없습니다.')
    }
  }

더 알아보고 싶은 것

• try catch 구문에서 어떤 depth까지의 에러를 캐치할 수 있는지 궁금하다. 현재 코드에서는 한 depth가 들어가도 캐치하지 못하는 게 아닌가 싶었다.
• Promise 객체의 리턴 타입 Promise<T>

개선 계획

• Form 컴포넌트가 중요한 기능이나 복잡한 코드를 갖고 있는 것은 아니라서, Form과 TodoForm을 한 파일로 합치는 것을 고민 중이다.
• 유저 동작에 대해 적절한 피드백을 되돌려 주기 위해 alert()를 다른 컴포넌트로 바꿀 계획이다.
• 중복되는 styled-components 컴포넌트들을 추출해서 파일로 분리하는 것을 고민 중이다.
• 배포를 시도하려고 했으나 Node.js로 작성된 api 코드도 배포가 필요해서 앞으로 시도해보고자 한다.