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

⚙️ Eslint & Husky+lint-staged 설정 #11

Merged
merged 13 commits into from
Apr 18, 2024

Conversation

Legitgoons
Copy link
Member

@Legitgoons Legitgoons commented Apr 17, 2024

작업 이유

  • Husky와 lint-staged를 사용한 pre-commit 단계에서의 lint 실행 구현
    • 현재는 CI에서 eslint를 catch하는 방식
    • CI과정에서 모든 코드를 한 번에 확인하고 수정하는 것은 많은 시간과 리소스 소모
  • 합의된 Eslint 규칙 추가

작업 사항

1️⃣ Husky와 lint-staged를 사용한 pre-commit 단계에서의 lint 실행

  • .husky/pre-commit을 통해 commit 전에 문구가 출력되고, lint-front가 실행되도록 설정
//.husky/pre-commit
echo "🐶 Running lint-staged..."
yarn lint-front
  • package.json에서는 lint-front가 실행되면, lint-staged가 동작하도록 구현
  • lint-staged에서는 eslint --fix와 prettier --write를 실행
"scripts": {
  "postinstall": "husky",
  "lint-front": "lint-staged"
},
"lint-staged": {
  "*.{js,jsx,ts,tsx}": "eslint --fix",
  "*.{js,jsx,ts,tsx,css,md}": "prettier --write"
}
  • commit시 자동 실행
    image

2️⃣ 합의된 Eslint 규칙 추가

"no-var": "error", // var 금지
"no-multiple-empty-lines": "error", // 여러 줄 공백 금지
"no-console": ["error", { "allow": ["warn", "error", "info"] }], // console.log() 금지
"eqeqeq": "error", // 일치 연산자 사용 필수
"dot-notation": "error", // 가능하다면 dot notation 사용
"no-unused-vars": "error" // 사용하지 않는 변수 금지
"react/jsx-no-useless-fragment": "error" // 불필요한 <React.Fragment>, <> 사용 금지
"react/react-in-jsx-scope": "off", // import React 입력 필요 X

import/order

"groups": ["builtin", "external", "parent", "sibling", "index"],
// node.js의 내장모듈, 외부 설치 모듈, 상위 디렉토리 모듈, 현재/하위 디렉토리 모듈, 현재 디렉토리 인덱스 모듈
// 위의 순서대로 그룹화하여 정렬됩니다.
//...
'newlines-between': 'always', // 그룹 사이에 줄바꿈
alphabetize: {
  order: 'asc', // a~z순으로
  caseInsensitive: true, // 대소문자 구분 X

fsd-import

  • fsd 아키텍처를 원활하게 사용하기 위해 설치했습니다.
'fsd-import/fsd-relative-path': 'error',
'fsd-import/public-api-imports': 'error',
'fsd-import/layer-imports': 'error',

.eslintignore에 shared/mocks/* 추가

  • Mock API의 경우 임시적으로 서버의 역할을 대체하는 역할입니다.
  • 프로덕트처럼 엄격한 eslint rule을 적용할 필요가 없기에, eslintignore에 shared/mocks/*을 추가하였습니다.

3️⃣ 기타

components -> features 이름 변경

  • 합의에 따라 components 폴더를 features로 이름을 변경하였습니다.
  • 이에 따라 README의 이미지와 설명도 수정했습니다.

리뷰어가 중점적으로 확인해야 하는 부분

  • Husky와 lint-staged가 정상적으로 작동하는지
  • 추가된 Eslint 규칙이 합의와 일치하는지
  • broswer.ts 파일에서 msw & import/order 이슈가 있다는 것을 인지했는지
  • components 폴더를 features로 이름을 변경한 것을 인지했는지

발견한 이슈

  • eslint의 rule Definition를 찾지 못하는 이슈
    • eslint-plugin-react를 설치하고, eslint의 plugins에 'react'를 추가해 해결
  • eslint의 import/order에서 절대경로를 찾지 못하는 이슈
    • eslint-import-resolver-alias를 설치해 해결
  • mocks/browser.ts에서 import/order rule과 충돌하는 코드가 있어 에러가 발생함

package.json과 pre-commit 파일을 사용해 commit 전 lint를 실행하도록 설정하였습니다.
eslint의 rule Definition를 찾지 못하는 이슈가 있었습니다.
해결을 위해 eslint-plugin-react를 설치하고, eslint의 plugins에 'react'를 추가하였습니다.
broswer.ts에서 import/order 활성화 시 msw의 경로로 인해 에러가 발생합니다.
해당 이슈는 eslint-import-resolver-alias가 원인으로 추측됩니다.
우선 해당 파일에서 import/order를 비활성화하였습니다.
@Legitgoons Legitgoons self-assigned this Apr 17, 2024
Copy link

This pull request is automatically being deployed by Amplify Hosting (learn more).

Access this pull request here: https://pr-11.d37mn03xh3qyyz.amplifyapp.com

Copy link
Collaborator

@BangDori BangDori left a comment

Choose a reason for hiding this comment

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

fsd-import 플러그인에 대한 규칙이 빠져있는 것 같아서, Request Changes 하였습니다!

확인 부탁드립니다~

.eslintrc.cjs Show resolved Hide resolved
src/shared/mocks/browser.ts Outdated Show resolved Hide resolved
합의에 따라 mocks 관련 파일들에 ESLint 규칙 적용을 제외하기 위해 .eslintignore에 추가했습니다.
또한 mocks/browser.ts의 eslint-disable import/order 주석도 삭제처리했습니다.
Copy link
Collaborator

@BangDori BangDori left a comment

Choose a reason for hiding this comment

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

음 아래 의견은 제 개인적인 의견이니, 반영하셔도 좋고 바로 Merge 하셔도 좋을 거 같습니다~

고생하셨습니다~

.eslintignore Show resolved Hide resolved
@Legitgoons Legitgoons merged commit 1a8fa4a into main Apr 18, 2024
3 checks passed
BangDori pushed a commit that referenced this pull request Apr 19, 2024
@BangDori BangDori deleted the feature/eslint-husky-setting branch April 19, 2024 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants