chore: standardize catalog deps and chip package#210
chore: standardize catalog deps and chip package#210Yeom-JinHo wants to merge 3 commits intomainfrom
Conversation
|
WalkthroughThis PR updates dependency version specifiers across 17 package.json files to use pnpm's Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/chip/tsup.config.ts`:
- Around line 6-7: The config currently spreads defaultConfig and replaces the
external array; instead merge arrays so shared externals aren't lost: read
defaultConfig.external (handle it possibly undefined), concat or spread it with
['react','@radix-ui/react-slot','clsx'], and deduplicate the resulting array
before assigning to external in the exported config (reference defaultConfig and
the external property in tsup.config.ts).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: bc92bb8b-d4e5-4340-82e5-87730c9b6a09
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!**/pnpm-lock.yaml
📒 Files selected for processing (19)
packages/accordion/package.jsonpackages/avatar/package.jsonpackages/badge/package.jsonpackages/button/package.jsonpackages/card/package.jsonpackages/checkbox/package.jsonpackages/chip/package.jsonpackages/chip/tsup.config.tspackages/divider/package.jsonpackages/flex/package.jsonpackages/grid/package.jsonpackages/input/package.jsonpackages/radio/package.jsonpackages/skeleton/package.jsonpackages/switch/package.jsonpackages/theme/package.jsonpackages/tooltip/package.jsonpackages/typography/package.jsonpnpm-workspace.yaml
| ...defaultConfig, | ||
| external: ['react', '@radix-ui/react-slot', 'clsx'], |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Root tsup config =="
sed -n '1,220p' tsup.config.ts
echo
echo "== Chip tsup config =="
sed -n '1,220p' packages/chip/tsup.config.ts
echo
echo "== external declarations =="
rg -n --type=ts -C2 '\bexternal\s*:' tsup.config.ts packages/chip/tsup.config.tsRepository: sipe-team/side
Length of output: 831
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== All tsup.config.ts files in packages/* =="
fd 'tsup.config.ts' packages/
echo
echo "== External declarations in all package configs =="
fd 'tsup.config.ts' packages/ -x rg -l 'external' {}
echo
echo "== Show external patterns in all package configs =="
fd 'tsup.config.ts' packages/ -x sh -c 'echo "=== {} ==="; rg -A3 -B1 "external" {} || echo "(no external property)"'Repository: sipe-team/side
Length of output: 875
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Chip package.json (dependencies section) =="
jq '.dependencies, .peerDependencies, .devDependencies' packages/chip/package.json
echo
echo "== Sample other package configs (button, avatar) =="
echo "--- button/tsup.config.ts ---"
cat packages/button/tsup.config.ts
echo
echo "--- avatar/tsup.config.ts ---"
cat packages/avatar/tsup.config.ts
echo
echo "== Verify if any package defines peerDependencies for react =="
fd 'package.json' packages/ -x jq -r 'select(.peerDependencies.react != null) | input_filename' 2>/dev/null | head -5Repository: sipe-team/side
Length of output: 1048
Merge external array instead of replacing it for consistent configuration handling.
The packages/chip/tsup.config.ts spreads defaultConfig then overwrites external. While currently working (the root config has no external property), this pattern will break maintainability if the root config evolves to define shared externals. The spread operator won't merge arrays—it will replace them entirely.
Suggested refactor
export default defineConfig({
...defaultConfig,
- external: ['react', '@radix-ui/react-slot', 'clsx'],
+ external: [
+ ...((defaultConfig as { external?: string[] }).external ?? []),
+ 'react',
+ '@radix-ui/react-slot',
+ 'clsx',
+ ],
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/chip/tsup.config.ts` around lines 6 - 7, The config currently
spreads defaultConfig and replaces the external array; instead merge arrays so
shared externals aren't lost: read defaultConfig.external (handle it possibly
undefined), concat or spread it with ['react','@radix-ui/react-slot','clsx'],
and deduplicate the resulting array before assigning to external in the exported
config (reference defaultConfig and the external property in tsup.config.ts).
Changes
pnpm-workspace.yaml의catalog항목을 확장해 공통 의존성 버전을 한 곳에서 관리하도록 정리했습니다.packages/*전반에서 직접 고정하던 의존성 버전을catalog:참조로 전환해 버전 관리 방식을 일관되게 맞췄습니다.packages/chip패키지명을@side/chip에서@sipe-team/chip으로 변경해 모노레포 네임스페이스 규칙에 맞췄습니다.packages/chip/package.json의 주요 의존성과 개발 의존성을 모노레포 표준에 맞게catalog:및catalog:react기반으로 정리했습니다.packages/chip/tsup.config.ts가 루트tsup.config.ts를 상속하도록 변경해vanilla-extract빌드 설정을 공통 설정과 동일하게 맞췄습니다.pnpm-lock.yaml을 갱신했습니다.Visuals
Checklist
Additional Discussion Points
pnpm testpnpm --filter @sipe-team/chip buildpnpm --filter @sipe-team/chip testpnpm --filter @sipe-team/chip typecheckpackages/chip빌드 시package.json의exports.types순서 관련 warning은 있었지만, 빌드 자체는 정상적으로 완료되었습니다.Summary by CodeRabbit