Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughThis PR adds a Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
Comment |
|
🏷️ Labeler has automatically applied labels based on your PR title, branch name, or commit message. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
package.json (1)
16-16: Critical: Remove duplicateclsxentry.
clsxis declared in bothdependencies(line 16) anddevDependencies(line 36). This duplication will cause package manager conflicts and should be resolved.Apply this diff to remove the duplicate from
devDependencies:"tailwindcss": "4.1.14", "typescript": "^5", "tailwind-merge": "^3.3.1", "class-variance-authority": "^0.7.1", - "clsx": "^2.1.1"Also applies to: 36-36
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
package.json(1 hunks)src/shared/lib/index.ts(1 hunks)src/shared/lib/utils.ts(1 hunks)
🔇 Additional comments (2)
src/shared/lib/index.ts (1)
1-1: LGTM!The barrel export pattern is appropriate and provides a clean public API for the shared library utilities.
src/shared/lib/utils.ts (1)
71-73: LGTM: Thecnutility implementation is correct.The pattern
clsx(inputs)works correctly becauseclsxaccepts arrays. Theinputsrest parameter is already an array, and passing it directly toclsxproduces the expected behavior.
package.json
Outdated
| "tailwind-merge": "^3.3.1", | ||
| "class-variance-authority": "^0.7.1", |
There was a problem hiding this comment.
Move runtime utilities to dependencies.
tailwind-merge and clsx are used at runtime by the cn() utility in src/shared/lib/utils.ts. They should be in dependencies, not devDependencies, to ensure they're installed in production environments.
Apply this diff to move them to dependencies:
"dependencies": {
"axios": "^1.12.2",
"clsx": "^2.1.1",
"next": "15.5.4",
"react": "19.1.0",
"react-dom": "19.1.0",
"react-hook-form": "^7.64.0",
- "svgo": "^4.0.0"
+ "svgo": "^4.0.0",
+ "tailwind-merge": "^3.3.1"
},
"devDependencies": {
"@eslint/eslintrc": "^3",
"@tailwindcss/postcss": "^4",
"@types/node": "^20",
"@types/react": "^19",
"@types/react-dom": "^19",
"eslint": "^9",
"eslint-config-next": "15.5.4",
"svg-sprite-loader": "^6.0.11",
"tailwindcss": "4.1.14",
"typescript": "^5",
- "tailwind-merge": "^3.3.1",
- "class-variance-authority": "^0.7.1",
- "clsx": "^2.1.1"
+ "class-variance-authority": "^0.7.1"
}📝 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.
| "tailwind-merge": "^3.3.1", | |
| "class-variance-authority": "^0.7.1", | |
| "dependencies": { | |
| "axios": "^1.12.2", | |
| "clsx": "^2.1.1", | |
| "next": "15.5.4", | |
| "react": "19.1.0", | |
| "react-dom": "19.1.0", | |
| "react-hook-form": "^7.64.0", | |
| "svgo": "^4.0.0", | |
| "tailwind-merge": "^3.3.1" | |
| }, | |
| "devDependencies": { | |
| "@eslint/eslintrc": "^3", | |
| "@tailwindcss/postcss": "^4", | |
| "@types/node": "^20", | |
| "@types/react": "^19", | |
| "@types/react-dom": "^19", | |
| "eslint": "^9", | |
| "eslint-config-next": "15.5.4", | |
| "svg-sprite-loader": "^6.0.11", | |
| "tailwindcss": "4.1.14", | |
| "typescript": "^5", | |
| "class-variance-authority": "^0.7.1" | |
| } |
🤖 Prompt for AI Agents
In package.json around lines 34-35, tailwind-merge (and clsx, used by cn() in
src/shared/lib/utils.ts) are currently in devDependencies; move them to
dependencies by removing their entries from devDependencies and adding them
under the dependencies object with the same versions (e.g., "tailwind-merge":
"^3.3.1", "clsx": "<current-version>"). After editing package.json, run your
package manager to update lockfile (npm/yarn/pnpm) so production installs
include these packages.
package.json
Outdated
| "typescript": "^5" | ||
| "typescript": "^5", | ||
| "tailwind-merge": "^3.3.1", | ||
| "class-variance-authority": "^0.7.1", |
There was a problem hiding this comment.
❓ Verification inconclusive
Verify usage of class-variance-authority.
class-variance-authority is added to devDependencies but is not imported in any of the provided files. The PR description mentions CVA for component variants, but no usage is demonstrated yet.
Run the following script to check if CVA is used elsewhere in the codebase:
🏁 Script executed:
#!/bin/bash
# Search for class-variance-authority imports or cva usage
rg -n --type=ts --type=tsx -C2 "from ['\"]class-variance-authority['\"]|import.*cva"Length of output: 27
🏁 Script executed:
#!/bin/bash
# Search for class-variance-authority imports or cva usage in .ts and .tsx files
rg -n -C2 "from ['\"]class-variance-authority['\"]|import.*cva" -g "*.ts" -g "*.tsx"
rg -n -C2 "cva\(" -g "*.ts" -g "*.tsx"Length of output: 0
Remove or consume class-variance-authority: it’s listed in devDependencies but no imports or cva() calls exist—either add the CVA usage for your component variants or remove this dependency.
🤖 Prompt for AI Agents
In package.json around line 35, "class-variance-authority" is declared but not
used; either remove it from devDependencies or consume it: to remove, delete the
line and run npm/yarn to update lockfile; to consume, add a cva usage in the
relevant component(s) by importing cva from 'class-variance-authority', define
your variant styles with cva(...), and replace existing conditional class logic
with the cva helper (or move the package to dependencies if it’s needed at
runtime).
skyblue1232
left a comment
skyblue1232
left a comment
There was a problem hiding this comment.
사용법이 잘 나와있네요!! 참고해서 공컴 구현하겠습니다~
KongMezu
left a comment
KongMezu
left a comment
There was a problem hiding this comment.
cva는 이번에 처음 사용해보는데 좋은 공부가 될 거 같습니다. 세팅 해주셔서 감사합니다 🫡🫡
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/pages/index.tsx (1)
19-21: Clarify whether this duplicate content is intentional.The paragraph duplicates the text from lines 16-18 with different styling. If this is temporary for testing the
text-display-mddesign token, consider removing it before merging. If it's intentional, this would be a good opportunity to demonstrate the newcn()utility introduced in this PR.For example, if testing the utility:
- <p className='text-display-md'> + <p className={cn('text-display-md', 'text-center')}> Next.js(Page Router) + TS + Tailwind + Axios </p>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
src/pages/index.tsx(1 hunks)src/shared/lib/utils.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/shared/lib/utils.ts
…/cn-lib-setting
…/cn-lib-setting
…/cn-lib-setting
KongMezu
left a comment
KongMezu
left a comment
There was a problem hiding this comment.
세팅할 때 덕분에 많이 배워갑니다... 이렇게까지 해본적 사실 별로 없거든요
여담으로 별 다른 오류 없었으면 좋겠습니다..
3 participants
🔥 작업 내용
컴포넌트 분기(variant/size/state)는 CVA(Class Variance Authority) 기반으로 정리
🔗 이슈
PR Point (To Reviewer)
유틸 cn() 도입
className 조합을 더 읽기 쉽고 안전하게 만들기 위해 cn()(= clsx + 확장된 tailwind-merge)와 cva를 도입했어요. 기존 방식은 삼항 연산자와 조건부 클래스가 군데군데 흩어져 중복, 충돌이 잦고, 최종 적용 색/타이포가 무엇인지 코드만으로 파악하기 어려웠어요. cn()은 임의 클래스와 조건을 자유롭게 섞으면서도 우리 디자인 토큰(팔레트·타이포)을 인식해 같은 계열이 겹치면 마지막 것만 남기도록 자동 정리해 줍니다.
덕분에 스타일 우선순위를 개발자가 수동으로 관리할 필요가 줄고, 코드가 단순해져 변경에 용이해요.
사실 이유는 여러개 있지만 디자인 혹은 동작 분기 처리해서 코드짤때 유용해요
이런식으로 사용할 수 있어요
혹은 한곳에서 스타일이 많아 코드에 가독성이 떨어질때 다음과 같이 사용할 수 도 있어요
이것외에도 다양하게 사용 가능한데 한번 찾아보고 본인 스타일대로 유용하게 사용하시길바랍니다~
📸 피그마 스크린샷 or 기능 GIF
(작업 내역 스크린샷)
Summary by CodeRabbit