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

Turn on strict typescript flags in tsconfig.json #1280

Open
melink14 opened this issue Nov 17, 2022 · 5 comments
Open

Turn on strict typescript flags in tsconfig.json #1280

melink14 opened this issue Nov 17, 2022 · 5 comments
Labels
Code health Issues which if fixed would improve the health of the project (as opposed to new features/bugs)

Comments

@melink14
Copy link
Owner

When we can we should turn on:

  • noImplicitAny
  • noUnusedLocals
  • noUncheckedIndexedAccess

Found from testing which flags were required in typescript playground for rikaicontent.ts to compile.

@melink14 melink14 added the Code health Issues which if fixed would improve the health of the project (as opposed to new features/bugs) label Nov 17, 2022
@tora-pan
Copy link
Contributor

This could be quite the undertaking.

Do you think it would be possible to do this incrementally?

My thoughts would be to do something like:
tsc --p tsconfig.strict.json && tsc --p tsconfig.nonstrict.json

// strict.tsconfig.json
{
  "compilerOptions": {
    "strict": true
  },
  "include": [
    "a.ts"
  ]
}

and

// tsconfig.nonStrict.json
{
  "compilerOptions": {
    "strict": false
  },
  "include": [
    "b.ts",
    "c.ts"
  ]
}

Not sure if this would work or not but if so, it would allow us to do this file by file.

Thoughts?

@melink14
Copy link
Owner Author

To be clear, strict mode is already enabled for all files with exceptions for the flags as defined in the first post! (ref https://www.typescriptlang.org/tsconfig#strict)

That said, I think it's a great idea to allow flexibility if some files are easier to migrate than others. We could actually make a file for each flag or make a file for all three stricter flags depending on how easy/hard it was.

@melink14
Copy link
Owner Author

melink14 commented Apr 17, 2023

I was a bit off. noImplicitAny is already turned on since strict mode is on and as far as I can tell, we're not manually setting it to false.

The other two are separate from strict mode but just seem like good ideas I think.

I think this issue title was a bit misleading since it's goal is stricter TS flags but by using the word 'strict' it made it easy to confuse with that flag in particular.

(or I'm misunderstanding something about the current TS config)

@tora-pan
Copy link
Contributor

Sorry, I shouldn't have used the word "strict" what I meant was just to have different tsconfig files with different levels of strictness, "flags" turned on.

I think I remember previously when I turned them on, there were hundreds of warnings/errors so I wanted to find a way to decrease the amount of changes for each PR.

@melink14
Copy link
Owner Author

Did you try only the flags listed here or other flags too.

unused locals should be fairly easily to auto-fix (since they're unused!) unless some missing type information makes some of the alerts false positives.

Unchecked index access is trickier since it requires accurate types and there are some places in rikaikun which were written in a pretty type free manner...

Either way, the first step would be to finalize the set of flags we want to attempt and then I think breaking down like you said is a good idea!

mergify bot pushed a commit that referenced this issue Jun 5, 2023
- Add noUnusedLocals property mentioned in this issue: #1280
- Remove unused `dictCount` property detected by added flag.

Other useful tsconfig flags can be handled in subsequent pull requests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code health Issues which if fixed would improve the health of the project (as opposed to new features/bugs)
Projects
None yet
Development

No branches or pull requests

2 participants