Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[김창희] sprint8 #110

Conversation

changhui-chan
Copy link
Collaborator

@changhui-chan changhui-chan commented Oct 19, 2024

주요 변경사항

  • 리액트 기반으로 진행한 미션 코드의 타입스크립트화

스크린샷

interface FetchDataOptions {
  url: string;
  query?: Record<string, any>;
  method?: string;
  body?: any;
  headers?: Record<string, string>;
}

interface FetchDataResult {
  data: any;
  loading: boolean;
  error: any;
}

export const fetchData = async ({
  url,
  query = {},
  method = 'GET',
  body = null,
  headers = {}
}: FetchDataOptions): Promise<FetchDataResult> => {
  const queryString = new URLSearchParams(query).toString();
  let data: any = null;
  let error: any = null;
  let loading: boolean = true;

멘토에게

  • type도 안썼고 interface도 React.FC<interface> 처럼 컴포넌트에 걸어줬는데 잘 쓴지도 모르겠습니다
  • 작성하면서 에러 천국이라 바닐라 자바스크립트에서 진행한 미션 내용까지는 반영하지 못 했습니다
  • 이전 코드 리뷰 내용은 반영하지 못 했습니다
  • POSIX 표준을 조금씩 적용하려고 작성하면서 맨 끝 줄에 엔터를 넣었는데 그것 때문에 파일 체인지가 많아 보일 수 있습니다

@changhui-chan changhui-chan changed the base branch from main to React-김창희 October 19, 2024 01:34
@changhui-chan changhui-chan self-assigned this Oct 19, 2024
@changhui-chan changhui-chan added 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. 미완성🫠 죄송합니다.. labels Oct 19, 2024
@GANGYIKIM GANGYIKIM changed the title [React] Sprint8 [김창희] sprint8 Oct 21, 2024
Copy link
Collaborator

@GANGYIKIM GANGYIKIM left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

창희님 이번주차 미션 고생하셨습니다~
반복되는 사항의 경우 한번만 리뷰를 드렸으니 고치실때 다른 부분도 찾아보세요~

url: string;
query?: Record<string, any>;
method?: string;
body?: any;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1:
any보다 더 적절한 타입을 쓰면 좋겠네요~

Suggested change
body?: any;
body?: BodyInit;

export const fetchData = async (url, query = {}, method = 'GET', body = null, headers = {}) => {
interface FetchDataOptions {
url: string;
query?: Record<string, any>;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1:
query도 URLSearchParams의 값으로 넘기는 것이니 string[][] | Record<string, string>를 추천드립니다.

const queryString = new URLSearchParams(query).toString();
let data, error, loading = true;
let data: any = null;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1:
이런경우 generic type을 이용해서 어떤 값이 응답값으로 내려오는지 알 수 있게 해주시는 것을 추천드립니다.

interface FetchDataResult<T> {
  data: T;
  ...
}

export const fetchData = async<T>({...}): Promise<FetchDataResult<T>> => {}

// 사용시
fetchData<UserInfo>()

https://www.typescriptlang.org/ko/docs/handbook/2/generics.html

}

const AddProduct: React.FC<AddProductProps> = ({
productImage = '',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2:
안쓰는 값인데 확인해보세요~

Comment on lines +4 to +10
interface AddProductProps {
productImage: string;
productTitle: string;
productDescription: string;
productPrice: string;
productTags: string[];
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2:
AddProductProps라는 이름이 있으니 내부 속성들은 product를 빼는 것이 가독성에 더 좋을 것 같습니다.

Suggested change
interface AddProductProps {
productImage: string;
productTitle: string;
productDescription: string;
productPrice: string;
productTags: string[];
}
interface AddProductProps {
image: string;
title: string;
description: string;
price: string;
tags: string[];
}

import { useState } from "react";

interface AddItemSharedData {
productImage: string | undefined;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3:
아래코드도 동일동작합니다.

Suggested change
productImage: string | undefined;
productImage?: string;

@@ -37,12 +53,14 @@ const InquireItem = ({setCommentData, commentData, content, userNickname, userIm

const handleDelete = () => {
setIsDropdownOpen(false);
setCommentData(commentData.filter((comment) => comment.content !== content));
setCommentData(commentData.filter((comment: any) => comment.content !== content));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1:
commentData는 string[]이라고 선언하셨는데 comment는 string으로 선언하면 에러가 나는 이유가 뭘까요?
commnetData 가 string[]이 맞는지 확인해보시면 좋겠습니다.

marginBottom = 0
}) => {
return (
<hr style={{ marginTop: marginTop, marginBottom: marginBottom }} className={styles['line']} />
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3:
아래 코드도 동일 동작합니다~

Suggested change
<hr style={{ marginTop: marginTop, marginBottom: marginBottom }} className={styles['line']} />
<hr style={{ marginTop, marginBottom }} className={styles['line']} />

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2:
가능하면 해당 파일의 any를 지워주세요 🥹🥹

@@ -20,7 +33,7 @@ const AddItem = () => {
setProductDescription,
setProductPrice,
setProductTags
} = useAddItemSharedData();
} = useAddItemSharedData() as AddItemProps;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1:
왜 여기서 as 가 필요할까요?
as 를 지우시고 에러가 안 나는 방향으로 코드를 변경하시는 것을 추천드릴께요~

@GANGYIKIM GANGYIKIM merged commit 4656dc7 into codeit-bootcamp-frontend:React-김창희 Oct 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. 미완성🫠 죄송합니다..
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants