hackerwins / codepair-old

Real-time markdown editor for interviews, meetings and more...
https://codepair.yorkie.dev
Apache License 2.0
90 stars 29 forks source link

Add Lint and Type Check step to CI #204

Closed mu-hun closed 2 years ago

mu-hun commented 2 years ago

What this PR does / why we need it?

I thought it would be good to add a type check and a lint check step in CI.

I will remove the Draft status when the #201 PR is merged. (#201 is related to typing correction)

Any background context you want to provide?

I've remove npm build command in CI script. because it is a duplicate that already exists in other CD script.

Checklist

CLAassistant commented 2 years ago

CLA assistant check
All committers have signed the CLA.

mu-hun commented 2 years ago

npm ERR! notsup Unsupported platform for fsevents@2.1.3: wanted {"os":"darwin"} (current: {"os":"linux","arch":"x64"})

CI Run is failed in Install npm dependencies step :( Do I have to add the -f option when running the npm install command?

blurfx commented 2 years ago

npm ERR! notsup Unsupported platform for fsevents@2.1.3: wanted {"os":"darwin"} (current: {"os":"linux","arch":"x64"})

CI Run is failed in Install npm dependencies step :( Do I have to add the -f option when running the npm install command?

I think we should use -f flag for now. (We added the -f in #202. )

Or, could you please investigate cross-platform build is possible in any other way? (without adding -f?)

mu-hun commented 2 years ago

There is a problem that the npm run start command cannot be executed by webpack related dependencies:

> codepair@0.1.2 start
> REACT_APP_GIT_HASH=`git rev-parse --short HEAD` react-scripts start

There might be a problem with the project dependency tree.
It is likely not a bug in Create React App, but something you need to fix locally.

The react-scripts package provided by Create React App requires a dependency:

  "webpack": "4.44.2"

Don't try to install it manually: your package manager does it automatically.
However, a different version of webpack was detected higher up in the tree:

  x86chi/codepair/node_modules/webpack (version: 5.74.0)

Manually installing incompatible versions is known to cause hard-to-debug issues.

If you would prefer to ignore this check, add SKIP_PREFLIGHT_CHECK=true to an .env file in your project.
That will permanently disable this message but you might encounter other issues.

The below error log is telling me how to fix it, I tried up to step 4 but I'm having the same problem.

To fix the dependency tree, try following the steps below in the exact order:

  1. Delete package-lock.json (not package.json!) and/or yarn.lock in your project folder.
  2. Delete node_modules in your project folder.
  3. Remove "webpack" from dependencies and/or devDependencies in the package.json file in your project folder.
  4. Run npm install or yarn, depending on the package manager you use.

In most cases, this should be enough to fix the problem.
If this has not helped, there are a few other things you can try:

  5. If you used npm, install yarn (http://yarnpkg.com/) and repeat the above steps with it instead.
     This may help because npm has known issues with package hoisting which may get resolved in future versions.

  6. Check if x86chi/codepair/node_modules/webpack is outside your project directory.
     For example, you might have accidentally installed something in your home folder.

  7. Try running npm ls webpack in your project folder.
     This will tell you which other package (apart from the expected react-scripts) installed webpack.
  1. If you used npm, install yarn (http://yarnpkg.com/) and repeat the above steps with it instead.

Is it okay to try replacing the package manager with yarn by following this instructions?

blurfx commented 2 years ago

Hmm. If there is no problem, I think we can change it to Yarn.

@x86chi Have you tried Yarn to see if the problem is resolved?

@yorkie-team/maintainers What do you think?

hackerwins commented 2 years ago

@blurfx , @x86chi For this reason, using yarn seems to be a good idea.

mu-hun commented 2 years ago

I've resolve this issue by remove duplicate "devDependencies" key in 5585a28 commit.

I haven't made any changes to this. this issue was created from #202 PR https://github.com/yorkie-team/codepair/commit/026331c10fa15fbb2045bec6aa3e04c49b5b0635#diff-7ae45ad102eab3b6d7e7896acd08c427a9b25b346470d7bc6507b6481575d519R103-R104 commit.

mu-hun commented 2 years ago

@blurfx: Hmm. If there is no problem, I think we can change it to Yarn.

A long time has passed. 🎉 This problem has been resolved independently, so I will make a separate PR for moving to Yarn later.

npm start ScreenShot

스크린샷 2022-09-12 오후 5 25 34

I started codepair with docker container and checked that the service is running well.

mu-hun commented 2 years ago

I'm waiting for the merge. 🥺

mu-hun commented 2 years ago

No, there was an unintended typo in config file. I've fixed it.

mu-hun commented 2 years ago

https://github.com/yorkie-team/codepair/actions/runs/3052834437/jobs/4922683966#step:6:5

The previous problem was solved, but type check was not passed due to an invalid type definition in an react-simplemde-editor external module. 😕

hackerwins commented 2 years ago

Do we need to add skipLibCheck option? tsc --skipLibCheck https://stackoverflow.com/questions/51634361/how-to-force-tsc-to-ignore-node-modules-folder

mu-hun commented 2 years ago

Thats strange.. skipLibCheck option is already true in the tsconfig.json file :(

https://github.com/x86chi/codepair/blob/ci/lint/tsconfig.json#L10

blurfx commented 2 years ago

Maybe we should use npm run instead of npx on ci workflow? or pass tsconfig filepath?

mu-hun commented 2 years ago

I tried both suggestions but still got the same error.

I looked at two related cases, but I encountered more errors about @types/react-router:

It seems to be a version dependency issue.

blurfx commented 2 years ago

I've already talked to @x86chi, this problem seems to be a typescript version compatibility issue between codepair(3.7) and the library(4.x).