무한 스크롤 기능 구현 및 공용 UI, 유틸, API 관련 파일 추가 #22
Conversation
…po-simple-crud into feature/post-list
…po-simple-crud into feature/post-list
There was a problem hiding this comment.
코드를 살펴보니 packages/ui에 이미 정의된 button 컴포넌트가 있는데, 새로운 button 컴포넌트를 추가로 생성하셨네요.
혹시 기존 컴포넌트로 충족되지 않는 특별한 요구사항이 있으셨는지, 아니면 다른 의도가 있으셨는지 여쭤봐도 될까요?
There was a problem hiding this comment.
제가 직접 작성한 button 컴포넌트(shared/ui/button.tsx)는 onClick이라는 prop이 있는데 packages/ui에 정의된 button 컴포넌트를 훑어봤을 때 onClick 같은 prop이 없는 것 같아 보여서 활용하지 않았던 것 같습니다.
shadcn이나 radix ui의 컴포넌트를 사용해본 적이 없어서 slot 컴포넌트와 asChild 패턴에 대해 이해하지 못 했고, 결국 더 확장된 기능이 포함된 packages/ui의 button 컴포넌트를 알아보지 못했네요😓
원오님이 짚어주신 덕분에 그냥 지나칠 수 있었던 slot과 asChild 패턴의 구조, 역할, 사용 사례 등을 찾아보게 되었습니다. 감사합니다!
| interface CursorPaginationResponse<T> { | ||
| data: T[]; | ||
| nextCursor: string | null; | ||
| hasMore: boolean; | ||
| } | ||
| interface OffsetPaginationResponse<T> { | ||
| data: T[]; | ||
| currentPage: number; | ||
| totalPages: number; | ||
| hasMore: boolean; | ||
| } |
There was a problem hiding this comment.
CursorPaginationResponse와 OffsetPaginationResponse 인터페이스가 잘 정의되어 있네요!
혹시 post-types에 위치하기보다는 더 범용적으로 사용될 수 있는 타입이어서 별도의 파일로 분리해보는 건 어떨까요?
이렇게 하면 다른 모듈에서도 페이지네이션 인터페이스를 쉽게 재사용할 수 있을 것 같습니다.
There was a problem hiding this comment.
현재는 게시글 기능을 기준으로 작성했습니다.
추후에 댓글 기능과 병합할 때 폴더 구조, 파일명 등을 댓글 팀과 논의한 뒤 필요하다면 별도의 파일로 분리하는 게 좋을 것 같습니다.
좋은 의견 감사합니다:)
There was a problem hiding this comment.
entities 슬라이스에, model 세그먼트에 아래코드를 작성하셨는지 이유가 있을까요?
There was a problem hiding this comment.
@dgd03146 님 리뷰 감사합니다!😊
@rmkim7 님과 논의하여 해당 파일을 배치하였는데 처음에는 해당 코드를 단순하게 데이터를 다루는 요소의 일부라는 저의 생각 하에 model에 배치했던 것 같습니다.
리뷰를 통해 다시 논의해본 결과, API는 데이터를 가져오는 역할이고, 모델은 데이터를 가공하고 사용하는 역할이라고 의견을 모았습니다. 예를 들어, PostViewModel 같은 파일을 만든다면 model 폴더에 두겠지만, API는 api 폴더에 있는 게 더 명확해 보일 거란 의견을 주고 받았습니다.
따라서 데이터 소스 역할을 하는 해당 파일은 entities/post/api/에 두고, 파일명 또한 post-api.ts 로 변경할 예정입니다. 이 부분은 빠른 시일 내에 업데이트하도록 하겠습니다.
좋은 관점 제공해주셔서 감사합니다!🙂
| @@ -0,0 +1,40 @@ | |||
| import { httpClient } from '@/shared/api/http-client'; | |||
There was a problem hiding this comment.
FSD에서는 index.ts를 통해서만 외부로 공개하는 걸 원칙으로 두고 있는데 세그먼트에서 코드를 직접적으로 가져오는 것은 캡슐화 원칙을 깨는 것 아닐까 하는 생각이 듭니다. Shared에서는 segment 하나당 별도의 공개 API를 적용하는게 추천된다고 하네요.
https://feature-sliced.design/kr/docs/get-started/tutorial#%EC%97%84%EA%B2%A9%ED%95%9C-%EA%B3%B5%EA%B0%9C-api-%EC%A0%95%EC%9D%98
There was a problem hiding this comment.
우선은 팀원인 선오님과 배럴 파일을 따로 생성할지 말지 논의하다가
어떤 게 더 나을지 비교해보려고 선오님은 배럴 파일을 생성하고
저는 생성하지 않기로 해서 해당 파일의 경로를 직접 지정했습니다.
추후에 비교해보고 하나를 정할 예정인데 그때 거정님이 말씀하신 부분 참고하겠습니다.
조언 감사합니다!
There was a problem hiding this comment.
네이밍만 보면 무한 스크롤 기능을 제공하는 컴포넌트로 보이는데, 내부 코드에 post 관련 로직이 포함되어 있어서
이 컴포넌트가 특정 데이터(Post)에 종속적인 구조인지, 아니면 범용적인 무한 스크롤 컴포넌트인지 잘 모르겠습니다.
There was a problem hiding this comment.
@dgd03146 님이 말씀하신 것처럼 파일명만 보고도 구분할 수 있도록 명시하는 게 좋겠네요.
post에 관한 무한스크롤 기능을 제공하는 컴포넌트라는 것을 바로 알 수 있도록
파일명을 post-infinite-scroll으로 수정하도록 하겠습니다.
리뷰 감사합니다!
| 'use client'; | ||
|
|
||
| import { useCallback, useEffect, useRef, useState } from 'react'; | ||
|
|
There was a problem hiding this comment.
공개 API를 통해 캡슐화를 유지하는 방식으로 리팩토링하는 게 어떨까요? 현재 코드에서는 slice 내부의 특정 구현을 직접 import하는 부분이 있어서 slice 내부 구조가 변경될 때, 이를 사용하는 모든 코드도 함께 수정해야 하는 문제가 발생할 수 도 있을 것 같습니다.
There was a problem hiding this comment.
공개 API에 그런 장점이 있었군요!🧐
공개 API 사용의 장단점에 대해 좀 더 찾아본 뒤에
팀원인 선오님과 함께 이 부분에 대해 논의할 때 참고하도록 하겠습니다!
|
|
||
| const target = useRef<HTMLHeadingElement>(null); | ||
|
|
||
| useEffect(() => { |
There was a problem hiding this comment.
observer를 체크해서 데이터를 불러오는 로직은 다른 댓글 같은 곳에서 사용될 수도 있기에 커스텀 훅으로 분리해서 사용하는거도 고려해볼만 한 것 같습니다.
There was a problem hiding this comment.
이 부분은 댓글팀과 논의해서 댓글에도 무한 스크롤 방식을 사용한다는 게 결정되면
추후에 분리하는 방식으로 진행하면 좋을 것 같습니다.
@dgd03146 님 좋은 의견 감사합니다!
| hasMore, | ||
| }: InfiniteScrollProps) { | ||
| const [loading, setLoading] = useState(false); | ||
| const [posts, setPosts] = useState<Post[]>(postList); |
There was a problem hiding this comment.
서버 컴포넌트에서 props로 받아온 초기 데이터를 상태에 저장하지 않고 바로 렌더링하는 방식은 어떨까요?
There was a problem hiding this comment.
서버 컴포넌트에서 받아온 props 중에서
postList를 state로 관리하는 이유는
cursor의 값이 변할 때마다 불러오는 postList의 값이 달라져야
무한 스크롤이 제대로 작동하기 때문입니다.
postList의 초기 데이터를 그대로 렌더링해봤더니
상황에 맞게 업데이트된 postList 값이 아닌 초깃값만 가져오게 돼서
의도했던 대로 작동하지 않았습니다.
따라서 postList는 업데이트 될 수 있도록
state로 관리하는 것이 좋을 것 같습니다.
| const response = await fetch(`${BASE_URL}${followingURL}`, { | ||
| method: 'GET', | ||
| headers: commonHeaders, | ||
| }); | ||
|
|
||
| if (!response.ok) { | ||
| throw new Error(`${response.status} ${response.statusText}`); | ||
| } | ||
|
|
||
| return response.json(); |
There was a problem hiding this comment.
http-client.ts 파일에서 이미 설정된 httpClient를 사용하지 않고 fetch API를 사용하여 사용하고 계신 것 같습니다.
프로젝트의 일관성을 위해 가능하다면 이미 구성된 httpClient를 활용하는 것이 어떨까요?
별도 구현이 필요한 특별한 이유가 있으신지 궁금합니다!
There was a problem hiding this comment.
API 요청과 관련한 파일을 저는 fetch API를 사용하는 코드로 작성했고 팀원인 선오님은 httpClient를 별도로 분리하는 방식으로 코드를 작성하신 상태였는데요.
이렇게 서로 다른 방식으로 작성한 후 통일하는 과정에서 post-infinite-scroll.ts의 getInfiniteScrollData는 cursor, limit 2개의 쿼리파라미터를 props로 받아야 하는 상황인데 httpClient의 props인 endpoint, options가 cursor, limit props까지 포함할 수 있도록 추상화되지 않은 상황이었습니다.
그런데 제가 1주일 이상 자리를 비우게 된 상황이어서 이 부분은 추후 작업으로 남겨둔 채 우선 PR을 하게됐습니다. 그러다 보니 일관성 측면에서 부족한 점을 미처 수정하지 못했던 것 같습니다.
@choi1five 말씀대로 일관성을 위해 아래와 같이 수정하고자 합니다.
- getInfiniteScrollData의 쿼리 파라미터 props까지 포함할 수 있도록 httpClient를 좀 더 추상화된 코드로 변경
- post-infinite-scroll.ts에서 httpClient를 사용하는 코드로 변경
- http-client.ts
export const BASE_URL = 'http://localhost:4000/api';
interface HttpClientOptions extends RequestInit {
queryParams?: Record<string, string | number | undefined>;
}
export const httpClient = async <T>(
endpoint: string,
options?: HttpClientOptions
): Promise<T> => {
const url = new URL(`${BASE_URL}${endpoint}`);
// queryParams가 있을 경우 URL에 추가
if (options?.queryParams) {
Object.entries(options.queryParams).forEach(([key, value]) => {
if (value !== undefined) {
url.searchParams.append(key, String(value));
}
});
}
const response = await fetch(url.toString(), {
headers: {
'Content-Type': 'application/json',
},
...options,
});
if (!response.ok) {
throw new Error(`API 요청 실패: ${response.status}`);
}
return response.json();
};- post-infinite-scroll.ts
import { httpClient } from '@/shared/api/http-client';
import { GetPostsCursor } from '@/shared/types/post-types';
export async function getInfiniteScrollData(
cursor?: string,
limit?: number
): Promise<GetPostsCursor> {
return httpClient<GetPostsCursor>('/posts/infinite', {
method: 'GET',
queryParams: {
cursor,
limit,
},
});
}이렇게 수정하려고 하는데 추가로 수정이 더 필요한 곳이 있을까요?
더 나은 개선 방법이 있다면 조언 부탁드리겠습니다.
감사합니다🙂
There was a problem hiding this comment.
API 요청 시 쿼리파라미터 처리에 대한 고민이 인상적이네요! 👍
httpClient에서 queryParams라는 옵션을 통해 새로운 URL을 구성하는 코드가 있는데, 이 부분은 별도 함수로 추출하면 어떨까요? 이렇게 하면 각 함수가 더 명확한 책임을 가질 수 있습니다!
- httpClient는 순수하게 네트워크 요청에만 집중하고
- 추출된 함수는 전달받은 endpoint와 queryParams로 URL 생성에만 집중할 수 있습니다 ✨
함수 추출 후에는 fetch에서 기본 제공하지 않는 queryParams 옵션을 직접 만들기보다, httpClient 사용 시 완성된 엔드포인트를 전달하는 방식이 더 직관적일 수 있습니다. 이 방식의 큰 장점은 결합도가 낮아진다는 점입니다!
httpClient를 호출하는 쪽(예: getInfiniteScrollData 함수)이 httpClient 내부의 쿼리파라미터 처리 로직 변경에 영향을 받지 않게 되어, 두 코드 간의 의존성이 줄어들죠. 개발자 경험(DX) 측면에서도 이점이 있을 것 같아요!
쿼리스트링 관련 구현 방식으로는 두 가지가 있을 것 같아요!
qs같은 라이브러리 도입하는 방법 (의존성 추가)URLSearchParams활용하는 방법 (추가 의존성 없음)
두 가지 모두 좋은 선택지라 생각되어, 프론트 동료들과 회의를 통해 결정하면 좋을 것 같습니다.
특히 URLSearchParams는 브라우저 호환성도 좋고 별도 의존성 없이 깔끔하게 사용할 수 있어요! 💫
이렇게 변경하면 유지보수성이 더 향상될 것 같습니다!
물론, 제안해주신 코드처럼 내부적으로 추상화한 방식이 꼭 나쁜 것은 아니에요! 오히려 사용자 입장에서는 쿼리파라미터 생성 로직을 직접 구현할 필요 없이 queryParams 객체만 전달하면 되는 간결한 API를 사용할 수 있어 편리하다는 장점도 있을 수 있겠네요🧐
| <h3 className="pb-4 font-semibold">{title}</h3> | ||
| <hr /> | ||
| <p className="pt-4">{content}</p> | ||
| <br></br> |
There was a problem hiding this comment.
<br></br> 태그를 <br />(self-closing 형태)로 변경하면 코드의 일관성과 가독성이 더 좋아질 것 같습니다.
14번 라인의 <hr /> 처럼요!
There was a problem hiding this comment.
제가 못 보고 놓친 부분을 찾아주셔서 감사합니다🙏
말씀하신 대로 수정해보도록 하겠습니다!
| <Button | ||
| divClassName="text-right mr-8" | ||
| buttonClassName="bg-black text-white font-semibold text-base p-2 rounded" | ||
| > | ||
| <Link href="/posts/write">새 글 작성</Link> | ||
| </Button> |
There was a problem hiding this comment.
<Button> 컴포넌트 내부에 <Link> 컴포넌트를 사용하신 부분이 있네요. 이 구조는 HTML 웹 표준 관점에서 인터랙티브 요소 중첩으로 인해 접근성 문제가 발생할 수 있습니다.
이런 방법으로 개선해보시는 건 어떨까요?
<Link>컴포넌트에 버튼과 유사한 스타일을 적용하는 방법- 버튼의 onClick 이벤트에서 프로그래밍 방식으로 페이지 이동을 처리하는 방법
- shadcn/ui의 Button 컴포넌트(packages/ui)를 Button 컴포넌트 교체 후,
asChild속성을 활용하는 방법<Button asChild> <Link href="/posts/write">새 글 작성</Link> </Button>
There was a problem hiding this comment.
이런 구조로 사용해도 에러가 발생하지 않아서 문제가 될 거라는 생각은 못 했는데 역할이나 상호작용, 접근성 측면에서 문제가 될 수 있군요😮
제시해주신 개선 방법들을 비교해본 결과 디자인 시스템을 유지하면서도 Link 컴포넌트를 사용할 수 있고, 조건에 따른 추가 로직도 유연하게 사용할 수 있는 packages/ui의 button 컴포넌트로 교체하는 게 좋을 것 같습니다. 개선 방법에 대해 상세하게 조언해주셔서 감사합니다👍
There was a problem hiding this comment.
HTML5 명세 관련해 참고하시면 좋을 것 같아 공유드립니다!🧐
- Interactive content에 명시된 대로,
<a>태그와<button>태그는 모두 인터랙티브 콘텐츠로 분류 - MDN의 버튼 요소 문서에 따르면 permitted content는 "Phrasing content but there must be no Interactive content"로 명시
validator.w3.org/ 사이트에서 유효성 검사를 해보면 오류로 표시되는 것을 확인하실 수 있으니 참고하시면 좋을 것 같습니다!✅
| const followingURL = `/posts/infinite?${new URLSearchParams({ | ||
| ...(cursor && { cursor }), // cursor가 있으면 추가, undefined or null이면 추가 안 함 | ||
| ...(limit && { limit: limit.toString() }), // limit가 있으면 추가 | ||
| })}`; |
There was a problem hiding this comment.
쿼리 스트링을 구성하는 방식에 대한 좋은 접근입니다. URLSearchParams를 활용해 쿼리 문자열을 만드는 코드가 잘 작성되어 있네요.
템플릿 리터럴 내에서 URLSearchParams 인스턴스를 사용할 때 .toString()을 명시적으로 호출하지 않으셨는데, 이렇게 해도 JavaScript에서는 자동 변환되어 정상 작동합니다. 그러나 코드의 명확성과 가독성을 위해 명시적으로 .toString()을 호출하는 것은 어떠실까요?
또한, 이런 쿼리 파라미터 생성 로직을 utils 폴더에 별도 함수로 분리하면 여러 곳에서 재사용할 수 있고 코드 중복을 줄일 수 있을 것 같습니다. 예를 들어 createQueryString(params) 같은 유틸리티 함수를 만들어 사용하면 코드가 더 깔끔해질 것 같습니다.
There was a problem hiding this comment.
#22 (comment) 에서 http-client.ts 파일과 post-infinite-scroll.ts 파일을 수정하는 과정에서 해당 부분은 post-infinite-scroll.ts 파일에서 사라지게 될 것 같습니다.
대신 http-client.ts 파일에 다음과 같이 반영될 예정입니다.
말씀 주신 부분과 연관해서 fetch 요청할 url을 변수화한 뒤 명시적으로 .toString( )을 호출하는 방식으로 변경했습니다.
const url = new URL(`${BASE_URL}${endpoint}`);
// queryParams가 있을 경우 URL에 추가
if (options?.queryParams) {
Object.entries(options.queryParams).forEach(([key, value]) => {
if (value !== undefined) {
url.searchParams.append(key, String(value));
}
});
}
const response = await fetch(url.toString(), {추가적으로 말씀해주신 쿼리 파라미터 생성 로직 관련 유틸리티 함수는 기존의 코드가 상당부분 변경된 관계로 좀 더 고민해보도록 하겠습니다. 이 부분은 분리할 수 있을 거라고 미처 생각하지 못 했던 포인트였는데 짚어주셔서 감사합니다!
| return ( | ||
| <div> | ||
| {posts.map(({ id, title, content, author, createdAt }) => { | ||
| const localeCreatedAt = formatToLocaleDate(createdAt); |
There was a problem hiding this comment.
서버에서 응답 받은 데이터를 view에 어떻게 노출시킬지에 대한 처리가 잘 되어 있습니다. formatToLocaleDate 함수를 사용해 날짜 형식을 지역화하는 것이 좋네요.
핵심적으로 고민해볼 점은 "이러한 데이터 가공 로직을 어디에서 처리하는 것이 좋을까?"입니다. 현재는 UI 컴포넌트 내에서 직접 처리하고 계신데, 이 데이터 변환 로직을 다음과 같은 위치에 배치하는 것을 고려해볼 수 있을 것 같네요!
- API 응답 처리 레이어: API 호출 함수 내부나 그 직후에 데이터 변환을 수행하여 컴포넌트에는 이미 가공된 데이터만 전달
- 커스텀 훅: 데이터 fetching과 함께 변환 로직을 포함한 커스텀 훅을 만들어 컴포넌트 로직과 분리
- 뷰모델: 모델과 UI 사이에 별도의 뷰모델 클래스나 함수를 두어 데이터 변환 담당
이렇게 관심사를 분리하면 코드 유지보수성이 향상되고 컴포넌트는 본연의 역할인 UI 표현에 집중할 수 있지 않을까요?
There was a problem hiding this comment.
제안해주신 데이터 변환 로직 위치 중에서
API 호출 함수 내부에서 데이터를 변환하는 API 응답 처리 레이어나
데이터 fetching과 데이터 변환이 묶여있는 커스텀 훅보다는
뷰모델 함수로 분리하는 것이 범용성이나 관심사의 분리 측면에서 더 나을 것 같습니다.
그래서 아래와 같이 entities/post/model 폴더 내에 post-view-model.ts 파일을 생성하려고 합니다.
- post-view-model.ts
import { formatToLocaleDate } from '@/shared/lib/format-date';
import { Post } from '@/shared/types/post-types';
export const mapPostToViewModel = (post: Post) => ({
...post,
localeCreatedAt: formatToLocaleDate(post.createdAt),
});
뷰모델 함수 생성 후 아래와 같이 데이터 변환 로직을 분리하도록 수정할 예정입니다.
- infinite-scroll.tsx
const postViewModels = useMemo(
() => posts.map((post) => mapPostToViewModel(post)),
[posts]
);
return (
<div>
{postViewModels.map((postViewModel) => (
<PostItem
key={postViewModel.id}
linkPostId={postViewModel.id}
title={postViewModel.title}
content={postViewModel.content}
author={postViewModel.author}
localeCreatedAt={postViewModel.localeCreatedAt}
/>
))}
- useMemo를 사용해 posts가 변경되는 경우에만 mapPostToViewModel가 실행되도록 했습니다.
@choi1five님 덕분에 데이터 가공 로직을 어디에서 처리할지 고민해볼 수 있는 좋은 계기가 됐던 것 같습니다. 상세한 조언 감사합니다!
|
우선 병합하겠습니다. 나중에 PR 리뷰 관련한 수정해주시면 될 것 같습니다! |
'Features를 기준으로 slice를 구분한다'는 게 어떤 의미인지 잘 모르겠어서 여쭤봅니다. |
관련 이슈 번호
#21
핵심 변경 사항 및 이유
무한 스크롤 구현
infinite-scroll.tsx)IntersectionObserver를 활용하여 스크롤 하단 감지 후 추가 데이터 로드post-infinite-scroll.tsx)page.tsx, post-item.tsx)Post 관련 공용 fetch API 추가
http-client.ts에서 API 요청을 통합 처리post-types.ts에서 타입을 정의하여 일관성 유지api.ts에서 게시글 관련 API 요청 통합 관리공용 UI 및 유틸 추가
button.tsx: 재사용 가능한 버튼 컴포넌트 추가format-date.ts: 날짜 포맷 통일을 위한 유틸 함수 추가PR 시 참고 사항
관련 스크린샷