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

fix: ensure strict checks are all recommended and related to #971

Merged
merged 1 commit into from
Aug 25, 2020

Conversation

agilgur5
Copy link
Contributor

@agilgur5 agilgur5 commented Aug 25, 2020

Description

  • strictPropertyInitialization and alwaysStrict were missing from
    recommended even though the other Strict Checks were in there and all
    of them are enabled if strict is enabled

  • alwaysStrict, strictNullChecks, noImplicitAny, and noImplicitThis were
    missing from strict's relatedTo even though all are enabled if strict
    is enabled

Tags

Found while working on #970.
Related to another PR I had on the topic of Strict Checks, #500

Review Notes

They're all listed as "recommended", but strict enables all of them and is the only one turned on in @tsconfig/recommend and by tsc --init, so maybe all of them should be removed from "recommended" except for strict instead? Thoughts?

- strictPropertyInitialization and alwaysStrict were missing from
  recommended even though the other Strict Checks were in there and all
  of them are enabled if strict is enabled

- alwaysStrict, strictNullChecks, noImplicitAny, and noImplicitThis were
  missing from strict's relatedTo even though all are enabled if strict
  is enabled
@orta
Copy link
Contributor

orta commented Aug 25, 2020

I'm in favor of this change, I agree that it could encourage redundant tsconfig entries ( which does make me wonder if we could make a page which helps you trim / understand your tsconfig visually #972 ) but I think it helps folks who aren't going to jump straight to strict

@orta orta merged commit 58683ff into microsoft:v2 Aug 25, 2020
@agilgur5
Copy link
Contributor Author

agilgur5 commented Aug 25, 2020

but I think it helps folks who aren't going to jump straight to strict

Doesn't the relatedTo help with that or the "Default false, unless strict enabled" handle that case?

I'm in favor of this change, I agree that it could encourage redundant tsconfig entries

TSDX itself had that too until I fixed it in jaredpalmer/tsdx#475. And I didn't notice the templates had them too until a user pointed it out in jaredpalmer/tsdx#619 and I fixed in jaredpalmer/tsdx#673, jaredpalmer/tsdx#690. The draft PR for monorepo template has redundancies too jaredpalmer/tsdx#778 (comment).. And I just noticed recently that we have declaration: true set redundantly too in the templates when it is the default

That is to say, it seems to be super common with TS users and a lot don't seem to know how all the options work (e.g. paths and baseUrl too). Though I'm sure you've seen that more than me. I only know after having read through the docs and now reference (which is a great addition!) several times as well as some issues in the TS repo.

( which does make me wonder if we could make a page which helps you trim / understand your tsconfig visually #972 )

Yea that could be pretty helpful if enough folks know of it and use it. I was thinking a tsc --trim command or something. Or other ways of recommending config -- I think the bases repo definitely helps as a reference point.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants