Skip to content

[review] 코드리뷰용 PR#173

Open
kimisadev27 wants to merge 401 commits intocode_reviewfrom
develop
Open

[review] 코드리뷰용 PR#173
kimisadev27 wants to merge 401 commits intocode_reviewfrom
develop

Conversation

@kimisadev27
Copy link
Collaborator

코드리뷰용 PR입니다! merge 금지~!

ssuminii and others added 30 commits August 30, 2024 14:03
[fix] popular 스타일링 수정 작업
[fix] 플레이리스트 카드 컴포넌트 퍼블리싱 수정
@kimisadev27 kimisadev27 added the 🚨 Help 도움 요청! label Sep 8, 2024
@ssuminii ssuminii assigned ssuminii and unassigned ssuminii Sep 10, 2024
@ssuminii ssuminii requested a review from yujiseok September 10, 2024 07:51
Comment on lines +1 to +5
function randomTags(arr: string[], count: number = 3): string[] {
const shuffled = arr.sort(() => 0.5 - Math.random());
return shuffled.slice(0, count);
}
export default randomTags;
Copy link

Choose a reason for hiding this comment

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

sort 메서드의 경우 기존 배열을 변경할 수 있으므로 [...arr]와 같이 기존 배열을 복사해 사용하는 것이 좋습니다.

Comment on lines 30 to 34
{
root: null,
rootMargin: '0px 0px -20px 0px',
threshold: 0.5,
}

Choose a reason for hiding this comment

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

매번 객체가 재생성될 필요 없으므로, 컴포넌트 외부로 빼도 좋아보입니다.

const { data, fetchNextPage, hasNextPage, isFetching, isFetchingNextPage } =
useFetchHomePlaylists(selectedOption, PAGE_SIZE);

const playlists = data?.pages.flatMap((page) => page.playlistsData) || [];

Choose a reason for hiding this comment

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

만약 반환되는 데이터를 변경하고 싶다면, 리액트 쿼리의 select 옵션을 사용하는 것은 어떨까요? select는 렌더링 최적화의 효과도 있으니 적용해보는 것을 추천드려요.

만약 초기값을 배열로 초기화하고 싶다면,

const { data = [], ...} = useFetchHomePlaylists(xxx)

이렇게 초기화할 수도 있어보입니다.
또는 리액트 쿼리의 placeholderData를 사용하는 것도 좋습니다.

const SORT_OPTIONS = ['최신순', '좋아요순', '댓글순'];

const Home: React.FC = () => {
const [selectedIndex, setselectedIndex] = useState<number>(0);

Choose a reason for hiding this comment

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

타입스크립트가 추론할 수 있는 타입이라면, 명시적으로 타입을 지정하지 않아도 좋습니다.
컨벤션인 카멜케이스를 따른 것은 어떨까요?

const PAGE_SIZE = 5;
const SORT_OPTIONS = ['최신순', '좋아요순', '댓글순'];

const Home: React.FC = () => {

Choose a reason for hiding this comment

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

불필요한 타입 제거해주세요!

Comment on lines +25 to +29
const units = [
{ text: '억', divNum: 100000000 },
{ text: '만', divNum: 10000 },
{ text: '천', divNum: 1000 },
];

Choose a reason for hiding this comment

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

함수 외부로 추출합시다!

Comment on lines +3 to +12
export const omittedText = (
value: string,
maxLength: number = DEFAULT_MAX_LENGTH
): string => {
if (!!!value || value.trim().length < 1) return '';
if (value.length > maxLength)
return value.trim().substring(0, maxLength) + '...';

return value;
};

Choose a reason for hiding this comment

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

말줄임표가 필요하다면, css의 -webkit-line-clamp 속성을 사용할 수도 있습니다.

searchResult: [ROUTES.SEARCH()],
};

export const checkHeaderType = (path: string): HeaderProps => {

Choose a reason for hiding this comment

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

some, find등 배열의 메서드를 활용해서 리팩토링 할 수도 있겠네요.

Comment on lines +28 to +30
const [selectedImage, setSelectedImage] = useState<File | null>(null);
const [previewUrl, setPreviewUrl] = useState<string>(profileImage);
const [newChannelName, setNewChannelName] = useState<string>(channelName);

Choose a reason for hiding this comment

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

객체로 관리해 보면 어떨까요? 혹은 폼데이터로 관리해볼 수도 있겠네요!

@@ -0,0 +1,95 @@
import { useInfiniteQuery } from '@tanstack/react-query';

Choose a reason for hiding this comment

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

fetch 함수와 hook을 분리하는 것이 좋아보입니다.

Comment on lines +15 to +23
interface VALUES {
comment: string;
currentVideoIndex: number;
}

const INIT_VALUES: VALUES = {
comment: '',
currentVideoIndex: 0,
};

Choose a reason for hiding this comment

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

VALUES 타입은 불필요합니다. INIT_VALUES를 초깃값으로 전달할 경우 훅 외부로 추출하는 것을 추천합니다.

Comment on lines +26 to +30
const onChanges = {
comment: (e: React.ChangeEvent<HTMLInputElement>) => {
setValues({ ...values, comment: e.target.value });
},
};

Choose a reason for hiding this comment

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

객체 형태로 함수를 정의하는 것에 이유가 있나요?

import { useAuthStore } from '@/stores/useAuthStore';
import randomTags from '@/utils/randomTags';

const useGenerateTags = (fixedTag: string) => {

Choose a reason for hiding this comment

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

기존 인기 급상승 동영상 키워드와 랜덤 해시태그를 사용하면, 굳이 상태와 훅 없이도 태그를 만들 수 있을 것으로 보입니다.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🚨 Help 도움 요청!

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

Comments