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

[fix] ts파일 첫째줄에서 빨간줄 에러 fix #560

Merged
merged 5 commits into from
Aug 18, 2024
Merged

Conversation

DaYoung-woo
Copy link
Contributor

Related issue

#557

Result

image

Work list

image

Discussion

vscode와 analysis-engine쪽에서 파일을 오픈하면 import 구문에서 빨간줄이 나타나는 현상이 있습니다.
.eslintrc.json파일을 변경하여 빨간줄이 나타나지 않도록 개선하였습니다.

Co-authored-by: bbanderson <[email protected]>
Co-authored-by: Madang Garden <[email protected]>
@xxxjinn
Copy link
Contributor

xxxjinn commented Jul 26, 2024

actions build 단계에서 에러가 나고 있는 것 같습니다..!!

@ytaek
Copy link
Contributor

ytaek commented Jul 28, 2024

actions build 단계에서 에러가 나고 있는 것 같습니다..!!

@xxxjinn님 말씀대로, action에서 설정해놓은 CI 단계에서 에러가 나는 현상입니다.

image

요기에 Details를 눌리면 build 어느 부분에서 에러인지 확인 가능하시구요.
(https://github.com/githru/githru-vscode-ext/actions/runs/10110905156/job/27961777656?pr=560)

local에서도 npm run build 등을 해보면 확인하실 수 있을꺼에요.

@mdgarden
Copy link
Contributor

mdgarden commented Jul 29, 2024

수정기간이 길어지고 있어 현재까지 확인한 내용에 대해서 정리하려고 합니다.

원래 이슈의 발생 원인

현재 githru의 리포지토리는 모노레포로 관리되고 있으며, vscode, view, analysis-engine의 세가지 리포지토리가 한 곳에서 관리되고 있습니다.
각각의 리포지토리의 루트에 tscofing.ts가 존재하며, 모노레포 전체의 루트 디렉토리에는 tsconfig.ts가 존재하지 않습니다.
각각의 리포지토리를 개별로 여는 것이 아닌, 모노레포 자체를 열게 되면 vscode 폴더와 analysis-engine 폴더에서는 상대경로가 아닌 절대경로로 루트 디렉토리에서 tscofing.ts를 찾고 있습니다. 이에 따라 린트 에러가 발생하고 있었습니다.

root
├── analysis-engine
│   ├── tsconfig.ts
│   ├── .eslintrc.json
│   └── package.json
├── view
│   ├── tsconfig.ts
│   ├── .eslintrc.json
│   └── package.json
├── vscode
│   ├── tsconfig.ts
│   ├── .eslintrc.json
│   └── package.json
└── package.json

이 PR에서 발생한 CI 빌드 에러 원인

저희가 수정하여 올린 extends의 작성방법이 잘못되었었습니다. "extends" 필드는 ESLint의 설정을 확장하는 데 사용되며, 배열 형태로 여러 가지 설정을 포함할 수 있습니다. 그러나 @typescript-eslint/parser는 ESLint 확장 설정이 아닌 파서이므로 extends 필드에 포함하면 안 됩니다.

작성했던 내용
"extends": ["eslint:recommended", "plugin:@typescript-eslint/recommended", "@typescript-eslint/parser", "prettier"],

올바른 작성 방법
"extends": ["eslint:recommended", "prettier", "plugin:@typescript-eslint/recommended", "plugin:prettier/recommended"],

추가적인 에러

상기의 방법으로 .eslintrc.json 파일의 extends 항목을 수정시, vscode폴더에서는 에러가 수정되나 analysis-engine폴더에서는 여전히 같은 에러가 발생하고 있으며, 이에 대한 해결책을 확인하고 있습니다.

@ytaek
Copy link
Contributor

ytaek commented Jul 30, 2024

분석해주신 것 잘 봣습니다!! 디테일해서 이해하기 쉬웠네요 👍
root에는 소스코드가 들어갈 계획이 없으므로, lint를 적용하지 않아도 될 것 같습니다.
그냥 npm script 실행만 되는 환경이면 ok 입니다.

@choisohyun
Copy link
Contributor

choisohyun commented Jul 30, 2024

요거 궁금해서 한번 테스트로 바꿔봤는데 쉽지 않군요 🥲
제가 시도한 방식은 eslint는 root로 이동하고, tsconfig도 비슷한 설정은 root에 두고 extends하는 방식으로 변경하는 거였는데 현재 프로젝트 설정이 거의 각각 패키지마다 별개로 되어 있는 형태라 고칠 곳이 너무 많은 것 같아요.. 🤔 (ci 돌리면 에러 폭발)
요건 테스트했던 PR입니다 #568
저도 처음에 빨간줄이 나왔는데 어찌 만져보다가 지금은 안나오는데 뭐때문에 안나오는지도 의문이군요..ㅋㅋㅋ(그동안 ci로 잘 돌았던거니 vscode 로컬설정으로 풀리는 것인지..?) 풀리면 속이 시원할 것 같아요!

@mdgarden
Copy link
Contributor

폭풍검색 끝에 두가지의 해결방법을 찾았습니다.

  1. 모노레포의 루트에 eslint파일을 추가하고 각 하위 패키지의 eslint파일들이 상위 디렉토리에 있는 eslint파일의 설정을 상속받게 하는 방법
  2. vscode가 모노레포인 것을 인식하도록 수정하기

1번은 수정범위도 크고 기존 eslint 파일의 의도도 전부 이해하기에는 어려움이 있었기에, 2번으로 진행하여 수정했습니다.
CI테스트 통과하였고, 각 패키지별로 vscode를 열었을 때도 정상적으로 작업할 수 있는 것을 확인했습니다.

Copy link
Contributor

@ytaek ytaek left a comment

Choose a reason for hiding this comment

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

오오오오 심플하게 해결되었네요!!!!
👍👍👍👍👍👍👍👍👍👍👍👍👍👍👍👍👍👍👍👍👍👍👍👍

Copy link
Contributor

@choisohyun choisohyun left a comment

Choose a reason for hiding this comment

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

요것이었군요..!!! 수고하셨습니다🤩🤩🥳🥳

@DaYoung-woo
Copy link
Contributor Author

헛 정원님 이슈를 해결해주셨군요 ㅠㅠ 제가 더 신경썼어야 하는데 너무 죄송하고 감사합니다 ㅠㅠ

@ytaek ytaek added this to the v0.7.0 milestone Aug 13, 2024
@ytaek ytaek merged commit eed5a2b into main Aug 18, 2024
2 checks passed
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.

6 participants