fix: 커피솝 페이지 필터 state에서 query parameter로 변경 #1777
Conversation
- usePageQueryParams 사용 - 검색어 상태 2개에서 1개로 변경
- 선택된 값 쿼리스트링으로 가져오기 - request에 맞는 값으로 쿼리키 변경
|
|
🚀 프리뷰 배포 확인하기 🚀 |
| }; | ||
| const { data, isLoading } = useGetMembersCoffeeChat(queryParams); | ||
|
|
||
| const formatQueryParams = (query: ParsedUrlQuery): { [key: string]: string } => { |
There was a problem hiding this comment.
현재 코드에서 normalizeCareer, normalizeSection 및 ...===('전체') return undefined; 등 값을 변환하는 로직이 반복적으로 적용이 되고 있어서 해당 로직을 합쳐서 처리한다면 중복 코드가 제거되고 가독성이 올라갈 수 있을 것 같아요!
|
|
||
| if (typeof strValue !== 'string') return [key, undefined]; | ||
|
|
||
| switch (key) { |
There was a problem hiding this comment.
현재는 각각 다르게 normalize를 진행하기 위해서 case를 나누었는데 모든 case에 적용할 수 있는 normalize로직으로 수정한다면 해당 부분도 아래와 같이 표현할 수 있을 것 같네요!
return Object.fromEntries(
Object.entries(query)
.map(([key, value]) => {
const strValue = Array.isArray(value) ? value[0] : value;
if (typeof strValue !== 'string') return [key, undefined];
return [key, normalizeValue(key, strValue)];
})
.filter(([_, value]) => value !== undefined), // undefined 값 제거
);| const formatQueryParams = (query: ParsedUrlQuery): { [key: string]: string } => { | ||
| const normalizeCareer = (career: string | undefined): string | undefined => { | ||
| if (!career || career === '전체') return undefined; | ||
| return career === '인턴' ? '인턴 경험만 있어요' : career === '아직 없음' ? '아직 없어요' : career; |
There was a problem hiding this comment.
해당 부분을 상수화해서 아래와 같은 객체로 정의하여 CAREER_MAPPING['인턴']처럼 조회를 할 수 있게하면 좋을 것 같아요!
const CAREER_MAPPING: Record<string, string> = {
'인턴': '인턴 경험만 있어요',
'아직 없음': '아직 없어요',
};
const SECTION_MAPPING: Record<string, string> = {
'프론트엔드': '프론트',
};
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Summary by CodeRabbit
Walkthrough커피챗 메인 페이지의 필터 상태 관리가 기존의 로컬 상태에서 Next.js 라우터의 URL 쿼리 파라미터와 동기화되도록 리팩토링되었습니다. 필터 값은 쿼리에서 초기화되고, 변경 시 URL 쿼리 파라미터가 직접 수정됩니다. 필터 UI와 데이터 패칭 흐름이 이에 맞게 조정되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CoffeeChatCategory
participant NextRouter
participant useGetMembersCoffeeChat
User->>CoffeeChatCategory: 페이지 진입/필터 변경/검색
CoffeeChatCategory->>NextRouter: 쿼리 파라미터 읽기/수정
NextRouter-->>CoffeeChatCategory: 쿼리 파라미터 전달
CoffeeChatCategory->>useGetMembersCoffeeChat: 정규화된 쿼리 파라미터 전달
useGetMembersCoffeeChat-->>CoffeeChatCategory: 데이터 반환
CoffeeChatCategory-->>User: 필터 UI 및 카드 목록 렌더링
Assessment against linked issues
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/components/coffeechat/CoffeeChatCategory/index.tsx (1)
73-82: 값 변환(normalize) 로직 중복 – 상수 매핑으로 통합 권장
normalizeCareer,normalizeSection함수가 하드코딩된 조건문을 반복하고 있습니다.
이전 리뷰에서 제안된 것처럼 매핑 객체를 활용하면 가독성과 유지보수성이 향상됩니다.const CAREER_MAP = { '인턴': '인턴 경험만 있어요', '아직 없음': '아직 없어요' } as const; const SECTION_MAP = { '프론트엔드': '프론트' } as const; const normalize = (map: Record<string,string>, value?: string) => !value || value === '전체' ? undefined : map[value] ?? value;그리고 switch 대신 공통
normalize함수를 적용하면 중복이 크게 줄어듭니다.
이 부분은 과거에도 동일 지적이 있었으므로 참고 부탁드립니다.
| onSubmit={() => { | ||
| logSubmitEvent('searchCoffeeChat', { | ||
| search_content: clientSearch, | ||
| search_content: search, | ||
| }); | ||
| setSearch(clientSearch); | ||
| addQueryParamsToUrl({ search: clientSearch || undefined }); | ||
| }} | ||
| onReset={() => setClientSearch('')} | ||
| onReset={() => handleFilterChange('seasrch', '')} |
There was a problem hiding this comment.
onReset 키 오타로 모바일에서 검색 초기화 실패
'seasrch' → 'search' 오타 때문에 모바일 필터에서 검색어가 제대로 초기화되지 않습니다.
- onReset={() => handleFilterChange('seasrch', '')}
+ onReset={() => handleFilterChange('search', undefined)}📝 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.
| onSubmit={() => { | |
| logSubmitEvent('searchCoffeeChat', { | |
| search_content: clientSearch, | |
| search_content: search, | |
| }); | |
| setSearch(clientSearch); | |
| addQueryParamsToUrl({ search: clientSearch || undefined }); | |
| }} | |
| onReset={() => setClientSearch('')} | |
| onReset={() => handleFilterChange('seasrch', '')} | |
| onSubmit={() => { | |
| logSubmitEvent('searchCoffeeChat', { | |
| search_content: search, | |
| }); | |
| addQueryParamsToUrl({ search: clientSearch || undefined }); | |
| }} | |
| onReset={() => handleFilterChange('search', undefined)} |
🤖 Prompt for AI Agents
In src/components/coffeechat/CoffeeChatCategory/index.tsx around lines 242 to
248, there is a typo in the onReset handler where 'seasrch' is used instead of
'search'. Correct the key from 'seasrch' to 'search' in the handleFilterChange
call to ensure the search filter resets properly on mobile devices.
| search_content: search, | ||
| }); | ||
| setSearch(clientSearch); | ||
| addQueryParamsToUrl({ search: clientSearch || undefined }); | ||
| }} | ||
| onReset={() => setClientSearch('')} | ||
| onReset={() => handleFilterChange('search', '')} | ||
| /> |
There was a problem hiding this comment.
검색 초기화 시 search 쿼리 파라미터가 남습니다
handleFilterChange('search', '')로 빈 문자열을 전달하면 skipNull: true 옵션에도 불구하고 '' 값이 그대로 URL에 유지됩니다.
또한 로그에 실제 입력값이 아닌 기존 search 값을 사용하고 있어 분석 데이터가 부정확할 수 있습니다.
- logSubmitEvent('searchCoffeeChat', {
- search_content: search,
- });
- addQueryParamsToUrl({ search: clientSearch || undefined });
+ logSubmitEvent('searchCoffeeChat', {
+ search_content: clientSearch,
+ });
+ addQueryParamsToUrl({ search: clientSearch || undefined });
...
- onReset={() => handleFilterChange('search', '')}
+ onReset={() => handleFilterChange('search', undefined)}이렇게 수정하면
- 실제 사용자가 입력한 값을 로그에 남기고
- 검색어를 비울 때 쿼리 파라미터가 완전히 제거됩니다.
📝 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.
| search_content: search, | |
| }); | |
| setSearch(clientSearch); | |
| addQueryParamsToUrl({ search: clientSearch || undefined }); | |
| }} | |
| onReset={() => setClientSearch('')} | |
| onReset={() => handleFilterChange('search', '')} | |
| /> | |
| logSubmitEvent('searchCoffeeChat', { | |
| search_content: clientSearch, | |
| }); | |
| addQueryParamsToUrl({ search: clientSearch || undefined }); | |
| }} | |
| onReset={() => handleFilterChange('search', undefined)} | |
| /> |
🤖 Prompt for AI Agents
In src/components/coffeechat/CoffeeChatCategory/index.tsx around lines 229 to
234, the onReset handler calls handleFilterChange with an empty string, but this
leaves the 'search' query parameter in the URL and logs the old search value
instead of the actual input. To fix this, update the onReset function to pass
undefined or null instead of an empty string to handleFilterChange so that the
'search' parameter is removed from the URL, and ensure the logging uses the
current input value to accurately reflect user actions.
| const handleSelectSection = (selected: string) => { | ||
| addQueryParamsToUrl({ section: selected !== '전체' ? selected : undefined }); | ||
| }; | ||
|
|
||
| const handleSelectTopic = (selected: string) => { | ||
| addQueryParamsToUrl({ topicType: selected !== '전체' ? selected : undefined }); | ||
| }; | ||
|
|
||
| const handleSelectCareer = (selected: string) => { | ||
| addQueryParamsToUrl({ career: selected !== '전체' ? selected : undefined }); | ||
| }; | ||
|
|
||
| const handleSelectPart = (selected: string) => { | ||
| addQueryParamsToUrl({ part: selected !== '전체' ? selected : undefined }); | ||
| }; | ||
|
|
There was a problem hiding this comment.
위 4개의 함수 아래처럼 로직 작성한다면 반복되는 로직 제거하고 가독성을 높일 수 있을 것 같아요!
const handleSelect = (key: string, selected: string) => {
addQueryParamsToUrl({
[key]: selected !== '전체' ? selected : undefined,
});
};
| }; | ||
|
|
||
| const handleSelectCareer = (selected: string) => { | ||
| addQueryParamsToUrl({ career: selected !== '전체' ? selected : undefined }); |
There was a problem hiding this comment.
해당 로직이 다른 코드에서는 아래와 같이 사용돼서 같은 목적의 사용이라면 호출부가 통일되었으면 좋겠다는 생각을 했습니다.
addQueryParamsToUrl({ search: '' });
분명 qs의 querystring 메소드는 "" 값을 받으면 key값을 지우는게 아니라 search= 방식으로 남아있는것으로 알고있었는데 내부 로직에 lodash-es의 isEmpty가 nullable한 값들을 모두 걸러주면서 정상 작동하고 있더군요!
해당 로직을 일관성있게 처리하려면
-
isEmpty를 사용하지 않거나, (qs.stringify는 undefined 값을 가진 key는 자동으로 serialize 대상에서 제외하기때문에 ""와 같은 사용은 문제를 일으킴)
-
addQueryParamsToUrl의 파라미터 타입을 아래와 같이 변경하는 방법이 있을 것 같습니다.
다만 아래의 방법은 기존의 NextRouter["query"] 타입을 사용하는것보다 직관성이나 가독성이 떨어지기 때문에 트레이드 오프가 있을 것 같군요...
export type SafeQueryParamValue = string & { __nonEmpty?: never } | null | undefined;
export type SafeQueryParams = Record<string, SafeQueryParamValue>;
제가 코드 파악하면서 스윽 둘러본 것이니 이 리뷰는 간단한 코멘트만 남겨주시고 그냥 무시하셔도 좋습니다.!
PS) NextRouter의 query 타입 뭐 대단한게 있나 보았는데 간단하네여
type ParsedUrlQuery = { [key: string]: string | string[] | undefined };
interface NextRouter {
query: ParsedUrlQuery;
// ... 그 외 push, pathname 등
}
3 participants
🤫 쉿, 나한테만 말해줘요. 이슈넘버
🧐 어떤 것을 변경했어요~?
🤔 그렇다면, 어떻게 구현했어요~?
useQueryParams를 사용해서 쿼리스트링을 붙였습니다.❤️🔥 당신이 생각하는 PR포인트, 내겐 매력포인트.
📸 스크린샷, 없으면 이것 참,, 섭섭한데요?