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

Add more Typescript and Eslint rules #33

Open
adamseth2 opened this issue Nov 10, 2024 · 4 comments
Open

Add more Typescript and Eslint rules #33

adamseth2 opened this issue Nov 10, 2024 · 4 comments
Assignees
Labels
High Priority Most of our effort should be on here Workload Medium Some things to do

Comments

@adamseth2
Copy link
Collaborator

adamseth2 commented Nov 10, 2024

Context

Due to the increasing size of the team, code quality might suffer as everyone has their coding style. To remedy this, we should implement stricter ESLInt and ts.config rules. Due to workflow in #29 , these checks will be done automatically so this will save us a lot of time in the future.

Prereq

  • Requires a deep knowledge of javascript and typescript.

Example

Explore if we should make this true (Currently it is false):
"noImplicitAny": ???

@adamseth2 adamseth2 added High Priority Most of our effort should be on here Workload Medium Some things to do labels Nov 10, 2024
@uellenberg
Copy link
Collaborator

noImplicitAny should remain in use, as it helps catch issues where type inference fails. Normally when this happens (without noImplicitAny), we opt out of type checking automatically (and implicitly), which makes TypeScript less useful.

@uellenberg
Copy link
Collaborator

It looks like noImplicitAny is actually off. I'll look into turning it on (and fixing all the problems that it highlights).

@adamseth2
Copy link
Collaborator Author

Oops, sorry I reversed it (fixed it in the issue). Yeah ideally we want to make it on as the issue previously was people wouldn't use typescript, and as they wouldn't get any errors, they assumed they were fine. But like you said, this will require fixing a lot of stuff, but in the future will fix a lot of issues

@uellenberg uellenberg linked a pull request Nov 11, 2024 that will close this issue
@uellenberg
Copy link
Collaborator

I did some work in #35, although there's still more we can do as far as eslint goes (especially turning warnings into errors).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
High Priority Most of our effort should be on here Workload Medium Some things to do
Projects
None yet
Development

No branches or pull requests

2 participants