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

[오다은] Week12 #396

Conversation

O-daeun
Copy link
Collaborator

@O-daeun O-daeun commented May 7, 2024

요구사항

기본

  • TypeScript를 활용해 프로젝트의 필요한 곳에 타입을 명시해 주세요.
  • ‘/shared’, ‘/folder’ 페이지에 현재 폴더에서 검색어가 포함된 링크를 찾을 수 있는 링크 검색 기능을 만들어 주세요.

심화

  • 폴더 페이지에 상단에 있는 링크 추가하기 input이 화면에서 가려질 때 최하단에 고정해 주세요.

주요 변경사항

  • 카카오톡 SDK로 카카오톡 공유하기 기능 구현
  • 타입스크립트 설치 및 마이그레이션 작업 완료
  • 기타 리팩토링

멘토에게

  • 배포링크 : https://linkbrary-oh.netlify.app/
  • 공유하기 기능을 구현하기 위해 shared페이지의 api 호출 부분도 수정하고, 리팩토링도 여러 군데 하다보니 시간이 촉박하여 기능 구현은 못 하였고, 타입스크립트로 마이그레이션만 진행 완료하였습니다!
  • 타입에러 수정을 잘 한 게 맞는지 모르겠습니다ㅜㅜ 타입스크립트로 마이그레이션을 잘 했는지 확인 부탁드립니다!!
  • 추후에 멘토님의 코멘트를 참고하여 리팩토링 및 코드 수정, 미완성인 부분 완성해보도록 하겠습니다!
  • 그동안 상세한 코드리뷰 감사했습니다!!! 🥰

@O-daeun O-daeun requested a review from choinashil May 7, 2024 14:57
@O-daeun O-daeun added 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. 미완성🫠 죄송합니다.. labels May 7, 2024
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

모달 컴포넌트는 각 기능별로 컴포넌트를 나누어서 작업하는 방향으로 리팩토링해볼 계획입니다!!

Comment on lines +38 to +43
width: ${({ $size }) => (SIZES as any)[$size].pc}px;
height: ${({ $size }) => (SIZES as any)[$size].pc}px;

