-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
3주차 코드리뷰 PR #3
3주차 코드리뷰 PR #3
Conversation
- Add Babel configuration (.babelrc) - Add custom issue template for GitHub issues and PR template - Add Chromatic CI workflow in GitHub actions - Add essential configuration files (.gitignore, .husky/pre-commit, .prettierrc) - Add Storybook configuration (main.ts, preview.ts) - Add ESLint configuration (eslint.config.js) - Add project entry files (index.html, package.json, package-lock.json) - Add assets and initial structure for the application: - src/assets/styles/global/resetStyles.ts - src/components/common/Button with stories - src/pages, routes, store, mocks, and types directories - Add Vite configuration (vite.config.ts, index.html) - Initialize TypeScript configuration (tsconfig.json)
chore: initialize project setup
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
안녕하세요! Step3 과정을 함께하게 된 리뷰어 황준일입니다 🙏
앞으로 잘 부탁드려요!
...
일단 큰 변경사항은 없는 것 같아서 바로 머지하도록 하겠습니다 ㅎㅎ
그리고 폴더 구조는 step2에서 제공된 형태를 그대로 가져가려고 하는 것 같아요.
근데 이게 정말 적합한 폴더구조인걸까요?
이에 대한 고민도 깊게 해보시면 좋겠어요!
"scripts": { | ||
"dev": "vite", | ||
"build": "tsc -b && vite build", | ||
"lint": "eslint --cache 'src/**/*.{ts,tsx}'", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tsc로 type 검사도 따로 만들어주면 어떨까요?
"tsc": "tsc --noEmit"
이렇게 하나 추가해주고 활용할 수 있도록 해주면 좋아보여요!
"lint-staged": { | ||
"**/*.{tsx,ts,jsx,js}": [ | ||
"eslint --fix --cache", | ||
"prettier --write --cache" | ||
], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
마찬가지로, lint-staged에 type도 같이 검사할 수 있게 해주면 좋을 것 같아요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
그렇네요! 피드백 감사합니다! 😊
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"lint-staged": {
"**/*.{tsx,ts,jsx,js}": [
"tsc --noEmit",
"eslint --fix --cache",
"prettier --write --cache"
]
},
package.json
의 lint-stage에 다음과 같이 tsc --noEmit
을 추가하였을 때, 다음과 같은 오류가 발생하는데 무슨 이유인지 혹시 알 수 있을까요?
@JunilHwang
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@assets
에 대한 정의가 제대로 안 되어있는 것 같네요 ㅎㅎ
아마 tsconfig 와 관련된 문제 같아요.
일단 이 부분은 제외하고 진행하셔도 될 것 같아요..!
}, | ||
"devDependencies": { | ||
"@chromatic-com/storybook": "^2.0.2", | ||
"@emotion/css": "^11.13.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
emotion/css
는 devDependencies가 아니라 dependencies에 있어야 하지 않을까요~?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@emotion/css
를 따로 설치하다보니 실수로 devDependencies에 설치하게되었네요! 😂
웹 환경에 영향을 끼치니 dependencies에 설치하도록 수정하겠습니다!
import { Global, css } from '@emotion/react'; | ||
import resetStyles from '@assets/styles/global/resetStyles'; | ||
|
||
const globalStyles = css` | ||
${resetStyles} | ||
`; | ||
|
||
export default function GlobalStyles() { | ||
return <Global styles={globalStyles} />; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
컴포넌트가 assets이 될 수 있는걸까요~?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GlobalStyles
는 스타일을 적용하는 역할을 갖는다고 판단하여 styles
폴더 내에 관리하게 되었습니다.
그렇다면 styles
를 assets
내부에 관리하는 것 보단 src
경로로 따로 분리해서 관리하는것이 더 낫은 방식이 될 수 있을까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
styles 라고 퉁치기보단, 다른 용어가 필요할 수 있을 것 같아요!
가령 style-components 라거나?
css와 구분되는 내용이 있어야되지 않을까 싶어서요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GPT에게 assets이 뭘까 물어봤을 때의 답변입니다! 참고해주세요~
프론트엔드 프로젝트에서 assets
폴더는 일반적으로 애플리케이션에서 사용하는 정적 리소스를 저장하는 데 사용됩니다. 혼동을 피하기 위해 assets
폴더에 포함되는 파일과 그 구조를 명확히 하는 것이 중요합니다. 다음은 assets
폴더에 포함시킬 수 있는 파일 유형과 조직 방법입니다:
-
이미지 (Images):
- 모든 이미지 파일(
.png
,.jpg
,.svg
등)을 저장합니다. - 하위 폴더로 분류할 수 있습니다:
assets/images/
- 모든 이미지 파일(
-
폰트 (Fonts):
- 커스텀 폰트나 아이콘 폰트를 저장합니다.
assets/fonts/
폴더에 보관합니다.
-
스타일시트 (Stylesheets):
- CSS 또는 SCSS 파일을 저장합니다.
- 만약 스타일 파일이 별도의 폴더에 관리되지 않는다면
assets/styles/
에 저장합니다.
-
미디어 파일 (Media Files):
- 오디오나 비디오 파일을 저장합니다.
assets/media/
폴더를 사용합니다.
-
데이터 파일 (Data Files):
- JSON 또는 기타 정적 데이터 파일을 저장합니다.
assets/data/
에 보관합니다.
-
스크립트 (Scripts):
- 패키지 매니저로 관리되지 않는 서드파티 라이브러리나 플러그인 등 별도의 스크립트 파일을 저장합니다.
assets/scripts/
폴더를 사용합니다.
혼동을 피하기 위한 팁:
-
명확한 폴더 구조: 각 파일 유형별로 하위 폴더를 만들어 파일을 분류하면 어떤 파일이 어디에 있는지 쉽게 파악할 수 있습니다.
-
일관성 있는 네이밍: 파일과 폴더의 이름을 일관성 있게 지정하여 가독성을 높입니다.
-
소스 코드 분리: React 컴포넌트나 애플리케이션 로직과 같은 소스 코드 파일은
assets
폴더가 아닌src
나components
폴더에 저장합니다. -
빌드 산출물 분리: 컴파일되거나 생성된 파일(예: 번들된 JS/CSS 파일)은
assets
폴더에 포함시키지 말고, 일반적으로 소스 컨트롤에서 제외되는dist
나build
폴더에 저장합니다. -
주석 및 문서화: 복잡한 구조를 가진 경우 README 파일이나 주석을 통해 폴더 구조와 파일 사용 방법을 문서화합니다.
이러한 관행을 따르면 assets
폴더의 내용물이 명확해지고, 팀원들 간의 혼동을 줄일 수 있습니다. 프로젝트의 규모나 요구사항에 따라 폴더 구조를 조정하되, 항상 명확성과 일관성을 유지하는 것이 중요합니다.
Description