Conversation
- 탭 컴포넌트 오버플로우 스타일 제거
- noNonNullAssertion 규칙을 off로 설정 - 비즈니스 로직 메서드로 존재 여부를 검증한 후 non-null assertion 사용하는 경우로 인해 off로 설정
- Query Hook 패턴 적용하여 컴포넌트에서 데이터 페칭 로직 분리 - Suspense와 ErrorBoundary 적용 - 영업시간 문자열 파싱 지원 (API 응답 형식 \"11:30:00\" 처리) - 시간 파싱 유틸 함수 추가 (parseHour, parseMinute, parseSecond) - 영업시간 비고를 별도 줄로 분리하여 모바일 UI 개선 - 불필요한 acceptsMealTicket 비즈니스 로직 제거 - 비즈니스 로직 메서드 일관성 적용 (canTakeout, hasPhone, hasMealTicketPrice)
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Jira Issue: NUGUDI-202
|
Summary of ChangesHello @hijjoy, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 PR은 구내식당 상세 정보 페이지의 데이터 연동 및 사용자 경험 개선에 중점을 둡니다. 목데이터 사용을 줄이고 실제 데이터 흐름을 구축하며, 영업시간 데이터 처리의 정확성을 높였습니다. 또한, UI 컴포넌트의 렌더링 로직을 최적화하고, 데이터 로딩 및 에러 처리를 위한 React의 Suspense와 ErrorBoundary 패턴을 도입하여 애플리케이션의 안정성과 반응성을 향상시켰습니다. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
이 PR은 구내식당 상세 정보 페이지를 목 데이터에서 실제 API와 연결하는 중요한 작업을 성공적으로 수행했습니다. useSuspenseQuery를 사용한 커스텀 훅을 도입하여 데이터 로딩 상태를 선언적으로 처리한 점이 인상적입니다. 또한, 코드 구조가 클린 아키텍처 원칙을 잘 따르고 있으며, non-null assertion(!) 사용에 대한 이유를 PR 설명에 상세히 기술해주셔서 리뷰에 큰 도움이 되었습니다. 전반적으로 훌륭한 변경 사항입니다.
| {cafeteria.hasNote && ( | ||
| <InfoRow icon="📝" label={cafeteria.businessHours!.note!} /> | ||
| )} | ||
| {cafeteria.hasMealTicketPrice && ( | ||
| <InfoRow | ||
| icon="💰" | ||
| label={`가격 ${formatPriceWithCurrency(cafeteria.mealTicketPrice!)}`} | ||
| /> | ||
| )} | ||
| {cafeteria.canTakeout && <InfoRow icon="📦" label="포장 가능" />} | ||
| {cafeteria.hasPhone && <InfoRow icon="📞" label={cafeteria.phone!} />} |
There was a problem hiding this comment.
PR 설명에서 언급해주신 것처럼, has... 프로퍼티로 존재 여부를 확인한 후 non-null assertion(!)을 사용하고 계십니다. biome.json에서 관련 규칙을 비활성화한 것은 팀의 결정으로 보이지만, 이 패턴은 has... 로직과 실제 프로퍼티의 상태가 일치하지 않을 경우 런타임 에러를 발생시킬 수 있는 위험을 내포하고 있습니다.
PR 설명에서 직접 제안해주신 타입 가드(type guard)를 사용하는 것이 훨씬 안전한 방법입니다. 타입 가드를 사용하면 컴파일 타임에 타입 안전성을 보장받을 수 있으며, ! 연산자를 사용하지 않아도 됩니다.
예를 들어, 다음과 같은 타입 가드를 정의하여 사용할 수 있습니다.
// 타입 가드 정의
function hasNote(cafeteria: CafeteriaDetailItem): cafeteria is CafeteriaDetailItem & { businessHours: BusinessHoursItem & { note: string } } {
return cafeteria.hasNote;
}
function hasPhone(cafeteria: CafeteriaDetailItem): cafeteria is CafeteriaDetailItem & { phone: string } {
return cafeteria.hasPhone;
} {hasNote(cafeteria) && (
<InfoRow icon="📝" label={cafeteria.businessHours.note} />
)}
{cafeteria.hasMealTicketPrice && (
<InfoRow
icon="💰"
label={`가격 ${formatPriceWithCurrency(
cafeteria.mealTicketPrice!,
)}`}
/>
)}
{cafeteria.canTakeout && <InfoRow icon="📦" label="포장 가능" />}
{hasPhone(cafeteria) && <InfoRow icon="📞" label={cafeteria.phone} />}
There was a problem hiding this comment.
@dydals3440 해당 부분에 대해 위에 설명을 추가적으로 적어두었습니다 ! 의견주시면 감사하겠습니다 🐿️
apps/web/src/domains/cafeteria/presentation/shared/ui/views/cafeteria-detail-view/index.tsx
Outdated
Show resolved
Hide resolved
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 개요시간 파싱 유틸리티 추가, 카페테리아 데이터 소스를 목(mock)에서 실제 HTTP 기반으로 전환, 도메인 엔티티 메서드 업데이트(hasNote 추가, acceptsMealTicket 제거), UI 컴포넌트 및 어댑터 리팩토링, 카페테리아 상세 조회 쿼리 훅 생성, 탭 컴포넌트 스타일 조정. 변경 사항
시퀀스 다이어그램sequenceDiagram
participant UI as Cafeteria Detail View
participant Hook as useGetCafeteriaDetail
participant UseCase as GetCafeteriaById
participant Adapter as CafeteriaAdapter
UI->>Hook: useGetCafeteriaDetail(cafeteriaId)
activate Hook
Hook->>UseCase: execute(cafeteriaId)
activate UseCase
UseCase-->>Hook: Cafeteria (domain entity)
deactivate UseCase
Hook->>Adapter: toUiDetailItem(cafeteria)
activate Adapter
Adapter-->>Hook: CafeteriaDetailItem
deactivate Adapter
Hook-->>UI: Query with data
deactivate Hook
UI->>UI: Render with Suspense/ErrorBoundary
sequenceDiagram
participant Old as 이전: Mock 데이터 소스
participant New as 새로운: HTTP 기반 데이터 소스
participant DI as DI Container
participant HTTP as HttpClient (인증)
rect rgb(200, 220, 255)
Note over Old: 이전 흐름
DI->>Old: CafeteriaRemoteDataSourceMockImpl 생성
Old-->>DI: Mock 데이터 반환
end
rect rgb(200, 255, 220)
Note over New: 새로운 흐름
DI->>DI: SessionManager, TokenProvider 생성
DI->>HTTP: AuthenticatedHttpClient 생성
DI->>New: CafeteriaRemoteDataSourceImpl 생성 (HttpClient 주입)
New->>HTTP: API 호출
HTTP-->>New: 인증된 응답
New-->>DI: 실제 데이터 반환
end
예상 코드 리뷰 노력🎯 3 (중간 복잡도) | ⏱️ ~25분 추가 검토 필요 영역:
관련 PR 목록
추천 레이블
추천 리뷰어
시
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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 |
|
This pull request (commit
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
apps/web/src/domains/cafeteria/data/mapper/cafeteria.mapper.ts (1)
61-61: 타입 안정성을 위해any타입을 구체적인 타입으로 변경하는 것을 권장합니다.이전 리뷰에서도 지적되었듯이,
time파라미터가any타입으로 지정되어 있어 타입 검사를 우회하고 있습니다.string | null | undefined와 같이 더 구체적인 타입을 사용하면 타입 안정성이 향상되고 런타임 오류를 방지할 수 있습니다.다음과 같이 수정할 수 있습니다:
- const convertLocalTime = (time: any) => { + const convertLocalTime = (time: string | null | undefined) => {apps/web/src/core/shared/util/date/parse-time.ts (1)
54-59: 일관성을 위해parseSecond도 유효하지 않은 값에 대해null을 반환하는 것을 고려해보세요.이전 리뷰에서도 지적되었듯이,
parseHour와parseMinute는 유효하지 않은 값에 대해null을 반환하는 반면,parseSecond는0을 반환합니다. 이러한 불일치는 파싱 실패를 감지하기 어렵게 만들 수 있습니다. 일관된 인터페이스를 제공하기 위해null을 반환하도록 변경하는 것을 고려해보세요.단, 현재 설계가 의도적인 것이라면(초가 선택적이고 기본값이 0), 호출하는 쪽에서
?? 0을 사용하여 기본값을 명시적으로 설정하는 것이 함수의 재사용성과 예측 가능성을 높일 수 있습니다.apps/web/src/domains/cafeteria/presentation/shared/ui/components/cafeteria-info-tab/index.tsx (1)
43-55: hasX 플래그 + non-null assertion 패턴은 도메인 인바리언트에 강하게 의존합니다.
hasNote/hasMealTicketPrice/canTakeout/hasPhone플래그로 존재 여부를 판단한 뒤businessHours!.note!,mealTicketPrice!,phone!을 사용하는 구조는, 도메인/어댑터가 항상 해당 인바리언트를 지켜준다는 전제하에서는 깔끔하지만 한 번만 어긋나도 런타임 에러가 날 수 있습니다. 팀에서 이미 논의하신 것처럼, 여유가 생기면 공용 타입 가드(예:hasNote(cafeteria),hasPhone(cafeteria))로 이 패턴을 감싸 두면 UI 쪽 타입 안전성이 조금 더 좋아질 것 같습니다.apps/web/src/domains/cafeteria/di/cafeteria-client-container.ts (1)
140-188: UseCase를 호출 시마다 새로 생성하는 전략은 현재로서는 괜찮아 보이지만, 호출 빈도가 늘면 캐싱도 고려해 볼 수 있습니다.각
get...UseCase가 매번 새 인스턴스를 반환하는 구조는 테스트/확장성 면에서는 단순하지만, 이전 리뷰에서 언급된 것처럼 호출이 잦은 경로에서는 불필요한 객체 생성이 누적될 수 있습니다. 현재 트래픽/복잡도 수준에서는 큰 문제는 아니겠지만, 추후 성능 튜닝이 필요해지면 UseCase도 Repository처럼 한 번 생성해 캐싱하는 방향을 옵션으로 열어두시면 좋겠습니다.
🧹 Nitpick comments (4)
apps/web/src/domains/cafeteria/presentation/shared/adapters/cafeteria.adapter.ts (1)
366-371: JSDoc 예제 업데이트 필요Line 402-403에서 구현이 변경되어 note를 별도로 표시하도록 변경되었으나, JSDoc의
@example(Line 366-367)에는 여전히 note가 포함된 예제가 명시되어 있습니다. 실제 구현과 일치하도록 예제를 업데이트해주세요.다음과 같이 수정하는 것을 권장합니다:
* @example - * // With note - * "점심: 11:30 ~ 14:00 / 저녁: 17:00 ~ 20:00 (주말 휴무)" + * // With lunch and dinner (note is displayed separately) + * "점심: 11:30 ~ 14:00 / 저녁: 17:00 ~ 20:00"apps/web/src/domains/cafeteria/presentation/client/hooks/queries/get-cafeteria-detail.query.ts (1)
14-26: React Query 훅 반환 타입을 명시하면 가독성이 조금 더 좋아질 수 있습니다.DI/어댑터 구조와 staleTime/gcTime 설정 모두 자연스럽습니다. 다만
useSuspenseQuery의 제네릭을 명시해 두면 IDE에서 반환 타입이 더 명확해지고, React Query 버전에 따라 추론 방식이 달라져도 안전합니다. (이미queryFn기반으로 잘 추론되고 있다면 그대로 두셔도 무방합니다.)예시:
import type { CafeteriaDetailItem } from "@cafeteria/presentation/shared/types"; export function useGetCafeteriaDetail(cafeteriaId: string) { const container = getCafeteriaClientContainer(); const getCafeteriaByIdUseCase = container.getGetCafeteriaById(); return useSuspenseQuery<CafeteriaDetailItem>({ queryKey: cafeteriaKeys.detail(cafeteriaId), queryFn: async () => { const entity = await getCafeteriaByIdUseCase.execute(cafeteriaId); return CafeteriaAdapter.toUiDetailItem(entity); }, staleTime: 30 * 60 * 1000, gcTime: 60 * 60 * 1000, }); }React Query 현재 사용 중인 버전에서
useSuspenseQuery의 제네릭 추론 동작이 어떻게 정의되어 있는지 한 번만 공식 문서로 확인해 보시면 좋겠습니다.apps/web/src/domains/cafeteria/domain/entities/business-hours.entity.ts (1)
47-51: hasNote 구현에서 공백 문자열 처리 여부를 한 번만 확인해 주세요.현재는
this._note !== null만 검사하고 있어서, 만약 백엔드에서" "같은 공백 문자열을 내려줄 가능성이 있다면hasNote는true지만 UI에는 내용 없는 줄이 표시될 수 있습니다. 그런 케이스를 방지하려면 아래처럼 trim 기준으로 체크하는 쪽이 도메인 인바리언트를 조금 더 강하게 보장해 줄 것 같습니다.hasNote(): boolean { return typeof this._note === "string" && this._note.trim().length > 0; }(실제 API가
null또는 의미 있는 문자열만 내려준다면 지금 구현도 충분히 괜찮습니다.)Also applies to: 173-175
apps/web/src/domains/cafeteria/di/cafeteria-client-container.ts (1)
7-11: HTTP/세션 관련 의존성의 lazy 초기화 구성이 잘 되어 있습니다.
ClientSessionManager → ClientTokenProvider → FetchHttpClient → AuthenticatedHttpClient → CafeteriaRemoteDataSourceImpl로 이어지는 계층이 한 컨테이너 인스턴스 내에서 한 번만 생성되도록 잘 묶여 있어서, 클라이언트 환경에서 과도한 객체 생성을 막는 구조로 보입니다. 한 가지 정도만 더 보자면,NEXT_PUBLIC_API_URL이 설정되지 않았을 때baseUrl이 빈 문자열이 되는데, 이 값이 필수 설정이라면 초기화 시점에 에러를 던지거나 의미 있는 기본값을 두는 것도 운영 상 안전망이 될 수 있습니다.Also applies to: 37-43, 61-67, 71-105, 106-113
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
apps/web/src/core/shared/util/date/index.ts(1 hunks)apps/web/src/core/shared/util/date/parse-time.ts(1 hunks)apps/web/src/domains/cafeteria/data/mapper/cafeteria.mapper.ts(2 hunks)apps/web/src/domains/cafeteria/data/remote/api/cafeteria-remote-data-source-impl.ts(3 hunks)apps/web/src/domains/cafeteria/di/cafeteria-client-container.ts(4 hunks)apps/web/src/domains/cafeteria/di/cafeteria-server-container.ts(2 hunks)apps/web/src/domains/cafeteria/domain/entities/business-hours.entity.ts(2 hunks)apps/web/src/domains/cafeteria/domain/entities/cafeteria.entity.ts(1 hunks)apps/web/src/domains/cafeteria/presentation/client/hooks/queries/get-cafeteria-detail.query.ts(1 hunks)apps/web/src/domains/cafeteria/presentation/shared/adapters/cafeteria.adapter.ts(2 hunks)apps/web/src/domains/cafeteria/presentation/shared/types/cafeteria.ts(1 hunks)apps/web/src/domains/cafeteria/presentation/shared/ui/components/cafeteria-info-card/index.tsx(2 hunks)apps/web/src/domains/cafeteria/presentation/shared/ui/components/cafeteria-info-tab/index.css.ts(1 hunks)apps/web/src/domains/cafeteria/presentation/shared/ui/components/cafeteria-info-tab/index.tsx(3 hunks)apps/web/src/domains/cafeteria/presentation/shared/ui/sections/cafeteria-hero-section/index.tsx(2 hunks)apps/web/src/domains/cafeteria/presentation/shared/ui/views/cafeteria-detail-view/index.tsx(2 hunks)biome.json(1 hunks)packages/react/components/tab/src/style.css.ts(0 hunks)
💤 Files with no reviewable changes (1)
- packages/react/components/tab/src/style.css.ts
🔇 Additional comments (18)
biome.json (1)
34-34: 린터 규칙을 전역적으로 비활성화하기보다는 타입 안전성 문제를 근본적으로 해결하세요.비-널 단언(non-null assertion)을 피하려는 의도는 좋지만, 린터 규칙을 전역적으로 끄는 것은 코드베이스 전체의 타입 안전성을 훼손합니다. PR 목표에서 설명했듯이 "보일러플레이트 코드 추가"를 피하기 위해 이 결정이 내려졌다면, 더 나은 대안을 고려해야 합니다:
- 타입 가드 함수 (Adapter에서): 엔티티 메서드와 함께 타입 가드를 작성하여 TypeScript가 자동으로 타입을 좁힐 수 있도록 설계
satisfies키워드: TypeScript 4.9+에서 타입 검증을 지원하며 더 간결함- 로컬 비활성화: 정말 필요한 특정 위치에서만
// @ts-expect-error또는// biome-ignore주석 사용현재 접근 방식은 타입 안전성 검사를 완전히 우회하므로, 향후 null 참조 버그가 런타임에서만 발견될 수 있습니다.
더 적절한 타입 좁히기 패턴을 적용한 대체 방안을 제시해 드릴 수 있습니다. 타입 가드 도입으로 인한 실제 보일러플레이트 코드량이 얼마나 되는지 함께 검토해보시겠어요?
apps/web/src/core/shared/util/date/index.ts (1)
13-13: 새로운 시간 파싱 유틸리티 export가 적절합니다.세 개의 시간 파싱 함수들을 date 유틸리티 모듈의 공개 API에 추가한 것이 적절하며, 다른 모듈에서 사용할 수 있도록 올바르게 구성되었습니다.
apps/web/src/domains/cafeteria/data/mapper/cafeteria.mapper.ts (2)
6-6: 시간 파싱 유틸리티 import가 적절합니다.새로운 시간 파싱 함수들을 올바르게 import하여 사용하고 있습니다.
64-76: 문자열 기반 시간 파싱 로직이 잘 구현되었습니다."HH:MM:SS" 및 "HH:MM" 형식의 문자열을 적절하게 파싱하고 있으며, 유효성 검증 로직도 올바르게 구현되어 있습니다.
parseHour,parseMinute,parseSecond유틸리티 함수를 활용하여 각 구성 요소를 검증하고, 유효하지 않은 경우 null을 반환하는 것이 적절합니다.apps/web/src/core/shared/util/date/parse-time.ts (2)
19-23:parseHour함수 구현이 적절합니다.0-23 범위의 시간 값을 올바르게 검증하고 있으며, 유효하지 않은 입력에 대해 null을 반환하는 것이 적절합니다.
36-40:parseMinute함수 구현이 적절합니다.0-59 범위의 분 값을 올바르게 검증하고 있으며, 유효하지 않은 입력에 대해 null을 반환하는 것이 적절합니다.
apps/web/src/domains/cafeteria/presentation/shared/ui/components/cafeteria-info-tab/index.css.ts (1)
21-23: 변경사항 확인 완료한글 레이블에 대해
wordBreak: "keep-all"을 적용하여 어색한 줄바꿈을 방지한 것은 적절한 처리입니다.apps/web/src/domains/cafeteria/presentation/shared/types/cafeteria.ts (1)
67-67: 타입 정의 확인 완료
hasNote필드 추가가 도메인 엔티티의 변경사항(CafeteriaEntity.hasNote())과 일관성 있게 반영되었습니다.apps/web/src/domains/cafeteria/presentation/shared/adapters/cafeteria.adapter.ts (1)
176-176: 변경사항 확인 완료
hasNote필드가 엔티티의hasNote()메서드를 통해 올바르게 매핑되었습니다.apps/web/src/domains/cafeteria/presentation/shared/ui/views/cafeteria-detail-view/index.tsx (1)
19-23: 에러 처리 및 로딩 상태 관리 개선 확인
ErrorBoundary와Suspense를 사용하여 hero 섹션의 에러 및 로딩 상태를 적절하게 처리했습니다.apps/web/src/domains/cafeteria/domain/entities/cafeteria.entity.ts (1)
134-139: 메서드 추가 확인 완료
hasNote()메서드가 기존 boolean 체크 메서드들(hasBusinessHours,hasPhone등)과 동일한 패턴으로 구현되었으며, null-safety 처리도 적절합니다.apps/web/src/domains/cafeteria/data/remote/api/cafeteria-remote-data-source-impl.ts (1)
125-127: 임시 Mock 데이터 사용 확인PR 목표에 명시된 대로 메뉴 타임라인 조회는 의도적으로 Mock 데이터를 사용하고 있습니다. TODO 주석이 명확하게 작성되어 향후 실제 API로 교체할 계획이 문서화되어 있습니다.
apps/web/src/domains/cafeteria/di/cafeteria-server-container.ts (1)
80-81: 실제 데이터 소스로 마이그레이션 확인Mock 데이터 소스에서 실제 HTTP 클라이언트 기반의 데이터 소스로 전환되었습니다. 인증된 HTTP 클라이언트 설정이 적절하게 구성되어 있으며, DI 패턴이 올바르게 적용되었습니다.
apps/web/src/domains/cafeteria/presentation/shared/ui/sections/cafeteria-hero-section/index.tsx (2)
3-3: 데이터 페칭 로직 개선 확인
useGetCafeteriaDetail훅을 사용한 데이터 페칭 방식으로 리팩토링되어 코드가 더욱 선언적이고 유지보수하기 쉬워졌습니다. 상위 뷰에 추가된Suspense경계와도 잘 연동됩니다.Also applies to: 16-16
26-30: 타입 안정성 개선 확인
HeroImageProps인터페이스를 명시적으로 정의하여 컴포넌트의 타입 안정성과 가독성이 향상되었습니다.apps/web/src/domains/cafeteria/presentation/shared/ui/components/cafeteria-info-card/index.tsx (1)
13-26: CafeteriaDetailItem으로의 전환과 canTakeout 사용이 도메인 모델과 잘 맞습니다.상세 화면에서 사용하는 타입을
CafeteriaDetailItem으로 통일한 점과, 포장 여부를canTakeout플래그로 분리해 표현한 구조가 명확하고 유지보수에도 좋아 보입니다.apps/web/src/domains/cafeteria/presentation/shared/ui/components/cafeteria-info-tab/index.tsx (2)
65-77: 위치 정보 처리 로직이 hasLocation 플래그와 잘 정렬되어 있습니다.
hasLocation으로 좌표 존재 여부를 분기하고, 있을 때만(latitude, longitude)를 렌더링하며 없을 때는 명시적으로 "위치 정보 없음"을 보여주는 UX가 명확하고, 잠재적인 null 접근 문제도 없어 보입니다.
123-124: InfoRow의 텍스트 스타일 분리가 적절합니다.
Body에styles.infoLabel클래스를 분리해서 적용한 덕분에 아이콘과 라벨 스타일을 독립적으로 조정하기 쉬워졌고, 재사용 관점에서도 좋은 방향으로 보입니다.
…nse 통합 - CafeteriaTabSection에서 useGetCafeteriaDetail Query Hook 사용 - 같은 쿼리를 사용하는 HeroSection과 TabSection을 단일 Suspense로 감싸 효율성 개선 - Skeleton과 Error 컴포넌트를 별도 파일로 분리 (skeleton.tsx, error.tsx)
|
This pull request (commit
|
There was a problem hiding this comment.
70c1258 : 해당 부분을 처리하면서 비즈니스 로직으로 생성한 메서드(hasPhone 등)를 사용하여 UI 로직에 보여주려고 했으나
hasPhone 메서드로 검증해도 TypeScript가 phone의 non-null을 추론하지 못해 ! 연산자를 사용하신 부분에 대한 의견입니다.
현재 접근 방식도 실용적이지만, 조금 더 나아가면 DDD에서는 Value Object 패턴도 많이 사용해서 한번 고려는 해볼만 할 것 같습니다!
// domain/value-objects/Phone.ts
class Phone {
private constructor(private readonly value: string) {}
static create(phone: string | null): Phone | null {
if (phone === null || phone.trim().length === 0) {
return null;
}
return new Phone(phone.trim());
}
getValue(): string {
return this.value;
}
}
// Entity
class CafeteriaDetailEntity {
constructor(private readonly phone: Phone | null) {}
hasPhone(): boolean {
return this.phone !== null;
}
}
// Adapter
function toCafeteriaDetailItem(entity: CafeteriaDetailEntity) {
return {
phone: entity.phone?.getValue() ?? null,
hasPhone: entity.hasPhone(),
};
}
// Component - 타입 안전성 자동 확보
{cafeteria.hasPhone && cafeteria.phone && (
<InfoRow icon="📞" label={cafeteria.phone} />
)}현재 단계에서는 비즈니스 로직이 이미 검증을 한 상태이니 ! 연산자 방식을 통해 Biome 에러를 임시 해결하는 것도 충분하다고 생각하기는하지만 향후 확장성을 고려하면 VO 패턴도 검토해보면 좋을 것 같아요!
{cafeteria.hasPhone && cafeteria.phone && (
<InfoRow icon="📞" label={cafeteria.phone} />
)}
type CafeteriaDetailItem =
| { hasPhone: true; phone: string; ... }
| { hasPhone: false; phone: null; ... };해당 방법을 사용하면 타입 자체가 정말 너무 길어지고 모든 필드에서 이렇게 작성하는 것은 올바르지 않다고 느꼈습니다.
{cafeteria.phone && (
<InfoRow icon="📞" label={cafeteria.phone} />
)}일단은 현재방식으로 진행 후에 추후 변경을 하려고 하는데 어떻게 생각하시나용 🐲 @dydals3440 |
2 participants
🌀 Issue Number
😵 As-Is
🥳 To-Be
hasPhone 부분으로 cafeteria.phone이 실제로 존재하는 것을 확인해도 TypeScript는 이 메서드가 true를 반환하는 경우,
해당객체(cafeteria)의 특정 속성(phone)이 자동으로 null 또는 undefined가 아니라는 사실까지는 추론하지 못하는 문제가 생겨 !
연산자를 사용하였습니다. 이 과정에서 biome 오류가 있어 c67a8a0 을 적용해 두었습니다.
단, 이 부분에 대해서 Adapter에 타입 가드를 추가하는 방법도 생각해 보았는데 오히려 보일러플레이트가 되는 것이 아닌가 해서 앞선
방법으로 적용을 해둔 상태입니다. (hasValidPhone나 hasPhone이나 사실 역할이 동일하기에 엔티티 메소드에서 한번 Adapter에서 한번 두번 정의하는 것이 보일러플레이트라 느낌)
또는 엔티티 메서드(hasPhone)를 사용하지 않고 직접 속성 체크를 하는 방법도 있지만,
이는 Entity의 비즈니스 로직(공백 문자열 검증 등)을 우회하게 되어 Clean Architecture의 관심사 분리 원칙에 어긋나므로
지양했습니다. hasPhone 메서드는 단순히 null 체크뿐만 아니라 trim().length > 0 같은 추가 검증 로직을 포함하고 있어, 비즈니스
규칙을 일관되게 적용하기 위해서는 Entity의 메서드를 사용하는 것이 올바르다고 판단했습니다. 하지만 해당 부분에 대해 다른 의견이 있으시면 리뷰 남겨주세요 ! 🙆🏻♀️
overflowX: "hidden"를 추가했던 이력이 있었습니다. 하지만 해당 변경사항으로 인해 가로스크롤이 생겼던 문제는 아니였고 실제로overflowX: "hidden"해당 코드를 삭제하면서 3번 문제를 해결하고, 가로스크롤도 생기지 않았습니다.✅ Check List
📷 Screenshot (Optional)
👾 Additional Description (Optional)
Summary by CodeRabbit
릴리스 노트
새 기능
버그 수정
사용자 경험