[TIL] 2024-05-21 제휴점 이미지 등록 관련 코드리뷰

Type 관련

interface ImgProps {
  // as-is
  imgType: string;
  // to-be
  imgType: 'THUMBNAIL' | 'SIGNATURE' | 'MENU';
}

이미지 타입으로 들어올 것들이 한정되어 있다면 그것이 무엇인지 명시적으로 표현해주는 것이 좋다.

type casting에 undefined 표시

// as-is
interface TcropBlop {
  blobData: string | Blob | null | undefined;
}

// to-be
interface TcropBlop {
  blobData?: string | Blob | null;
}

blobData가 처음부터 있는 게 아니라 페이지가 로드 되고 데이터를 주고 받으면서 들어오는 때도 있는데 그러면 undefined 일 때도 있는 거니까 타입 자체에 적었다.
그랬더니 위보다는 아래와 같이 optional chaining으로 표시해주는 게 더 좋다고 했다.

변하지 않는 상수는 컴포넌트 밖으로 빼기

// as-is
const SignatureImg = ({}) => {
const type = {
  THUMBNAIL: {
    title: '썸네일',
    maxImgCount: 1,
    imgWidth: 1000,
    imgHeight: 1000,
    aspectRatio: 1
  },
  SIGNATURE: {
    title: '대표',
    maxImgCount: 7,
    imgWidth: 1440,
    imgHeight: 810,
    aspectRatio: 1.777777777777777
  },
  MENU: {
    title: '메뉴판',
    maxImgCount: 7,
    imgWidth: 2000,
    imgHeight: 2000,
    aspectRatio: null
  }
};

return ()
}

// to-be
const DEFAULTINFO = {
  THUMBNAIL: {
    title: '썸네일',
    maxImgCount: 1,
    imgWidth: 1000,
    imgHeight: 1000,
    aspectRatio: 1
  },
  SIGNATURE: {
    title: '대표',
    maxImgCount: 7,
    imgWidth: 1440,
    imgHeight: 810,
    aspectRatio: 1.777777777777777
  },
  MENU: {
    title: '메뉴판',
    maxImgCount: 7,
    imgWidth: 2000,
    imgHeight: 2000,
    aspectRatio: null
  }
};

const SignatureImg = ({}) => {
	return ();
}

일단 변하지 않는 정보이니까 컴포넌트 안에 있으면 컴포넌트가 재렌더링 될 때마다 이 정보들도 계속 로딩이 되는 거라서 낭비라고 한다. 변하지 않는다면 컴포넌트 바깥에 선언해서 가져다 쓰는 것이 맞다고.

그리고 스타일의 차이도 있지만 이런 것들은 구분을 위해 변수명을 대문자로 한다고도 한다.
_(under bar) 같은 경우에 규석님은 ENV 이외에는 넣지 않고, 정수님은 넣는 편이라고 했다.

라이브러리 문서 꼼꼼하게 읽기

<CollapseBox
  defaultActiveKey={['1']}
  expandIconPosition="end"
  collapsible="icon"
  expandIcon={({ isActive }) => <div className="expand-icon">{isActive ? '닫기' : '내용확인'}</div>}
  checkbox={checked}
>

이 컴포넌트는 antd의 Collapse를 쓴 건데 collapse가 활성 되어있냐 아니냐에 따라서 아이콘이 '닫기'이거나 '내용확인'이어야 했다.
원래는 isActive를 내가 상태로 관리해서 넣어줬었는데 문서를 잘 보면 원래 이 라이브러리에 isActive라는 속성이 있어서 내가 따로 상태로 관리할 필요가 없다고 했다.
문서를 잘 읽어야 한다고 생각했다.

반복문 한 번에 돌기

// as_is
useEffect(() => {
  if (!imgFile || !imgFile?.length) {
    setState((v) => ({ ...v, modalImgType: imgType, signatureImgArr: [] }));
  } else if (imgFile) {
    const tempSignatureImg = imgFile.map((el) => {
      return { idx: el.idx ? el.idx : 0, imgInfo: el.image, name: `${imgType}_${el.idx}` };
    });

    const tempBlobArr = imgFile.map((el) => {
      const blob = new Blob([el.image], { type: 'image/png' });
      return { blobData: blob };
    });

    const tempFile = imgFile.map((el, idx) => {
      const fileName = el.image.split('/').pop();
      const metadata = { type: 'image/png' };
      return { fileInfo: new File([tempBlobArr[idx].blobData], fileName, metadata) };
    });
    setState((v) => ({
      ...v,
      modalImgType: imgType,
      signatureImgArr: tempSignatureImg,
      filesArr: tempFile,
      blobArr: tempBlobArr,
      crop: true,
      isDeletedImage: false
    }));
  }
}, [open, imgType, imgFile]);

