Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughJobPins 페이지를 정적 목 데이터에서 동적 API 기반 데이터로 변경했습니다. useInfiniteQuery를 통한 무한 스크롤 기능을 추가하고, 새로운 API 함수와 타입 정의를 도입하여 아티클 목록을 페이지 단위로 로드합니다. Changes
Sequence DiagramsequenceDiagram
actor User
participant JobPins as JobPins<br/>Component
participant useInfiniteScroll as useInfiniteScroll<br/>Hook
participant ReactQuery as React Query<br/>(useInfiniteQuery)
participant API as getJobPinsArticles<br/>API
participant Backend as Backend<br/>Server
User->>JobPins: 페이지 로드
JobPins->>ReactQuery: useGetJobPinsArticles() 호출
ReactQuery->>API: getJobPinsArticles(page: 0, size: 20)
API->>Backend: GET /api/v3/articles/shared/job
Backend-->>API: JobPinsResponse (articles[])
API-->>ReactQuery: 데이터 반환
ReactQuery-->>JobPins: 첫 페이지 데이터 표시
JobPins->>useInfiniteScroll: observerRef에 sentinel div 연결
User->>JobPins: 페이지 하단 스크롤
useInfiniteScroll->>useInfiniteScroll: intersection 감지
useInfiniteScroll->>ReactQuery: fetchNextPage() 호출
ReactQuery->>API: getJobPinsArticles(page: 1, size: 20)
API->>Backend: GET /api/v3/articles/shared/job?page=1
Backend-->>API: 추가 articles[]
API-->>ReactQuery: 데이터 반환
ReactQuery-->>JobPins: 다음 페이지 데이터 추가 렌더링
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
✅ Storybook chromatic 배포 확인: |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
apps/client/src/pages/jobPins/types/api.ts (1)
1-15: JobPinArticle 및 JobPinCategory 타입 export 고려
JobPinArticle과JobPinCategory가 export되지 않아 다른 파일에서 개별 아티클이나 카테고리를 타입으로 참조할 수 없습니다. 향후 재사용성을 위해 export하는 것이 좋습니다.♻️ 개선 제안
-interface JobPinCategory { +export interface JobPinCategory { categoryId: number; categoryName: string; categoryColor: string; } -interface JobPinArticle { +export interface JobPinArticle { articleId: number; url: string; title: string; thumbnailUrl: string; memo: string; ownerName: string; category: JobPinCategory; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/client/src/pages/jobPins/types/api.ts` around lines 1 - 15, The JobPinArticle and JobPinCategory interfaces are not exported, preventing reuse elsewhere; update the file to export them by adding exports for the JobPinCategory and JobPinArticle interfaces (i.e., export interface JobPinCategory and export interface JobPinArticle) so other modules can import these types by name.apps/client/src/pages/jobPins/apis/axios.ts (1)
21-21: 타입 단언(type assertion) 사용에 대한 참고
data as ApiResponse<JobPinsResponse>타입 단언은 API 응답 구조가 변경될 경우 런타임 오류를 유발할 수 있습니다. 가능하다면 axios 인스턴스에 제네릭 타입을 전달하여 타입 안전성을 높이는 것을 고려해 주세요.♻️ 개선 제안
export const getJobPinsArticles = async ( page: number, size: number ): Promise<JobPinsResponse> => { - const { data } = await apiRequest.get('/api/v3/articles/shared/job', { + const { data } = await apiRequest.get<ApiResponse<JobPinsResponse>>('/api/v3/articles/shared/job', { params: { page, size, }, }); - return (data as ApiResponse<JobPinsResponse>).data; + return data.data; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/client/src/pages/jobPins/apis/axios.ts` at line 21, The return uses a type assertion "(data as ApiResponse<JobPinsResponse>).data" which is brittle; update the axios call to use generics so the response is typed (e.g., axios.get<ApiResponse<JobPinsResponse>>(...) or configure the axios instance generic) and then return response.data.data (or the correctly typed response property) without asserting; reference ApiResponse and JobPinsResponse and the axios request call that produces "data" to locate where to apply the generic and remove the cast.apps/client/src/pages/jobPins/apis/queries.ts (1)
10-16: 마지막 페이지 판별 로직 개선 고려현재 로직은
articles.length === 0일 때만 페이지네이션을 종료합니다. 이 경우 마지막 페이지에 1~19개의 아티클이 있을 때 불필요한 API 호출이 한 번 더 발생합니다.페이지 사이즈(20)보다 적은 아티클이 반환되면 마지막 페이지로 판단하는 것이 더 효율적입니다.
♻️ 개선 제안
+const PAGE_SIZE = 20; + export const useGetJobPinsArticles = () => { return useInfiniteQuery<JobPinsResponse>({ queryKey: ['jobPinsArticles'], - queryFn: ({ pageParam = 0 }) => getJobPinsArticles(pageParam as number, 20), + queryFn: ({ pageParam = 0 }) => getJobPinsArticles(pageParam as number, PAGE_SIZE), initialPageParam: 0, getNextPageParam: (lastPage, allPages) => { - if (lastPage.articles.length === 0) { + if (lastPage.articles.length < PAGE_SIZE) { return undefined; } return allPages.length; }, }); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/client/src/pages/jobPins/apis/queries.ts` around lines 10 - 16, The pagination stop logic in getNextPageParam currently only returns undefined when lastPage.articles.length === 0, causing one extra API call if the final page has fewer than the full page size; update getNextPageParam to treat any lastPage where lastPage.articles.length < PAGE_SIZE (20) as the final page and return undefined in that case, otherwise return allPages.length as the next page index (use the PAGE_SIZE constant or inline 20 where appropriate).apps/client/src/pages/jobPins/JobPins.tsx (1)
59-59: window.open 보안 개선
window.open으로 외부 URL을 열 때noopener와noreferrer옵션을 추가하면 tabnabbing 공격을 방지하고 referrer 정보 노출을 막을 수 있습니다.🔒 보안 개선 제안
- onClick={() => window.open(article.url, '_blank')} + onClick={() => window.open(article.url, '_blank', 'noopener,noreferrer')}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/client/src/pages/jobPins/JobPins.tsx` at line 59, The onClick handler calling window.open(article.url, '_blank') should include 'noopener,noreferrer' to prevent tabnabbing and referrer leakage; update the onClick in the JobPins component (the element with onClick={() => window.open(article.url, '_blank')}) to call window.open(article.url, '_blank', 'noopener,noreferrer') and optionally set the opened window's opener to null if you capture the return value (e.g., const w = window.open(...); if (w) w.opener = null).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/client/src/pages/jobPins/JobPins.tsx`:
- Around line 10-11: Destructure isError from the hook call (const { data,
isPending, fetchNextPage, hasNextPage, isError } = useGetJobPinsArticles()) and
add an early render branch in JobPins (e.g., if (isError) return <div>아티클을 불러오는데
실패했습니다.</div>;), so API failures from useGetJobPinsArticles are surfaced to the
user instead of silently failing.
---
Nitpick comments:
In `@apps/client/src/pages/jobPins/apis/axios.ts`:
- Line 21: The return uses a type assertion "(data as
ApiResponse<JobPinsResponse>).data" which is brittle; update the axios call to
use generics so the response is typed (e.g.,
axios.get<ApiResponse<JobPinsResponse>>(...) or configure the axios instance
generic) and then return response.data.data (or the correctly typed response
property) without asserting; reference ApiResponse and JobPinsResponse and the
axios request call that produces "data" to locate where to apply the generic and
remove the cast.
In `@apps/client/src/pages/jobPins/apis/queries.ts`:
- Around line 10-16: The pagination stop logic in getNextPageParam currently
only returns undefined when lastPage.articles.length === 0, causing one extra
API call if the final page has fewer than the full page size; update
getNextPageParam to treat any lastPage where lastPage.articles.length <
PAGE_SIZE (20) as the final page and return undefined in that case, otherwise
return allPages.length as the next page index (use the PAGE_SIZE constant or
inline 20 where appropriate).
In `@apps/client/src/pages/jobPins/JobPins.tsx`:
- Line 59: The onClick handler calling window.open(article.url, '_blank') should
include 'noopener,noreferrer' to prevent tabnabbing and referrer leakage; update
the onClick in the JobPins component (the element with onClick={() =>
window.open(article.url, '_blank')}) to call window.open(article.url, '_blank',
'noopener,noreferrer') and optionally set the opened window's opener to null if
you capture the return value (e.g., const w = window.open(...); if (w) w.opener
= null).
In `@apps/client/src/pages/jobPins/types/api.ts`:
- Around line 1-15: The JobPinArticle and JobPinCategory interfaces are not
exported, preventing reuse elsewhere; update the file to export them by adding
exports for the JobPinCategory and JobPinArticle interfaces (i.e., export
interface JobPinCategory and export interface JobPinArticle) so other modules
can import these types by name.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/client/src/pages/jobPins/JobPins.tsxapps/client/src/pages/jobPins/apis/axios.tsapps/client/src/pages/jobPins/apis/queries.tsapps/client/src/pages/jobPins/types/api.ts
| const { data, isPending, fetchNextPage, hasNextPage } = | ||
| useGetJobPinsArticles(); |
There was a problem hiding this comment.
에러 상태 처리 누락
useGetJobPinsArticles에서 isError를 destructure하지 않아 API 호출 실패 시 사용자에게 적절한 피드백을 제공할 수 없습니다.
🛡️ 에러 처리 추가 제안
- const { data, isPending, fetchNextPage, hasNextPage } =
+ const { data, isPending, isError, fetchNextPage, hasNextPage } =
useGetJobPinsArticles();그리고 렌더링 부분에 에러 상태 처리를 추가하세요:
if (isError) {
return <div>아티클을 불러오는데 실패했습니다.</div>;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const { data, isPending, fetchNextPage, hasNextPage } = | |
| useGetJobPinsArticles(); | |
| const { data, isPending, isError, fetchNextPage, hasNextPage } = | |
| useGetJobPinsArticles(); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/client/src/pages/jobPins/JobPins.tsx` around lines 10 - 11, Destructure
isError from the hook call (const { data, isPending, fetchNextPage, hasNextPage,
isError } = useGetJobPinsArticles()) and add an early render branch in JobPins
(e.g., if (isError) return <div>아티클을 불러오는데 실패했습니다.</div>;), so API failures from
useGetJobPinsArticles are surfaced to the user instead of silently failing.
| // TODO: 아티클 없는경우 UI 수정 | ||
| <p className="body2-m text-font-gray-3 mt-[4rem]"> | ||
| 아직 공유된 아티클이 없어요. | ||
| </p> |
There was a problem hiding this comment.
네넵 지금 다른 북마크/리마인드 아티클 없을 때 뷰는 있는데 여기 직무 부분은 따로 없네요.
생각보다 다른 느낌의 기능이라 같은 UI가 재사용되는 것은 아닐 것 같아서 디자인한테 문의 드려보고 반영하겠습니다~
📌 Related Issues
📄 Tasks
⭐ PR Point (To Reviewer)
직무 목록 조회 API 연결했습니다~
Summary by CodeRabbit