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

Update versions of lint libraries #201

Closed gollumnima closed 2 years ago

gollumnima commented 2 years ago

What this PR does / why we need it?

Any background context you want to provide?

I've uploaded issue with type on #196 Do I roll back typescript to previous version for the backward compatibility or just add type file for navigator?

What are the relevant tickets?

Working on #196

Checklist

CLAassistant commented 2 years ago

CLA assistant check
All committers have signed the CLA.

gollumnima commented 2 years ago

I've pulled updated code and downgraded version of typescript.

blurfx commented 2 years ago

CI build fails because sass is not included in the dependencies. I think this problem will be solved when #202 is merged into master.

gollumnima commented 2 years ago

@blurfx I've rolled back the code. Sorry for the late response.

blurfx commented 2 years ago

@gollumnima I think I missed something. Why did you commented these codes before? to remove or just temporary?

gollumnima commented 2 years ago

@blurfx Because when I updated the version of typescript, the commented code made type error. So I just rolled back to the previous version. At the time, I was considering whether updating the version of typescript and making d.ts file to fix type error or just removing this code. I thought I had left a note for this somewhere, but I cannot find any note🥲

gollumnima commented 2 years ago

@blurfx Ah I found this link. Then can I delete the commented code here?

blurfx commented 2 years ago

@gollumnima Sorry for late response. I think we can drop old browser support through remove these commented codes.

gollumnima commented 2 years ago

image @blurfx In the local environment, the build succeeded but ci/build failed for this lint issue. Can I just add dependencies in useEffect logic? I'm afraid that my work might affect the result of the whole code.

blurfx commented 2 years ago

@gollumnima I think some empty dependencies are intended. How about changing the eslint react-hooks/exhaustive-deps rule to warn?

gollumnima commented 2 years ago

@blurfx I think our ci rule is too strict for the lint issue 🥲.

blurfx commented 2 years ago

The warning was treated as an error when the action runner set process.env.CI to true.

Now we have many ways to solve this problem.

  1. Turn off the react-hooks/exhaustive-deps rule
  2. Add eslint-ignore comments to all code where errors occur
  3. Modify hook dependency
  4. set process.env.CI to false manually
gollumnima commented 2 years ago

@blurfx Finally...!

blurfx commented 2 years ago

Great work 😄 Thank you for your contribution!