dx: add typescript-eslint

csdojo-defaang / defaang

A website that will curate recently-asked interview questions from FAANG+. Currently inactive. Check out: https://github.com/ykdojo/OpenStream

MIT License

509 stars 120 forks source link

dx: add typescript-eslint #155

Closed benny123tw closed 2 years ago

benny123tw commented 2 years ago

🛠️ Fixes Issue

Closes #154

👨‍💻 Changes proposed

add @typescript-eslint/eslint-plugin and @typescript-eslint/parser to ESLint
add 5 type check to tsconfig
add type check script to package.json to check Typescript errors
remove unnecessary pattern in prettierignore and eslintignore

📄 Note to reviewers

none

📷 Screenshots

none

vercel[bot] commented 2 years ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated
defaang	✅ Ready (Inspect)	Visit Preview	Sep 2, 2022 at 10:03PM (UTC)

iShibi commented 2 years ago

Thank you for your contribution. Unfortunately, we can't merge these changes because of the following reasons:

The linter in Next.js already supports TypeScript out of the box.
The strict rule in tsconfig turns on all the strict mode family options, no need to turn them on individually again.
The contents of the .ignore files are not doing any harm, so let's not change something that's not broken.

benny123tw commented 2 years ago

Hi @iShibi, thanks for the comment.

I don't think eslint-plugin-next has Typescript lint.
didn't notice strict mode is on, haha, but strict mode only enables all strict type-checking options, which do not include Additional Checks like noUnusedLocals, noUnusedParameters, or noImplicitReturns.
removing unnecessary patterns in the .ignore file can keep things clean and not break any things, IMO.

Just a suggestion. It's all up to you.

iShibi commented 2 years ago

Looks good. Now to pass the vercel build, run npm run type:check and fix the errors that show up. Most of them are unused variables that you can just remove. In hello.ts replace req with an _

benny123tw commented 2 years ago

Looks good. Now to pass the vercel build, run npm run type:check and fix the errors that show up. Most of them are unused variables that you can just remove. In hello.ts replace req with an _

Done. Should we put npm run type:check in the commit hook? Because I found that it is hard to debug on Vercel deploy (to someone that is no permission).

iShibi commented 2 years ago

I'll add that in a PR I was thinking about creating to add a type check job in the test workflow.