[♻️ Refactor/#59] Portal Root 공통 관리 구조 적용#60
Merged
Merged
Conversation
5 tasks
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🧪 테스트 결과
|
🚦 CI 검증 결과
|
WalkthroughPortal Root를 ChangesPortal Root 중앙화
Estimated code review effort: 2 (Simple) | ~15 minutes Sequence Diagram(s)sequenceDiagram
participant RootLayout
participant SurfacePortal
participant TooltipContent
participant Portal
participant DOM
RootLayout->>DOM: PORTAL_ROOT_ID div 렌더링
SurfacePortal->>Portal: container prop 전달
TooltipContent->>Portal: container prop 전달
Portal->>DOM: PORTAL_ROOT_ID 요소 조회 (container 없을 시)
DOM-->>Portal: 요소 존재 여부 반환
Portal->>DOM: 없으면 document.body로 fallback
Portal->>DOM: createPortal(children, container)
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/shared/ui/surface/Surface.types.ts (1)
57-61: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
container주석에 기본 동작(Portal Root) 설명 추가 권장.
Tooltip.types.ts의container주석은 "생략하면 앱 전역 Portal Root를 사용합니다.null을 전달하면 portal 렌더링을 하지 않습니다."로 갱신되었지만, 이 파일의container주석은 여전히 "portal을 렌더링할 대상 요소입니다."로만 남아 있습니다.SurfacePortal.tsx의 함수 JSDoc은 이미 기본값 동작을 설명하고 있어, 타입 파일만 문서가 뒤처진 상태입니다. 일관성을 위해 갱신을 권장합니다.📝 제안 diff
interface SurfacePortalProps { children: ReactNode; - /** portal을 렌더링할 대상 요소입니다. */ + /** + * portal을 렌더링할 대상 요소입니다. + * + * 생략하면 앱 전역 Portal Root를 사용하고, `null`이면 렌더링하지 않습니다. + */ container?: PortalContainer | null; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/shared/ui/surface/Surface.types.ts` around lines 57 - 61, The SurfacePortalProps container doc is outdated and should match the actual default behavior documented in SurfacePortal.tsx and Tooltip.types.ts. Update the container comment in SurfacePortalProps to explain that omitting it uses the app-wide Portal Root, and passing null disables portal rendering. Keep the change limited to the SurfacePortalProps type documentation so the API docs stay consistent.src/shared/ui/portal/Portal.tsx (1)
9-15: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win컨테이너 조회를 메모이즈하는 것을 고려해 보세요.
container가undefined인 경우 매 렌더마다getDefaultPortalContainer()가document.getElementById를 다시 호출합니다. Tooltip처럼 위치 갱신 등으로 자주 리렌더링되는 컴포넌트가 이Portal을 사용하는 경로(TooltipContent.tsx)에서는 불필요한 DOM 조회가 반복될 수 있습니다.containerprop 값이 렌더 간 안정적으로 유지되는 경우가 많으므로useMemo로 캐싱하면 이 다운스트림 리렌더링 비용을 줄일 수 있습니다.♻️ 제안 diff
+import { useMemo } from 'react'; + export function Portal({ children, container }: PortalProps) { - const portalContainer = container === undefined ? getDefaultPortalContainer() : container; + const portalContainer = useMemo( + () => (container === undefined ? getDefaultPortalContainer() : container), + [container] + ); if (!portalContainer) { return null; } return createPortal(children, portalContainer); }Also applies to: 30-38
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/shared/ui/portal/Portal.tsx` around lines 9 - 15, `Portal`에서 `container`가 없을 때마다 `getDefaultPortalContainer()`가 `document.getElementById`를 다시 호출하고 있으니, 이 조회 결과를 메모이즈하도록 수정하세요. `Portal` 컴포넌트와 `getDefaultPortalContainer` 주변에서 `container` prop이 안정적일 때는 `useMemo`(또는 동등한 캐싱)로 기본 포털 컨테이너를 한 번만 계산하게 바꾸고, `TooltipContent.tsx`처럼 자주 리렌더링되는 경로에서 불필요한 DOM 조회가 반복되지 않도록 하세요.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/shared/ui/portal/Portal.tsx`:
- Around line 9-15: `Portal`에서 `container`가 없을 때마다
`getDefaultPortalContainer()`가 `document.getElementById`를 다시 호출하고 있으니, 이 조회 결과를
메모이즈하도록 수정하세요. `Portal` 컴포넌트와 `getDefaultPortalContainer` 주변에서 `container` prop이
안정적일 때는 `useMemo`(또는 동등한 캐싱)로 기본 포털 컨테이너를 한 번만 계산하게 바꾸고, `TooltipContent.tsx`처럼
자주 리렌더링되는 경로에서 불필요한 DOM 조회가 반복되지 않도록 하세요.
In `@src/shared/ui/surface/Surface.types.ts`:
- Around line 57-61: The SurfacePortalProps container doc is outdated and should
match the actual default behavior documented in SurfacePortal.tsx and
Tooltip.types.ts. Update the container comment in SurfacePortalProps to explain
that omitting it uses the app-wide Portal Root, and passing null disables portal
rendering. Keep the change limited to the SurfacePortalProps type documentation
so the API docs stay consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 43b437f0-7b06-4898-a49e-7d1376157322
📒 Files selected for processing (10)
src/app/layout.tsxsrc/shared/constants/portal.tssrc/shared/ui/portal/Portal.tsxsrc/shared/ui/portal/Portal.types.tssrc/shared/ui/portal/index.tssrc/shared/ui/surface/Surface.types.tssrc/shared/ui/surface/SurfacePortal.tsxsrc/shared/ui/tooltip/Tooltip.tsxsrc/shared/ui/tooltip/Tooltip.types.tssrc/shared/ui/tooltip/TooltipContent.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#️⃣연관된 이슈
체크 사항
📝작업 내용
Portal Root 공통 관리 구조 적용
layout.tsx에 앱 전역 Portal Root를 추가했습니다.src/shared/constants/portal.ts에서 상수로 관리하도록 분리했습니다.src/shared/ui/portal에 공통Portal컴포넌트를 추가해createPortal호출과 기본 container 선택 로직을 중앙화했습니다.Portal 기반 컴포넌트 리팩터링
Surface.Portal이 직접createPortal을 호출하지 않고 공통Portal을 사용하도록 수정했습니다.Tooltip.Content가 공통Portal을 사용하도록 수정했습니다.스크린샷 (선택)
추가한 라이브러리 (선택)
💬리뷰 요구사항(선택)
Summary by CodeRabbit
새 기능
개선 사항