Skip to content
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

[Feat] 패키지 업데이트 및 린터 설정 #140

Merged
merged 29 commits into from
Sep 7, 2022
Merged

[Feat] 패키지 업데이트 및 린터 설정 #140

merged 29 commits into from
Sep 7, 2022

Conversation

seokzin
Copy link
Contributor

@seokzin seokzin commented Sep 2, 2022

📌 이슈


📝 설명

✔️ ESLint

1. 린트 적용 및 오류 수정 완료

오류 수정 과정에서 리팩토링을 진행했는데 논의가 필요해 보여요.

2. Import/Export 정렬

가나다순까지 정렬되는 꽤 야무진 플러그인이네요! 👍

3. Next 린트 범위

  • next lint 명령은 기본적으로 /pages, /components/ /lib만 해당돼요.
  • 그래서 --dir를 통해 범위를 전체로 변경했어요.

✔️ stylelint

1. 린트 적용 및 postcss 버전 의존성 문제 해결

TypeError: Cannot read properties of undefined (reading 'stringify')

2. 스크립트 주의 사항

  • 스크립트에서 곁따옴표와 홑따옴표를 혼용하면 에러가 발생해요.
  • 이스케이프 시퀀스를 통해 해결했어요.

4. 일부 규칙들 논의


✔️ 패키지 업데이트

Yarn으로 일괄 업데이트 했으니 재설치 해주세요.
새 환경에서 테스트하다가 스크립트 이슈를 발견했어요. (28ffe1e)


✔️ Git Hooks

  • husky + lint-staged를 통해 린트 및 테스트를 자동화했어요.
    • pre-commit : lint + prettier + stylelint
    • pre-push ; jest --coverage

📷 스크린샷

image

깨끗 🤗

@netlify
Copy link

netlify bot commented Sep 2, 2022

Deploy Preview for moida ready!

Name Link
🔨 Latest commit b337b44
🔍 Latest deploy log https://app.netlify.com/sites/moida/deploys/6317eb353ba93a00086cc21e
😎 Deploy Preview https://deploy-preview-140--moida.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@netlify
Copy link

netlify bot commented Sep 2, 2022

Deploy Preview for moida-ui ready!

Name Link
🔨 Latest commit b337b44
🔍 Latest deploy log https://app.netlify.com/sites/moida-ui/deploys/6317eb35b7f1e00008abe23a
😎 Deploy Preview https://deploy-preview-140--moida-ui.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@seokzin seokzin changed the title [Chore] 패키지 및 린트 설정 [Chore] 패키지 업데이트 및 린트 설정 Sep 2, 2022
@seokzin seokzin changed the title [Chore] 패키지 업데이트 및 린트 설정 [Feat] 패키지 업데이트 및 린트 설정 Sep 2, 2022
@seokzin seokzin changed the title [Feat] 패키지 업데이트 및 린트 설정 [Feat] 패키지 업데이트 및 린터 설정 Sep 2, 2022
- #055a9d1
- 이미 함수형 컴포넌트에 수정을 가해 린트 에러 발생
- 무의미한 수정이라 생각하여 초기 상태로 롤백
- HTMLInputElement의 기본 타입 (value: string) 참고
- 객체지향 원칙을 토대로 불필요한 의존성을 위임했어요
- no-empty-function 오류 해결 위한 일종의 더미
- includes는 집합 관계를 엄격히 따져요.
  (microsoft/TypeScript#26255)

- unknown을 통한 이중 타입 단언으로 타입을 조심스래 맞춰야 해요.
  (이펙티브 타입스크립트 item 9)

- 그리고 size 또한 undefined가 아님을 보장해야 돼요.
letter-spacing: ${({ size }: any) =>
TitleList.includes(size) ? "-0.03rem" : "-0.02rem"};
letter-spacing: ${({ size }) =>
size && (TitleList as unknown as FontSizeKey).includes(size)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

font-size: ${({ size = 'body1', theme }) => theme.fontSize[size]}px;
위에 이 코드가 있어서, 여기서도 같은 방식으로 처리하면 좋을 것 같아요!

NavOptions {}
NavOptions {
topItemsLogin: itemOptions[]
topItemsLogout: itemOptions[]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NavOptions를 따로 분리하는게 불필요하다고 생각되는데요.
다른 컴포넌트도 Options를 다 분리했던걸로 기억하는데, 요거 다음에 합치면 좋을 것 같아요

export interface ChildrenProps<Children = React.ReactNode> {
children?: Children;
export interface ChildrenProps {
children?: React.ReactNode
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

굿굿쟙 👍

@seokzin
Copy link
Contributor Author

seokzin commented Sep 7, 2022

사용하면서 불편함을 찾아 보완할 예정이에요.
특히 pre-push의 테스트 자동화는 pre-commit과 합치고 싶어요.

@seokzin seokzin merged commit a96051a into dev Sep 7, 2022
@seokzin seokzin deleted the feat/lint branch September 7, 2022 01:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feat] 패키지 업데이트 [Feat] 린터 설정 [Feat] Prettier 적용 및 자동화
2 participants