@media (max-width: 767px) {
width: ${({ $size }) => (SIZES as any)[$size].mo}px;
height: ${({ $size }) => (SIZES as any)[$size].mo}px;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

이 부분에서 그냥
${({ $size }) => SIZES[$size].pc}px;
이렇게 작성하면 에러가 발생해서 구글링하여 이 페이지를 참고해서 에러를 없애긴 했는데요..!
올바른 해결방안이 아닌 것 같아서 질문드립니다..!
any는 최대한 안 쓰는게 좋다고 배운 것 같은데 어떻게 수정하면 좋을까요..??

Copy link
Collaborator

Choose a reason for hiding this comment

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

$size 는 string 이니 다양한 문자열이 들어올 수 있는데,
SIZES 객체 내에는 키가 s 와 m 밖에 없기 때문에, s 또는 m 이 아닌 문자열이 들어왔을 때 pc, mo 에 접근을 할 수가 없는 건데요

보통 상수는 as const 를 붙여서 객체의 프로퍼티가 변하지 않는 값이라고 못박아두고 사용합니다

const SIZES = {
  s: {
    pc: 28,
    mo: 28,
  },
  m: {
    pc: 60,
    mo: 40,
  },
} as const;

위와 같이 작성하시면 되고, (그러면 고정된 값으로 타입이 추론되기 때문에 Size 라는 인터페이스는 필요없게 됩니다)

export const Img = styled.img<{ $size: 's' | 'm' }>`

이에 맞게 $size 인자도 s 또는 m 으로 줄여주셔야 하는데요

위와 같이 작성했을 때,
혹시 이후 SIZES 객체에 프로퍼티가 추가되었다면 $size 의 타입도 같이 추가해줘야 하므로 유지보수가 불편해집니다
(ex. 'l' 이 추가됐다면?)

그래서

export const Img = styled.img<{ $size: keyof typeof SIZES }>` 

와 같이 keyof, typeof 을 이용해 's' | 'm' 과 동일한 결과가 나오게 작성할 수 있습니다

  • typeof SIZES : SIZES 의 타입을 가져오는 부분
  • keyof: 해당 타입의 모든 프로퍼티의 키를 유니온 타입으로 가져오는 부분

즉 SIZES 객체의 모든 키를 유니온으로 가져오는거라서 's' | 'm' 이 되는거고, 이후 SIZES 객체에 프로퍼티가 추가되면
이 부분 코드를 수정하지 않아도 알아서 추가된 키값도 참고할 수 있게 됩니다

지금 시점에서 어려울 만한 내용이라,
이해 안가시면 일단 지금처럼 any 로 하고 넘어가셔도 되고
as const + 's' | 'm' 조합 쓰셔도 됩니다 !

Comment on lines 11 to 30
interface UserContextValue {
user:
| {
id: number;
createdAt: string;
name: string;
imageSource: string;
email: string;
auth_id: string;
}
| undefined;
setUser:
| Dispatch<SetStateAction<boolean>>
| Dispatch<SetStateAction<undefined>>;
}

export const UserContext = createContext<UserContextValue>({
user: undefined,
setUser: () => {},
});
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Context API에서 타입오류를 해결하는 데에 오래 걸렸는데, 이 코드로 해결은 됐지만 주먹구구식으로 한 것 같습니다ㅜㅜ
잘 작성한게 맞는지 리뷰해주시면 감사하겠습니다!!!

Copy link
Collaborator

@choinashil choinashil left a comment

Choose a reason for hiding this comment

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

다은님 미션 제출 시기가 밀려서 한 주에 2개 미션하느라고 촉박하셨을텐데
끝까지 포기하지 않고 최선다해서 마무리해주시느라고 수고 많으셨습니다!

타입스크립트 처음 접하면 너무 낯설고 어려운데요,
여태까지 새로운 개념 배울 때마다 그랬듯 시간이 지나면서 서서히 익숙해지니
지금 너무 고민하지 않으셔도 될 거 같아요 :)

멘토링 때마다 리액션 잘해주셔서 너무 감사하고 파트3도 화이팅하세요! 😄

const { Kakao } = window;

Kakao.cleanup();
Kakao.init('d433130307621ad24713cbacf6a9f93b');
Copy link
Collaborator

Choose a reason for hiding this comment

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

이런 키값은 보통 분리해서 관리합니다

외부에 노출되어도 되는 값이라면 상수로만 분리해도 괜찮을 것 같고,
client key, secret key 처럼 노출되서는 안되는 값이라면 환경변수로 관리합니다

환경변수의 개념과, 리액트 또는 next.js 에서 환경변수를 관리하는 방식에 대해서 찾아보세요~