// to_be
useEffect(() => {
    if (!imgFile || !imgFile?.length) {
      setState((v) => ({ ...v, modalImgType: imgType, signatureImgArr: [] }));
    } else if (imgFile) {
      const tempSignatureImg = [];
      const tempBlobArr = [];
      const tempFile = [];

      imgFile.forEach((el, idx) => {
        const metadata = { type: 'image/png' };
        const imgBlob = new Blob([el.image], metadata);
        const fileName = el.image.split('/').pop();

        tempSignatureImg.push({ idx: el.idx ? el.idx : 0, imgInfo: el.image, name: `${imgType}_${el.idx}` });
        tempBlobArr.push({ blobData: imgBlob });
        tempFile.push({ fileInfo: new File([tempBlobArr[idx].blobData], fileName, metadata) });
      });

      setState((v) => ({
        ...v,
        modalImgType: imgType,
        signatureImgArr: tempSignatureImg,
        filesArr: tempFile,
        blobArr: tempBlobArr,
        crop: true,
        isDeletedImage: false
      }));
    }
  }, [open, imgType, imgFile]);

로직 상 한 번에 똑같은 배열이 3개가 셋팅 되어야 했는데,
그 각각의 배열의 temp 배열을 만들어서 걔네로 데이터를 만진 다음에 맨 마지막에 실질적으로 셋팅하는 형식이었다.
나는 하나씩 하나씩 생성해서 만들게 해놨는데 to-be에서는 한 번에 돌아서 셋팅할 수 있게 정리되었다.

조건문 깔끔하게 정리

// as-is

// eslint-disable-next-line consistent-return
const saveBtnDisabled = () => {
  if (imgType === 'THUMBNAIL') {
    if (signatureImgArr?.length) {
      return !!(!checked || blobedNull);
    }
    return true;
  }
  if (imgType === 'SIGNATURE' || imgType === 'MENU') {
    return !!(!checked || blobedNull);
  }
};

// to-be
  const saveBtnDisabled = () => {
    const isImgNullWhenThumbnail = imgType === 'THUMBNAIL' && !signatureImgArr?.length;
    const isImgNullWhenSignatureMenu = (imgType === 'SIGNATURE' || imgType === 'MENU') && !signatureImgArr?.length;

    if (isImgNullWhenThumbnail) {
      return true;
    }
    if (isImgNullWhenSignatureMenu && checked) {
      return false;
    }
    return !!(!checked || blobedNull);
  };

as-is에서 eslint를 해 줘야 하는 이유가 밑의 함수가 기본적으로 return 하는 값이 없어서였다.
일단 true나 false로 리턴을 하게 만든 뒤에 렌더링 하면서 로직을 타게 만들면 적어도 eslint 제외문은 안 써도 된다고 한다.

이 함수를 만들면서... 내 머리의 한계를 느낌...
근데 의외로 분기를 하는 방법이 간단했다.
조건이 복잡하면 아예 그 로직을 빼서 알아봄직한 변수명에 담은 다음에 if문으로 분기처리하는 것이 좋은 것이라고 한다.
그럼 나중에 보는 사람도 알아보기 쉬워지는 효과까지 있다.

as 로 타입캐스팅 하지 않게 변경

// as-is
const cropperRef = useRef<HTMLImageElement>(null);
const imageElement: ReactCropperElement = cropperRef?.current as ReactCropperElement;

// to-be
const cropperRef = useRef<ReactCropperElement>(null);
const imageElement = cropperRef?.current;

onCropData 리팩토링

