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 #241

Conversation

ayoooyh
Copy link
Collaborator

@ayoooyh ayoooyh commented Aug 2, 2024

요구사항

기본

  • [x]스프린트 미션 1 ~ 7에 대해 typescript를 적용해주세요

심화

주요 변경사항

스크린샷

image

멘토에게

  • 셀프 코드 리뷰를 통해 질문 이어가겠습니다.

@ayoooyh ayoooyh added the 순한맛🐑 마음이 많이 여립니다.. label Aug 2, 2024
@ayoooyh ayoooyh requested a review from arthurkimdev August 2, 2024 10:48
@ayoooyh ayoooyh self-assigned this Aug 2, 2024
Comment on lines 27 to 32
interface IconProps {
IconComponent: React.FC<React.SVGProps<SVGSVGElement>>;
size: number;
fillColor: string;
outlineColor: string;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

타입 추가 좋아요. 👍 동작하는 원리도 알면 좋을 것 같아서 남깁니다.

IconComponent: React.FC<React.SVGProps<SVGSVGElement>>;

위 경우는 중첩 제네릭이 발생한 경우인데요, 그 이유는 아래처럼 React.FC는 제너릭을 인자로 받고 있기 때문이에요.
참고로 FC는 실제로 FunctionComponent 인터페이스의 별칭입니다.
이 함수는 P 타입의 props를 받고 ReactElement를 반환하고 있어요. 실제 내부 타입 정의는 아래처럼 되어있을거예요.
여기서 P는 React.SVGProps<SVGSVGElement> 값이 들어왔을거구요.

type FC<P = {}> = FunctionComponent<P>;

interface FunctionComponent<P = {}> {
  (props: P, context?: any): ReactElement<any, any> | null;
  propTypes?: WeakValidationMap<P> | undefined;
  contextTypes?: ValidationMap<any> | undefined;
  defaultProps?: Partial<P> | undefined;
  displayName?: string | undefined;
}

Comment on lines +5 to +9
interface DropdownMenuProps {
onSortSelection: (v: string) => void;
}

function DropdownMenu({ onSortSelection }: DropdownMenuProps) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

현재 v: string은 조건 없이 타입이 너무 광범위해요. 가능하다면 아래 예시처럼 구체적인 타입을 사용해서 옵션을 제한적으로 두는 것이 좋습니다.

type SortOption = "recent" | "favorite";

type DropdownMenuProps = {
  onSortSelection: (option: SortOption) => void;
};

Comment on lines +5 to +17
interface ItemProps {
id: string;
name: string;
price: number;
images: string[];
favoriteCount: number;
}

interface Item {
item: ItemProps;
}

function ItemCard({ item }: Item) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

잘 해주셨는데요, 여기선 현재 Item 타입이 불필요해서 아래처럼 변경할 수 있겠어요. 😁😁

interface ItemProps {
  id: string;
  name: string;
  price: number;
  images: string[];
  favoriteCount: number;
}

function ItemCard({ item }: { item: ItemProps }) {
  // ...
}

Comment on lines +19 to +23
interface DeleteButtonProps {
onClick: (e: MouseEvent<HTMLButtonElement>) => void;
label: string;
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

MouseEvent 내부는 아래처럼 생겨서 HTMLButtonElement 값이 T 제너릭에 위치하고, UIEvent<HTMLButtonElement> 로 변경되면서 일반적인 UI 이벤트 속성을 상속 받게 되는거예요.

즉 아래처럼 되면서 동작하게 됩니다.

interface MouseEvent<HTMLButtonElement, E = NativeMouseEvent> extends UIEvent<HTMLButtonElement, E> {

아래는 React에서 제공하는 MouseEvent 타입이에요. 예시라서 실제 코드와는 다소 다를 수 있습니다.

interface MouseEvent<T = Element, E = NativeMouseEvent> extends UIEvent<T, E> {
    altKey: boolean;
    button: number;
    buttons: number;
    clientX: number;
    clientY: number;
    ctrlKey: boolean;
    metaKey: boolean;
    movementX: number;
    ...
    getModifierState(key: string): boolean;
}

Comment on lines 46 to 57
interface InputItemProps {
id: string;
label: string;
value: string;
onChange: (e: ChangeEvent<HTMLInputElement | HTMLTextAreaElement>) => void;
placeholder: string;
onKeyDown?: (
e: KeyboardEvent<HTMLInputElement | HTMLTextAreaElement>
) => void;
isTextArea?: boolean;
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

굳! 잘하셨습니다. 👍

Comment on lines +26 to 43
type Tag = string;

function AddItemPage() {
const [name, setName] = useState("");
const [description, setDescription] = useState("");
const [price, setPrice] = useState("");
const [tags, setTags] = useState([]);
const [tags, setTags] = useState<Tag[]>([]);

// 중복 등록 막기 위해 tags 배열에 없는 것 확인하고 삽입
const addTag = (tag) => {
const addTag = (tag: Tag) => {
if (!tags.includes(tag)) {
setTags([...tags, tag]);
}
};

const removeTag = (tagToRemove) => {
const removeTag = (tagToRemove: Tag) => {
setTags(tags.filter((tag) => tag !== tagToRemove));
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

해당 경우는 아래처럼 간단하게 표현 하는게 더 좋아요~

const addTag = (tag: string) => {

tsconfig.json Outdated
Comment on lines 1 to 20
{
"compilerOptions": {
"target": "ES6",
"lib": ["dom", "dom.iterable", "esnext"],
"allowJs": true,
"skipLibCheck": true,
"esModuleInterop": true,
"allowSyntheticDefaultImports": true,
"strict": true,
"forceConsistentCasingInFileNames": true,
"noFallthroughCasesInSwitch": true,
"module": "esnext",
"moduleResolution": "node",
"resolveJsonModule": true,
"isolatedModules": true,
"noEmit": true,
"jsx": "react-jsx"
},
"include": ["src"]
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

tsconfig 설정 잘하셨습니다.
각각 어떤 의미가 있는지 공부해보면 좋겠어요. 🙂
컴파일 대상 파일들을 어떻게 변환할지 다양한 옵션들이 많거든요.

https://www.typescriptlang.org/tsconfig/

Copy link
Collaborator

@arthurkimdev arthurkimdev left a comment

Choose a reason for hiding this comment

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

윤하님 타입스크립트 작업하시느라 수고하셨습니다! 🙏

@arthurkimdev arthurkimdev changed the base branch from main to React-신윤하 August 4, 2024 23:22
@arthurkimdev arthurkimdev merged commit 75932d6 into codeit-bootcamp-frontend:React-신윤하 Aug 4, 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