const GlobalStyle = createGlobalStyle`

* {
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 +12 to +18
{link ? (
<S.StyledLink to={link} className={className}>
{text}
</S.StyledLink>
) : (
<S.Button className={className}>{text}</S.Button>
)}
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 +36 to +40
const addDefaultImage = (e: {
currentTarget: {
src: string;
};
}) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

저도 요런 케이스를 많이 작업해보지 않아서 찾아봤는데
event 의 타입을 React.SyntheticEvent<HTMLImageElement, Event> 로 처리하는 것이 일반적이라고 하네요

SyntheticEvent 는 리액트 이벤트의 일반적인 타입을 의미하고,
꺽쇠(제네릭) 내 첫번째 매개변수인 HTMLImageElement 는 이벤트가 발생한 dom 요소의 타입,
두번째 매개변수는 기본 Event 의 타입입니다

자주 사용되는 케이스가 아니라 외워야 되는건 아니고 그때그때 찾아서 대응하면 됩니다!

e.currentTarget.src = defaultImage;
};

const handleStarClick = (e: React.MouseEvent) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

React. 가 반복되고 있어서 MouseEvent 자체를 import 해와서 써도 좋을 것 같습니다

useState 나 useEffect 도 보통 React.useState 로 쓰지 않고 import 해서 쓰는 것 처럼요!

import React, { useState, MouseEvent } from 'react';

const handleStarClick = (e: MouseEvent) => {

import React, { ButtonHTMLAttributes, useEffect, useState } from 'react';
import * as S from './MenuButton.styled';

interface Props extends ButtonHTMLAttributes<HTMLButtonElement> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

요기도 기본 버튼 요소의 타입 확장하는 방식으로 잘 처리해주셨네요 👍🏽


interface Props {
title: string;
semiTitle?: string;
Copy link
Collaborator

Choose a reason for hiding this comment

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

보통 부제를 subtitle 이라고 많이 합니당

@@ -0,0 +1,55 @@
import styled from 'styled-components';

interface Responsive {
Copy link
Collaborator

Choose a reason for hiding this comment

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

추후 시간되시면 interface 와 type 의 차이도 살펴보세요!

Comment on lines +38 to +43
width: ${({ $size }) => (SIZES as any)[$size].pc}px;
height: ${({ $size }) => (SIZES as any)[$size].pc}px;

@media (max-width: 767px) {
width: ${({ $size }) => (SIZES as any)[$size].mo}px;
height: ${({ $size }) => (SIZES as any)[$size].mo}px;
Copy link
Collaborator

Choose a reason for hiding this comment

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

$size 는 string 이니 다양한 문자열이 들어올 수 있는데,
SIZES 객체 내에는 키가 s 와 m 밖에 없기 때문에, s 또는 m 이 아닌 문자열이 들어왔을 때 pc, mo 에 접근을 할 수가 없는 건데요

보통 상수는 as const 를 붙여서 객체의 프로퍼티가 변하지 않는 값이라고 못박아두고 사용합니다

const SIZES = {
  s: {
    pc: 28,
    mo: 28,
  },
  m: {
    pc: 60,
    mo: 40,
  },
} as const;

위와 같이 작성하시면 되고, (그러면 고정된 값으로 타입이 추론되기 때문에 Size 라는 인터페이스는 필요없게 됩니다)

export const Img = styled.img<{ $size: 's' | 'm' }>`

이에 맞게 $size 인자도 s 또는 m 으로 줄여주셔야 하는데요

위와 같이 작성했을 때,
혹시 이후 SIZES 객체에 프로퍼티가 추가되었다면 $size 의 타입도 같이 추가해줘야 하므로 유지보수가 불편해집니다
(ex. 'l' 이 추가됐다면?)

그래서

export const Img = styled.img<{ $size: keyof typeof SIZES }>` 

와 같이 keyof, typeof 을 이용해 's' | 'm' 과 동일한 결과가 나오게 작성할 수 있습니다

  • typeof SIZES : SIZES 의 타입을 가져오는 부분
  • keyof: 해당 타입의 모든 프로퍼티의 키를 유니온 타입으로 가져오는 부분

즉 SIZES 객체의 모든 키를 유니온으로 가져오는거라서 's' | 'm' 이 되는거고, 이후 SIZES 객체에 프로퍼티가 추가되면
이 부분 코드를 수정하지 않아도 알아서 추가된 키값도 참고할 수 있게 됩니다

지금 시점에서 어려울 만한 내용이라,
이해 안가시면 일단 지금처럼 any 로 하고 넘어가셔도 되고
as const + 's' | 'm' 조합 쓰셔도 됩니다 !

interface Props {
user: string | undefined;
src: string | undefined;
$size?: string;
Copy link
Collaborator

Choose a reason for hiding this comment

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

참고로 Profile.styled.ts 에서 $size 를 's' | 'm' 으로 한정되게 처리하기 위해서는
여기도 같이 's' | 'm' 으로 맞춰주셔야 합니다

여기가 string 으로 열려있다면 20라인에서 넘겨줄 때 타입이 맞지 않아 에러가 날거라서요

@choinashil choinashil merged commit 19ff58e into codeit-bootcamp-frontend:part2-오다은 May 9, 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