refactor(card): migrate module css to vanilla extract#148
refactor(card): migrate module css to vanilla extract#148froggy1014 merged 4 commits intorelease/v1from
Conversation
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
packages/card/src/Card.tsx
Outdated
| export type CardRatio = keyof typeof styles.CardRatio; | ||
| export type CardVariant = keyof typeof styles.CardVariant; |
There was a problem hiding this comment.
타입도 스타일에서 내보내거나 반대로 상수를 이 파일에서 정의하면 어떨까? 동일한 개념이 서로 다른 파일에서 관리되면 응집도가 떨어지게 되는 것 같아~
There was a problem hiding this comment.
이런 multi-variants 같은 경우 recipe API를 사용해서 선언해도 괜찮을 것 같아서.. 이번 커밋에 적용해봤는데 어떻게 생각하세요? 가독성, 응집도면으로나 더 나은것 같구 추후 theme을 위해서 토큰 적용도 쉬울 것 같아요.
Reference
There was a problem hiding this comment.
recipe API는 좋은 것 같아~ 다만 기존에 외부로 export 해주던 CardRatio와 CardVariant가 사라지게 되면, 사용자에게는 Breaking Changes가 발생하게 될 것 같아 ㅎㅎ BC 없이 적용할 방법을 더 고려해보면 어떨까??
There was a problem hiding this comment.
호고곡 그건 생각못해봤네요. 상수를 다시 선언해서 말씀해주신대로 interface로 바꿔보았어욤
packages/card/src/Card.tsx
Outdated
| export type CardProps = CardVariants & | ||
| ComponentProps<'div'> & { | ||
| asChild?: boolean; | ||
| }; |
There was a problem hiding this comment.
| export type CardProps = CardVariants & | |
| ComponentProps<'div'> & { | |
| asChild?: boolean; | |
| }; | |
| export interface CardProps extends ComponentProps<'div'>, CardVariants { | |
| asChild?: boolean; | |
| }; |
우리 프로젝트에서 Props는 타입 보다는 인터페이스로 선언하는게 주된 흐름이라고 느끼는데, 특별한 이유가 있는게 아니라면 동일하게 유지해주는게 어떨까??
Changes
Visuals
Checklist
Additional Discussion Points