feat: Typography ui #20
Conversation
Walkthrough이 PR은 Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
packages/ui/typography/src/Typography.tsx(node:55081) ESLintIgnoreWarning: The ".eslintignore" file is no longer supported. Switch to using the "ignores" property in "eslint.config.js": https://eslint.org/docs/latest/use/configure/migration-guide#ignoring-files Oops! Something went wrong! :( ESLint: 9.15.0 ESLint couldn't find an eslint.config.(js|mjs|cjs) file. From ESLint v9.0.0, the default configuration file is now eslint.config.js. https://eslint.org/docs/latest/use/configure/migration-guide If you still have problems after following the migration guide, please stop by Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (3)
packages/ui/typography/src/Typography.tsx (2)
4-7: 의미론적 변형(Semantic Variants) 추가 제안현재 구현은 기본적인 HTML 태그 렌더링을 잘 지원하고 있습니다만, 디자인 시스템의 일관성을 위해 의미론적 변형을 추가하는 것이 좋을 것 같습니다.
다음과 같은 변형을 추가하는 것을 고려해보세요:
type TypographyProps<T extends TextTags = TextTags> = { as?: T; children: string; + variant?: 'heading' | 'body' | 'caption' | 'label'; } & Omit<React.HTMLAttributes<HTMLElement>, "color" | "as" | "variant">;
6-6: children 타입 제한 완화 제안현재
children이 문자열로만 제한되어 있어 컴포넌트의 유연성이 제한될 수 있습니다.다음과 같이 React.ReactNode로 확장하는 것을 고려해보세요:
- children: string; + children: React.ReactNode;ssr/app/layout.tsx (1)
Line range hint
18-27: 스타일 중복 개선 제안모든 Link 컴포넌트에 동일한 스타일이 반복되어 있습니다. 스타일 재사용성을 높이기 위해 공통 스타일을 분리하는 것을 제안드립니다.
다음과 같이 개선해보시는 건 어떨까요:
+ const linkStyle = { + border: "1px solid black", + borderRadius: "0.5em", + padding: "0.2em 1em", + backgroundColor: "#cccccc" + }; <div style={{ display: "flex", flexDirection: "column", gap: "0.5em" }}> - <Link style={{ border: "1px solid black", borderRadius: "0.5em", padding: "0.2em 1em", backgroundColor: "#cccccc" }} href="/button"> + <Link style={linkStyle} href="/button"> Button </Link> - <Link style={{ border: "1px solid black", borderRadius: "0.5em", padding: "0.2em 1em", backgroundColor: "#cccccc" }} href="/modal"> + <Link style={linkStyle} href="/modal"> Modal </Link> - <Link style={{ border: "1px solid black", borderRadius: "0.5em", padding: "0.2em 1em", backgroundColor: "#cccccc" }} href="/typography"> + <Link style={linkStyle} href="/typography"> Typography </Link> </div>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (6)
packages/ui/typography/package.json(1 hunks)packages/ui/typography/src/Typography.tsx(1 hunks)packages/ui/typography/src/index.ts(1 hunks)ssr/app/layout.tsx(1 hunks)ssr/app/typography/page.tsx(1 hunks)tsconfig.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- ssr/app/typography/page.tsx
🔇 Additional comments (8)
packages/ui/typography/src/index.ts (2)
1-3: 모듈 구조가 깔끔하게 구현되었습니다!
배럴 패턴을 사용한 깔끔한 모듈 구조와 타입 내보내기가 잘 되어있습니다. "use client" 지시문도 적절하게 배치되어 있습니다.
2-3: Typography 컴포넌트 구현 확인이 필요합니다.
Typography 컴포넌트의 실제 구현과 사용 예시를 확인해 보겠습니다.
✅ Verification successful
Let's check the usage example in the typography page to get a complete picture.
Typography 컴포넌트 구현이 적절합니다
Typography 컴포넌트는 다음과 같은 특징을 가지고 있습니다:
- HTML 텍스트 태그들을 유연하게 지원 (
span,p,h1-h6등) - React.forwardRef를 통한 ref 전달 지원
- 적절한 타입 정의와 제네릭 사용
- 불필요한 props 제외 처리 (
color,variant등)
개선 제안사항:
- 컴포넌트 사용 예시와 문서화가 부족합니다. README.md 파일 추가를 권장드립니다.
- 기본 스타일링이나 테마 시스템과의 연동이 없습니다. 향후 필요에 따라 추가를 고려해보세요.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Typography 컴포넌트 구현 확인
# Typography.tsx 파일 검색
echo "Typography 컴포넌트 구현 확인:"
fd "Typography.tsx" packages/ui/typography/src -x cat {}
# 사용 예시 검색
echo "\n사용 예시 확인:"
rg -l "Typography" ssr/app/typography/
Length of output: 889
Script:
#!/bin/bash
# Typography 사용 예시 확인
echo "Typography 페이지 구현:"
cat ssr/app/typography/page.tsx
# README나 문서 확인
echo "\n문서 확인:"
fd "README" packages/ui/typography -x cat {}
Length of output: 305
packages/ui/typography/src/Typography.tsx (2)
19-23: 구현이 잘 되었습니다!
컴포넌트 이름 설정과 내보내기가 React 모범 사례를 잘 따르고 있습니다.
10-10: 🛠️ Refactor suggestion
타입 안전성 개선 필요
as any 타입 캐스팅은 타입 안전성을 저해할 수 있습니다.
다음과 같이 더 안전한 방식으로 변경하는 것을 추천드립니다:
- const { as: Comp = "span" as any, children, ...rest } = props;
+ const { as: Comp = "span" as T, children, ...rest } = props;Likely invalid or redundant comment.
ssr/app/layout.tsx (1)
25-27: 구현이 일관성있게 잘 되었습니다!
Typography 섹션에 대한 새로운 링크가 기존 네비게이션 스타일과 일치하게 구현되었습니다.
packages/ui/typography/package.json (2)
1-4: 패키지 기본 정보가 적절히 구성되었습니다!
스코프 네이밍과 버전 관리가 npm 컨벤션을 잘 따르고 있습니다.
48-54: 저장소 정보가 올바르게 구성되었습니다!
GitHub 저장소 URL과 이슈 트래커가 적절히 설정되어 있습니다.
tsconfig.json (1)
40-41: 경로 매핑이 올바르게 추가되었습니다!
Typography 컴포넌트를 위한 경로 매핑이 기존 UI 컴포넌트들의 패턴을 잘 따르고 있습니다.
경로가 올바르게 설정되었는지 확인하기 위해 다음 스크립트를 실행합니다:
✅ Verification successful
Typography 컴포넌트의 경로 매핑이 정상적으로 설정되었습니다
소스 디렉토리와 필수 파일들이 모두 존재하는 것을 확인했습니다:
./packages/ui/typography/src/Typography.tsx./packages/ui/typography/src/index.ts
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Typography 컴포넌트의 소스 디렉토리 존재 여부 확인
# Typography 컴포넌트 소스 디렉토리 확인
if [ -d "./packages/ui/typography/src" ]; then
echo "✓ Typography 소스 디렉토리가 존재합니다"
# 필수 파일들의 존재 여부 확인
fd . "./packages/ui/typography/src" -t f -e tsx -e ts
else
echo "✗ Typography 소스 디렉토리를 찾을 수 없습니다"
fi
Length of output: 242
| "dependencies": { | ||
| "@radix-ui/react-slot": "^1.1.0", | ||
| "@layer-lib/react-use-mounted": "workspace:*" | ||
| }, | ||
| "peerDependencies": { | ||
| "@types/react": "*", | ||
| "@types/react-dom": "*", | ||
| "react": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc", | ||
| "react-dom": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc" | ||
| }, | ||
| "peerDependenciesMeta": { | ||
| "@types/react": { | ||
| "optional": true | ||
| }, | ||
| "@types/react-dom": { | ||
| "optional": true | ||
| } | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
의존성 버전 범위를 더 구체적으로 지정해주세요.
현재 구성의 잠재적 문제점들:
- React 19.0.0-rc와 같은 릴리스 후보 버전을 프로덕션에서 사용하는 것은 위험할 수 있습니다.
@types/react와@types/react-dom의 버전이*로 지정되어 있어 타입 불일치 문제가 발생할 수 있습니다.
다음과 같은 수정을 제안드립니다:
"peerDependencies": {
- "@types/react": "*",
- "@types/react-dom": "*",
+ "@types/react": "^17.0.0 || ^18.0.0",
+ "@types/react-dom": "^17.0.0 || ^18.0.0",
- "react": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc",
- "react-dom": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc"
+ "react": "^17.0.0 || ^18.0.0",
+ "react-dom": "^17.0.0 || ^18.0.0"
}📝 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.
| "dependencies": { | |
| "@radix-ui/react-slot": "^1.1.0", | |
| "@layer-lib/react-use-mounted": "workspace:*" | |
| }, | |
| "peerDependencies": { | |
| "@types/react": "*", | |
| "@types/react-dom": "*", | |
| "react": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc", | |
| "react-dom": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc" | |
| }, | |
| "peerDependenciesMeta": { | |
| "@types/react": { | |
| "optional": true | |
| }, | |
| "@types/react-dom": { | |
| "optional": true | |
| } | |
| }, | |
| "dependencies": { | |
| "@radix-ui/react-slot": "^1.1.0", | |
| "@layer-lib/react-use-mounted": "workspace:*" | |
| }, | |
| "peerDependencies": { | |
| "@types/react": "^17.0.0 || ^18.0.0", | |
| "@types/react-dom": "^17.0.0 || ^18.0.0", | |
| "react": "^17.0.0 || ^18.0.0", | |
| "react-dom": "^17.0.0 || ^18.0.0" | |
| }, | |
| "peerDependenciesMeta": { | |
| "@types/react": { | |
| "optional": true | |
| }, | |
| "@types/react-dom": { | |
| "optional": true | |
| } | |
| }, |
| "exports": { | ||
| ".": { | ||
| "import": { | ||
| "types": "./dist/index.d.mts", | ||
| "default": "./dist/index.mjs" | ||
| }, | ||
| "require": { | ||
| "types": "./dist/index.d.ts", | ||
| "default": "./dist/index.js" | ||
| } | ||
| } | ||
| }, | ||
| "source": "./src/index.ts", | ||
| "main": "./dist/index.js", | ||
| "module": "./dist/index.mjs", | ||
| "types": "./dist/index.d.ts", | ||
| "files": [ | ||
| "dist", | ||
| "README.md" | ||
| ], | ||
| "sideEffects": false, | ||
| "scripts": { | ||
| "clean": "rm -rf dist", | ||
| "version": "pnpm version" | ||
| }, |
There was a problem hiding this comment.
빌드 스크립트 추가가 필요합니다.
현재 clean과 version 스크립트만 정의되어 있습니다. TypeScript 소스를 컴파일하고 번들링하기 위한 빌드 스크립트가 필요합니다.
다음과 같은 스크립트 추가를 제안드립니다:
"scripts": {
"clean": "rm -rf dist",
- "version": "pnpm version"
+ "version": "pnpm version",
+ "build": "tsup src/index.ts --format esm,cjs --dts",
+ "dev": "tsup src/index.ts --format esm,cjs --watch --dts",
+ "lint": "eslint \"src/**/*.ts*\"",
+ "test": "jest"
}📝 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.
| "exports": { | |
| ".": { | |
| "import": { | |
| "types": "./dist/index.d.mts", | |
| "default": "./dist/index.mjs" | |
| }, | |
| "require": { | |
| "types": "./dist/index.d.ts", | |
| "default": "./dist/index.js" | |
| } | |
| } | |
| }, | |
| "source": "./src/index.ts", | |
| "main": "./dist/index.js", | |
| "module": "./dist/index.mjs", | |
| "types": "./dist/index.d.ts", | |
| "files": [ | |
| "dist", | |
| "README.md" | |
| ], | |
| "sideEffects": false, | |
| "scripts": { | |
| "clean": "rm -rf dist", | |
| "version": "pnpm version" | |
| }, | |
| "exports": { | |
| ".": { | |
| "import": { | |
| "types": "./dist/index.d.mts", | |
| "default": "./dist/index.mjs" | |
| }, | |
| "require": { | |
| "types": "./dist/index.d.ts", | |
| "default": "./dist/index.js" | |
| } | |
| } | |
| }, | |
| "source": "./src/index.ts", | |
| "main": "./dist/index.js", | |
| "module": "./dist/index.mjs", | |
| "types": "./dist/index.d.ts", | |
| "files": [ | |
| "dist", | |
| "README.md" | |
| ], | |
| "sideEffects": false, | |
| "scripts": { | |
| "clean": "rm -rf dist", | |
| "version": "pnpm version", | |
| "build": "tsup src/index.ts --format esm,cjs --dts", | |
| "dev": "tsup src/index.ts --format esm,cjs --watch --dts", | |
| "lint": "eslint \"src/**/*.ts*\"", | |
| "test": "jest" | |
| }, |
| } & Omit<React.HTMLAttributes<HTMLElement>, "color" | "as" | "variant">; | ||
|
|
||
| const Typography = React.forwardRef(<T extends TextTags>(props: TypographyProps<T>, ref: React.ForwardedRef<HTMLElement>) => { | ||
| const { as: Comp = "span" as any, children, ...rest } = props; |
There was a problem hiding this comment.
타입을 any로 타입을 강제 캐스팅하신 의도가 따로 있으신지 궁금합니다!
|
|
||
| type TypographyProps<T extends TextTags = TextTags> = { | ||
| as?: T; | ||
| children: string; |
There was a problem hiding this comment.
children을 string으로 강제하는 이유가 있나용 ? 🤔 시맨틱하게 ReactNode로 허용해도 좋지 않을까 제안드려보아요
There was a problem hiding this comment.
To. @Doeunnkimm
반갑습미다 도은님 : )
기존 레이어 서비스에서 타이포그래피 컴포넌트는 정말 문자에 대한 타입만 받도록 하고 있었어요!
(물론 지금은 디자인 시스템 관련 라이브러리를 만들고 있지만)
그래서 Typography 컴포넌트에서 타이포에 대한 목적을 가지는 컴포넌트라면 타입을 string 으로 받아도 어색하지 않다는 생각을 했었는데, 혹시 ReactNode로 했을 때의 장점 또는 변경에 따른 사이드 이펙트는 없을까요?!
There was a problem hiding this comment.
아하 답변 이해했어요!
확장성 면에서 말씀드린 것이였는데요!
Typography를 사용하다보면 확장된 형태로 쓸 때 내부에 ReactNode를 두는 케이스가 있는 것 같아요!
예를 들면
// 이런거1
<Typography>
안녕하세요~ <a>깃헙 링크</a> 보내드려요~
</Typography>
// 이런거2
<Typography>
검정 <span style={{ color: 'blue' }}>파랑</span> 검정
</Typography>처럼 Typography의 스타일을 inherit 해서 사용할 수 있을 때 이점이 있을 것 같아요!
There was a problem hiding this comment.
아하 이해했습니다! 지금 범용적 라이브러리 목적으로 봤을 때 코멘트 남겨주신 부분이 굳인 것 같습니다! 체고체고
There was a problem hiding this comment.
문득 궁금한 것이... Typography는 theme에만 존재해도 되지 않을까하는 생각이 들었어유
지금 구현되어 있는 내용만 보았을 때는 tag 부분만 as로 들어가는 것이 전부인데 어떤 의미를 가지고 있는지 잘 모르겠어요!
ui에도 들어가게 된 히스토리가 있나용 ? 😲
Typography 컴포넌트 제작
택스트를 나타내는 컴포넌트를 제작했어요.as속성을 통해 택스트를 표현하는 태그를 사용할 수 있어요.Summary by CodeRabbit
New Features
@layer-ui/typography패키지가 추가되어 텍스트 렌더링을 위한Typography컴포넌트를 제공합니다./typography경로로 연결되는 링크가RootLayout에 추가되었습니다.TypographyPage컴포넌트가 생성되어 "Typography" 제목을 표시합니다.Bug Fixes
Documentation
package.json파일에 패키지 메타데이터가 정의되었습니다.Chores
@layer-ui/typography경로 매핑이 추가되었습니다.