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

Merged

Conversation

sj0724
Copy link
Collaborator

@sj0724 sj0724 commented May 7, 2024

요구사항

기본

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

심화

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

주요 변경사항

  • 타입스크립트 적용, 타입스크립트 수정후에 다시 pr올립니다!

스크린샷

image

멘토에게

  • 이번 미션은 리액트로 작성한 코드를 타입스크립트로 마이그레이션해서 그런지 더 복잡하게 느껴지네요ㅜㅜ 처음부터 코드를 타입스크립트로 작성하면 헷갈리지 않고 조금 편하게 코드를 작성할 수 있을까요?

@sj0724 sj0724 requested a review from kiJu2 May 7, 2024 08:41
@sj0724 sj0724 self-assigned this May 7, 2024
@sj0724 sj0724 added the 매운맛🔥 뒤는 없습니다. 그냥 필터 없이 말해주세요. 책임은 제가 집니다. label May 7, 2024
@kiJu2
Copy link
Collaborator

kiJu2 commented May 8, 2024

수고 하셨습니다 ! 위클리 미션 하시느라 정말 수고 많으셨어요.
학습에 도움 되실 수 있게 꼼꼼히 리뷰 하도록 해보겠습니다.


function App() {
const userId = 1;
const userId: number = 1;
Copy link
Collaborator

Choose a reason for hiding this comment

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

다음과 같이 타입 추론을 통해 코드를 간결하게 작성할 수 있습니다 !:

Suggested change
const userId: number = 1;
const userId = 1; // `userId`는 `Number`입니다 😊

import { ReactNode } from 'react';
import ReactDom from 'react-dom';

const ModalPortal = ({ children }: { children: ReactNode }) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

굳굳 ! 포탈을 통해서 모달을 만드셨군요 😊👍

@@ -0,0 +1,82 @@
import axios from '../instance/axiosInstance';
Copy link
Collaborator

Choose a reason for hiding this comment

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

(제안)axios라고 작성되면 axios 라이브러리를 직접 사용하는 것과 혼동이 올 수 있을 것 같아요 !

axios 인스턴스임을 명시하는게 더욱 좋을 것 같습니다 😊

Suggested change
import axios from '../instance/axiosInstance';
import instance from '../instance/axiosInstance';


export async function getSampleUser() {
try {
const { data } = await axios.get('/sample/user');
Copy link
Collaborator

Choose a reason for hiding this comment

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

AxiosResponse 타입을 지정해볼까요?:

Suggested change
const { data } = await axios.get('/sample/user');
const { data } = await axios.get<{
id: string;
email: string;
// ...
}>('/sample/user');

Comment on lines +2 to +5

export async function getSampleUser() {
try {
const { data } = await axios.get('/sample/user');
Copy link
Collaborator

Choose a reason for hiding this comment

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

외부에서 해당 API의 타입이 필요할 경우 다음과 같이 작성할 수 있습니다:

Suggested change
export async function getSampleUser() {
try {
const { data } = await axios.get('/sample/user');
type GetSampleUserResponse = AxiosResponse<{
id: string;
email: string;
//...
}>
export async function getSampleUser() {
try {
const { data } = await axios.get('/sample/user');

Copy link
Collaborator

Choose a reason for hiding this comment

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

혹은 엔티티 타입을 정의하고 다음과 같이 작성해볼 수도 있어요:

Suggested change
export async function getSampleUser() {
try {
const { data } = await axios.get('/sample/user');
// 외부 파일
export type SampleUser = {
id: string;
email: string;
// ....
}
// src/api/api.ts
type GetSampleUserResponse = AxiosResponse<Pick<SampleUser, 'id' | 'email' /* ... */>>
export async function getSampleUser() {
try {
const { data } = await axios.get('/sample/user');

}
}

export async function getFolderList(id: number, folderId: number) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

파라메터가 어떤 것들이 있는지 타입과 네이밍이 명확해서 보기 좋네요 😊👍

Copy link
Collaborator

Choose a reason for hiding this comment

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

또한, folderIdstring으로 활용되지 않나요?

추 후 수식과 관련된 로직이 수행될 것 같지는 않아보여요 !

Comment on lines +4 to +10
function Button({ children, size }: { children: ReactNode; size: number }) {
return (
<>
<S.Cta size={size}>{children}</S.Cta>
</>
);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

React에서 제공하는 HTML type을 사용해보시는건 어떨까요?

다음과 같이 리액트에서 제공하는 Attributes를 사용할 수도 있습니다:

import cn from 'classnames';
import { ButtonHTMLAttributes } from 'react';

interface Props extends ButtonHTMLAttributes<HTMLButtonElement> {
  variant?: 'primary' | 'secondary' | 'none';
}

export default function MelonButton({ className, variant, ...rest }: Props) {

Copy link
Collaborator

Choose a reason for hiding this comment

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

상준님 코드에서는 다음과 같이 적용해볼 수 있어요:

type ButtonProps = { size: 'lg' | 'md' | 'sm' } & ButtonHTMLAttributes<HTMLButtonElement>;
function Button({ children, size }: ButtonProps) {
  return (
      <S.Cta size={size}>{children}</S.Cta>
  );
}


const { url, description } = item;

const createdText = `${createdAt.time} ${createdAt.result} ago`;
Copy link
Collaborator

Choose a reason for hiding this comment

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

다음과 같이 최적화 시킬 수 있어요 😊

Suggested change
const createdText = `${createdAt.time} ${createdAt.result} ago`;
const createdText = useMemo(() =>`${createdAt.time} ${createdAt.result} ago`, [createdAt]);

@@ -1,18 +1,19 @@
import styled from 'styled-components';

export const FolderName = styled.span`
export const FolderName = styled.span<{ $select: string | boolean }>`
Copy link
Collaborator

Choose a reason for hiding this comment

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

하단 조건부를 보면 boolean으로 취급되는 경우가 없는 것 같아요:

Suggested change
export const FolderName = styled.span<{ $select: string | boolean }>`
export const FolderName = styled.span<{ $select: boolean }>`

또한, booleanstring이 유니온 타입으로 지정될 일은 흔치 않을 것으로 보입니다 😃

toggleModal,
}: {
link: Folders;
setFolderName: React.Dispatch<React.SetStateAction<string>>;
Copy link
Collaborator

Choose a reason for hiding this comment

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

굳굳 ! 리액트 Dispatch타입을 적절하게 사용하셨군요 👍

@kiJu2
Copy link
Collaborator

kiJu2 commented May 8, 2024

상준님 정말 수고 많으셨습니다 !
함께 하면서 팀장님으로서 정말 고생 많으셨고.. 끝까지 위클리 미션 빼먹지 않고 항상 제출해주셨군요 😊👍
멘토링 미팅 때 상준님께서 항상 적극적으로 참여하시려는 모습이 보였습니다. 덕분에 멘토링 미팅 진행할 때 힘이 났습니다 ㅎㅎㅎ..🥺🥺🥺 고맙습니다 !

주말에 주강사로 활동하고 있으니, 궁금한거 있으시면 편하게 들어와서 물어보세요 ㅎㅎㅎ

@kiJu2 kiJu2 merged commit 54735fa into codeit-bootcamp-frontend:part2-박상준 May 8, 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