치춘짱베리굿나이스

[프리온보딩] 220515 강의 메모 01 (코드리뷰) 본문

프로젝트/원티드 프리온보딩

[프리온보딩] 220515 강의 메모 01 (코드리뷰)

치춘 2022. 5. 17. 19:48

코드리뷰

코드리뷰 #1 (내코드)

readme

  • 리드미만 보면 전교1등이다 (ㅋㅋㅋㅋ) 하이테크 볼펜 20개씩 있을것같다
  • 얼마나 타자를 열심히 쳤으면 손가락이 ㅋㅋ 와 ㅋㅋ

검색어 없을 때 불필요한 API 요청 없애기

export const MovieListContainer = () => {
    const res = fetchWrappedMovieData(setValue, 1);
    // 이 부분 때문에 컴포넌트가 로딩될 때마다 검색어가 없어도 값을 fetch한다
    ..
}
  • 페이지 리렌더링할 때마다 불필요하게 API 요청이 한번씩 더 들어간다
  • 조건문 등을 이용해서 검색어가 없을 땐 API 요청을 없애자
  • 서버가 헛수고하잖어!!

드래그앤 드랍 할 때 내부 문자열 긁히는 거 막기

user-select: none;
  • CSS에서 user-select 속성을 이용하자

section 태그를 일반 컨테이너로 사용하지 마세요

  • h1 태그가 들어갈 법한 (게시글 같은) 요소들만 section을 사용하자
  • 스타일링을 위해서라면 일반 div를 사용하기 (MDN 참고)

import 문 잘 정리하기

import { RecoilRoot } from 'recoil'; // 라이브러리 임포트
import { Router } from './router'; // 상대 / 절대경로 임포트
  • 센스있게 넣어주면 나중에 코드 보기 쉽다

router에서 슬래쉬 제거

<Route path='/favorites' element={<FavoritePage />} />
<Route path='favorites' element={<FavoritePage />} />
  • react-router-v6에서는 슬래쉬 빼는 것이 좋다

return null 하는 컴포넌트는 커스텀 훅으로 분리하기