// as-is
const onCropData = () => {
  if (cropperRef) {
    const imageElement = cropperRef?.current;
    const cropper: Cropper = imageElement?.cropper;
    const canvas = cropper.getCroppedCanvas({ width: DEFAULTINFO[imgType].imgWidth, height: DEFAULTINFO[imgType].imgHeight });
    let tempSignatureImgArr = [];
    let tempBlobArr = [];

    if (signatureImgArr.length === 1) {
      tempSignatureImgArr = [{ imgInfo: canvas.toDataURL(), name: signatureImgArr[clickedImgIdx].name }];
      canvas.toBlob((blob) => {
        tempBlobArr = [{ blobData: blob }];
        setCropState((v) => ({
          ...v,
          crop: false,
          isDeletedImage: false
        }));
        setImageState((v) => ({
          ...v,
          signatureImgArr: tempSignatureImgArr,
          blobArr: tempBlobArr
        }));
      });
    } else if (clickedImgIdx === 0) {
      tempSignatureImgArr = [{ imgInfo: canvas.toDataURL(), name: signatureImgArr[clickedImgIdx].name }];
      canvas.toBlob((blob) => {
        signatureImgArr.shift();
        blobArr.shift();
        tempBlobArr = [{ blobData: blob }];
        setCropState((v) => ({
          ...v,
          crop: false,
          isDeletedImage: false
        }));
        setImageState((v) => ({
          ...v,
          signatureImgArr: signatureImgArr ? [...tempSignatureImgArr, ...signatureImgArr] : tempSignatureImgArr,
          blobArr: blobArr ? [...tempBlobArr, ...blobArr] : tempBlobArr
        }));
      });
    } else if (clickedImgIdx === signatureImgArr.length - 1) {
      tempSignatureImgArr = [{ imgInfo: canvas.toDataURL(), name: signatureImgArr[clickedImgIdx].name }];
      canvas.toBlob((blob) => {
        signatureImgArr.pop();
        blobArr.pop();
        tempBlobArr = [{ blobData: blob }];
        setCropState((v) => ({
          ...v,
          crop: false,
          isDeletedImage: false
        }));
        setImageState((v) => ({
          ...v,
          signatureImgArr: signatureImgArr ? [...signatureImgArr, ...tempSignatureImgArr] : tempSignatureImgArr,
          blobArr: blobArr ? [...blobArr, ...tempBlobArr] : tempBlobArr
        }));
      });
    } else {
      tempSignatureImgArr = [{ imgInfo: canvas.toDataURL(), name: signatureImgArr[clickedImgIdx].name }];
      const tempFrontArr = signatureImgArr.slice(0, clickedImgIdx);
      const tempBackArr = signatureImgArr.slice(clickedImgIdx + 1);
      canvas.toBlob((blob) => {
        tempBlobArr = [{ blobData: blob }];
        const tempFrontBlobArr = blobArr.slice(0, clickedImgIdx);
        const tempBackBlobArr = blobArr.slice(clickedImgIdx + 1);
        setCropState((v) => ({
          ...v,
          crop: false,
          isDeletedImage: false
        }));
        setImageState((v) => ({
          ...v,
          signatureImgArr: [...tempFrontArr, ...tempSignatureImgArr, ...tempBackArr],
          blobArr: [...tempFrontBlobArr, ...tempBlobArr, ...tempBackBlobArr]
        }));
      });
    }
  }
};

// to-be

const onCropData = () => {
  if (cropperRef) {
    const imageElement: ReactCropperElement = cropperRef?.current as ReactCropperElement;
    const cropper: Cropper = imageElement?.cropper;
    const canvas = cropper.getCroppedCanvas({ width: DEFAULTINFO[imgType].imgWidth, height: DEFAULTINFO[imgType].imgHeight });

    canvas.toBlob((blob) => {
      const dupSignatureImgArr = Array.from(imageState.signatureImgArr);
      const dupBlobArr = Array.from(imageState.blobArr);

      dupSignatureImgArr[clickedImgIdx] = { imgInfo: canvas.toDataURL(), name: dupSignatureImgArr[clickedImgIdx].name };
      dupBlobArr[clickedImgIdx] = { blobData: blob };

      setCropState((v) => ({ ...v, crop: false, isDeletedImage: false }));
      setImageState((v) => ({ ...v, signatureImgArr: dupSignatureImgArr, blobArr: dupBlobArr }));
    });
  }
};

이건 사실... 약간 코딩테스트 같다고 생각했다...ㅋㅋㅋㅋㅋ
나는 배열의 첫번째일 때, 마지막일 때, 중간일 때를 다 구분지어서 해야만 돌아가서 ㅎ ㅏ... 했는데
정수님이 깔끔하게 정리해주었다...
이런 건 아직 그냥 스킬의 부족이라서 보면서 많이 배움.

이 과정에서 Array.from() 이라는 내장함수를 사용하여 복사본을 생성하는 것에 관해서도 찾아보며 더 배웠다.

[React.js] state값은 사본으로! (Feat. 얕은 복사, 깊은 복사)