export const FetchFavoriteData = () => {
    const setFavoriteData = useSetRecoilState(favoriteDataState);

    useMount(() => {
        ...
    };

    return null;
};

// 훅으로 전부 분리하기
export const Router = () => {
    useFetchFavoriteData();
    ...
};
// 라우터도 일종의 컴포넌트라고 볼 수 있기 때문에
// 커스텀 훅을 전역적으로 (모든 페이지에서) 동작하게 하고 싶으면
// 라우터에 커스텀 훅을 넣으면 된다
  • useMountuseEffect같은 걸 쓰려고 null 반환하는 컴포넌트를 만드는 건 너무 안티패턴이다
  • 커스텀 훅으로 빼서 router 내에 넣기
  • router도 컴포넌트야 컴포넌트!!!!!! 커스텀 훅 넣을 수 있따

커스텀 훅

// 커스텀 훅
export const useFetchFavoriteData = () => {
    const setFavoriteData = useSetRecoilState(favoriteDataState);

    useMount(() => {
        ...
    };

    return null;
};

// 라우터
export const Router = () => {
    useFetchFavoriteData();
    ...
}

// 위의 코드는 이것과 같다
export const Router = () => {
    const setFavoriteData = useSetRecoilState(favoriteDataState);

    useMount(() => {
        ...
    };

    return null;
    ...
};
  • 커스텀 훅이 거창한게 아니고 커스텀 훅이 위치한 자리에 훅 내부 코드 (useEffect, useState 등) 를 그대로 넣어준 것과 같음
  • useEffect, useState를 쓰고 싶은데 렌더링 요소는 필요가 없다면 커스텀 훅으로 분리하는 것을 적극 추천함

React. 로 시작하는 요소들 적절하게 불러오기

const handleValueChange = (e: React.ChangeEvent<...>) => {};

// 위의 코드를 간단하게 적기
import { useState, ChangeEvent} from 'react';

const handleValueChange = (e: ChangeEvent<...>) => {};
  • 이렇게 라이브러리로부터 불러오면 React.어쩌구로 길게 적을 필요가 없다

require 쓰지마시오

const store = require('store');

// yarn add @types/store 설치하기
import store from ('store');
  • require 거의 쓸 일 없어야한다, import 쓰시오!!!
  • store에서 오류났던건 @types/store 설치 안했기 때문
  • 아주 혼나야겠지요? 에러가 나면 에러를 열심히 안읽어봐 ㅡ.ㅡ

깃은 대소문자를 무시한다

$> git config core.ignorecase false

// 위의 설정 후 제대로 동작을 하지 않는다면 캐시삭제
$> git rm -rf --cached .
$> git add .
$> git commit -m "캐시 삭제"
  • 파일명 소문자였던거 대문자로 바꿔도 깃에서는 인식을 못하므로 깃허브에도 제대로 푸시가 안 될 가능성이 있음
  • 깃의 대소문자 무시 설정을 꺼야한다
  • 설정을 바꿨을 때 오류가 날 경우 캐시도 삭제해야한다

axios() 대신 axios.get()

axios({
    method: 'GET',
    params: {
        apikey: process.env.REACT_APP_MOVIE_API_ID,
        s: searchValue,
        page,
    },
    url: process.env.REACT_APP_MOVIE_API_URL,
});

axios.get(process.env.REACT_APP_MOVIE_API_URL ?? '', {
    params: {
        apikey: process.env.REACT_APP_MOVIE_API_ID,
        s: searchValue,
        page,
    }
});
  • 심플하게 적읍시다 장황하면 구려!! 보는사람 피곤해~~

axios는 then으로 후속동작 정의하기

const responseData = await fetchMovieData(searchValue, pages + 1);
if (responseData.Response === 'True') {
    setMovieArray((prevState) => ...);
    setPages((prevState) => ...);
}

// 위의 코드는 이렇게 정리할 수 있다
fetchMovieData(searchValue, pages + 1)
    .then((res) => {
        if (res.Response === 'True') {
            setMovieArray((prevState) => ...);
            setPages((prevState) => ...);
        })
    .catch((e) => {
        // 에러핸들링
    });
  • axios 자체가 promise이기 때문에 async await 필요없다
    • 대신 then을 이용해서 후속 동작을 정의합시다
    • (ㅎㅎ async, await가 없으면 비동기 핸들링이 안 될 것만 같은 느낌적인느낌..)

any가 나오면 거부감부터 느끼기

  • 구토하기 우욱
  • axios의 반환값을 직접 마우스 올려서 확인해보고, 타입을 꼭 any 말고 다른걸로 정의하자

scss에서 import 대신 use

@import 'styles/mixins/test';

...
@include testMixin(1, 1);

// 이렇게 수정하자
@use 'styles/mixins/test'

...
@include test.testMixin(1, 1);
  • @import 쓰면 코드가 통째로 그 위치에 들어가기 때문에, 다중 import를 사용하다 보면 값이 중복되어 양이 방대해질 수 있다
  • @use를 넣으면 사용하는 곳에서만 사용하게 되므로 트리가 꼬이지 않게 해 주므로 적합하다
  • @import를 거의 쓰지 않도록 하자

검색어를 입력했을 때 검색결과를 요청하는 API로 연결이 안 된다

  • 검색어를 recoil 상태값으로 한번 저장하고, 상태값이 바뀔 때 API 요청하고...
  • 알아보기 힘든 코드가 되어버렸다
  • 전역 상태값을 useEffect처럼 사용해버렸군,,

기타 팁

  • API 명세랑 실제 데이터랑 항상 같지않습니다.. ㅡ.ㅡ
  • 배포페이지를 만들어두면 코드리뷰 시에 편하다
  • 거창하게 짜는 건 못짜는거다, 심플하고 알아보기 쉽게 작성하기
  • 현재 어느 탭에 와 있는지 내비게이션 바에 스타일 주기
  • 즐겨찾기는 왜 즐겨찾기가 없습니다 안해놨을까?
  • 너무 거창해! 걸어다닐 때 BGM 나올 것 같다 (사실 내가 부르는타입이다)
  • 검색 페이지 컴포넌트 이름이 MainPage 말고 SearchPage 였으면 더 좋았을듯
  • setinputValuesetInputValue (실수한듯 ;)

코드리뷰 #2

API 키 노출시키지 않도록 꼭 env에 넣어놓고 사용하기

REACT_APP_API_KEY=apikey
REACT_APP_API_URL=apiurl
  • 보안 문제가 크다
  • 다른사람이 그 API 키로 들어가서 데이터를 수백개씩 불러온다고 생각해보면.. ㄷㄷ

ul 태그 바로 아래에 li 태그 넣으세요

<ul>
    <li key={어쩌구} />
        <button onClick={handleButtonClick}>
            버튼이다옹
        </button>
    </li>
</ul>
  • onClick같은 이벤트 동작을 정의할 때는 버튼을 li로 감싸는 한이 있더라도 ul과 li는 한쌍으로 움직여야 한다

dd, dt, dl

<dl>
    <dt>제목</dt>
    <dd>내용</dd>
</dl>
  • dtdd는 한 쌍으로 움직인다
  • dt제목은 dd내용이다 라고 생각하면 좋음
  • dt, dd 한 쌍을 dl로 감싸주자

폰트 불러오기

font-family: Inter, Arial, PingFangSC-Regular, "Microsoft YaHei", ...
  • 적힌 순서대로 (예시에선 Inter부터) 로컬이나 인터넷에서 불러오기를 시도하고 없으면 다음걸로 넘어간다
  • 한국어나 일본어 등 다른 언어가 섞여있을 경우 언어별로 서로 다른 폰트를 불러와서 사용하므로, 폰트의 통일성을 위해서 여러 언어를 지원하는 폰트를 앞에 배치하면 좋다

image 불러오기 실패했을 때 onError 이벤트

<image src={src} alt={imageStr} onError={handleImageError} />
  • onError 핸들러를 이용하여 사진 불러오기에 실패했을 경우 행동을 정의할 수 있다
  • 이를 이용하여 다른 사진을 출력하게 하는 등 예외처리가 가능함

모달 뒤에 블러 넣기

backdrop-filter: blur(2px);
  • 모달 백그라운드 작성할 때 좀 더 그럴듯하게 작성할 수 있다
  • 파이어폭스에서 지원안하긴 한다

—moz 이런애들 하나하나 신경쓰지 않아도 된다

  • 웹팩 내에 있는 autoprefixer가 스타일 접두사들 (webkit, moz 등)을 알아서 다 처리해준다
  • CRA가 어지간한 건 대신 다 해주기 때문에 정말 킹갓엠페러제너럴이다
  • 나는 바보같이 —-webkit 이랑 —-moz를 다 작성하고 앉았었네!

기타 팁

  • aside 태그는 화면에서 본문과 관계없는 내용만 (광고, 모달 같은 것) ⇒ 버튼에 aside를 사용하는 등은 높은 확률로 잘못되었다
  • splice, slice, findIndex 등을 남용하기보단 filter 메서드를 잘 사용하면 깔끔한 코드 작성 가능하다
  • NavLink + isActive 활용해서 내비게이션바 스타일 잘 적용하기 굿
  • 폴더구조는 취향껏 하기 (회사마다, 프로젝트마다, 개인마다 갈리기 때문) ⇒ 통일성만 지키면 됨
  • 파이어폭스 고려하지 않는 곳도 많음 (좀 마이너한 편)

코드리뷰 #3

async await이랑 then을 같이 쓸 필요가 없다

  • Promise 객체는 then으로 후속동작을 정의할 수 있으므로 async await를 쓸 필요가 없다
  • 같이 쓸 필요는 더더욱 없음!
  • 후속동작을 정의하되 반드시 then, catch 이후에 처리되어야 할 동작이 있다면 굳이 async await 쓰지말고 finally 사용하기

특정 조건에서 return을 사용하여 탭 수 줄이기

if (isShown)
    setState((prevState) => prevState + 1);

if (!isShown) return;
setState((prevState) => prevState + 1);
  • 줄 수는 똑같지만 아래줄이 탭이 적어서 조금 더 보기 편하다
  • 조건문과 실행 코드를 가급적 간단하게 쓸 수 있는 방향으로 작성하기

text-align: start 대신 left

  • text-align: start / text-align: end는 추가된지 얼마 안된 문법이다
  • 또한 text-align: start는 아랍어와 같이 우측에서 좌측으로 쓰는 언어 (우횡서 언어) 들을 위해 만들어진 스타일이라

splice 대신 filter 써버릇하기

  • splice 쓰면 뭐가 뭔지, 뭐하는 동작인지 더 알아보기 힘들어진다
  • 배열 자르는 기능이랑 더하는 기능이 둘 다 있기 때문인듯...

기타 팁

  • 쿼리스트링으로 검색결과 넣은 것 아주 훌륭 = >무엇을 검색했는지 알아보기도 쉽고 공유하기도 쉽다
  • 모달 바깥 클릭했을 때 닫히는 것 구현한 것도 좋음
  • 레이아웃 쓴 것도 좋았다
  • !length 대신 length === 0 을 사용하기 (좀 더 가독성에 좋다)
  • & > div 대신 클래스명을 사용하는 게 알아보기 쉽다
  • 렌더링 단 (jsx / tsx에서 html태그 반환하는 부분) 에 괄호가 들어간 함수를 넣지 말자
  • 태그 백그라운드에 이미지 넣을 땐 background-image 또는 backgroundImage 속성 사용하기
  • 여러분.. 리액트쿼리가 시급하군요
  • 드래그앤 드랍 직접구현도 굿

코드리뷰 #4

스타일링 팁

  • border-radiuspadding, margin 같은 건 px이나 rem 사용하자..
    • 박스가 정사각형이 아닌 이상 가로 세로 기준이 달라서 찌그러진다
    • px, rem, em같은 값 써야 브라우저나 창 크기가 줄어들었을 때 보기 예쁘다
  • 박스 안에 값들이 어떤 거는 좌측정렬, 어떤 거는 우측정렬 이런 식이면 안이쁘다
  • text-transform: uppercase, lowercase로 글자를 대문자 소문자 맞춰줄 수 있다
  • 회색은 아예 밝거나 진하지 않은 이상 안 쓰는게 좋다 (너무 칙칙해짐)
  • 이미지 불러오기 실패했을 경우도 세팅해주는 것이 이쁘다

line clamp mixin

@mixin lineClamp($line, $lineHeight) {
/* stylelint-disable-next-line value-no-vendor-prefix */
    display: -webkit-box;
    height: #($lineHeight * $line)px;
    overflow: hidden;
    word-break: break-all;
    -webkit-line-clamp: $line;
    -webkit-box-orient: vertical;
}
  • 문자열의 최대 라인 수와 라인 높이를 지정하여, 띄어쓰기나 단어 브레이크 등으로 다음 줄로 넘어갔을 때 지정한 라인 수를 오버하면 ellipsis 등으로 가려줄 수 있다
  • 속성이 많이 들어가므로 mixin으로 빼주는 게 좋다
  • 주석으로 stylelint 뮤트시키지 않으면 해당 줄 (display: -webkit-box) 이 린터에 걸려버림
  • -webkit-box가 크롬 계열 브라우저에서만 확실히 동작해서 다른 브라우저에서는 보장이 되지 않긴 함 (테스트 시에는 일단 사파리랑 파이어폭스는 됐다)

내비게이션 바 같은 고정컴포넌트는 position: fixed 속성 이용하기

  • position: fixed, top: 0, left: 0 등등
  • translate-y는 너무 과하다
  • 특히 속성 잘못 먹이면 내비게이션 바의 위치가 꼬일 수 있음
  • position: sticky는 비추

map 메서드

  • 배열의 map 메서드 쓸 거면 ul + li 태그 고려하기
  • 반환값이 필요 없을 땐 map 말고 forEach 메서드 사용하기

map + push, forEach + push 대신 reduce 쓰기

arr.map((item: ITempData) => {
    const { value } = item;
    ...
    tempArr.push({
        title: item.title,
        year: item.year,
        ...
        poster: item.poster,
    });
});

arr.reduce((arr: ITempData[], item: ITempData) => {
    const { title, year, poster, ... } = item;
    ...
    return {
        title,
        year,
        ...
        poster,
    }
), []); // 초기값은 빈 배열임
  • 배열 메서드 내에서 push와 함께 map, slice, split, filter등을 조합하기보단 reduce를 알아보자
  • reduce는 정의한 초기값에 함수의 반환값을 밀어넣는다
  • 보통 배열 내 값들 합 구할 때 많이들 사용함

기타 팁

  • IntersectionObserver는 어렵다... :sad_blob:
  • warning 다 지워주자..
  • .env 깃에 올리지 말자 (api key같은거 잘 숨기자)
  • 기본 alert 쓰지않기
  • 주석 다 지워주기
  • NavLink 쓰면 클래스랑 상태값 복잡하게 쓸 필요 없이 페이지별 하이라이트 가능함
  • i는 보통 index를 의미하므로, item 같은 다른 명사에 대해서는 i로 줄이지 말자
  • form 안에 submit 속성 버튼 없어도 엔터치면 submit이 되긴 한다
  • is~~~ 같은 이름의 변수는 대개 boolean값이 들어오리라 예상하게 된다
  • html에 스타일 속성 넣지 않기 ⇒ 예상치 못한 버그 유발
  • 서버에서 response로 날아온 값을 mutate하는 것은 좋지 않다 ⇒ 결과가 너무 거대할 경우 난리난다
Comments