Add strict flag to CI TS typechecking

[MasterOdin]

PR adds the --strict flag to the tsc typechecking that the CI was doing. This increases the strength of typechecking (see https://www.typescriptlang.org/tsconfig#strict for full details), but for this particular case, it's mostly just for catching no implicit any usage. An alternative would be to use --noImplicitAny flag instead, but it doesn't hurt to have tsc validate the definition file is as strict as possible even if some of the stuff doesn't really apply.

Note, this PR will fail until #143 is merged.

[eriktrom]

thankyou

i knew there was a flag :)

will wait for build

[eriktrom]

An alternative would be to use --noImplicitAny flag instead, but it doesn't hurt to have tsc validate the definition file is as strict as possible even if some of the stuff doesn't really apply.

let me rethink my typescript days a sec

[MasterOdin]

Yup, for posterity, can see https://github.com/MasterOdin/node-portfinder/runs/7820328564?check_suite_focus=true from the first commit for an example of a failed test run due to the usage of implicit any.

[eriktrom]

Approved, awesome.

Let's use strict - i wonder if any of the people 'building' this library (something that is "news" to me, i'll learn more soon i'm sure) - might be affected? is that a remote possibility?

if not, i'm game, let's ship this

[eriktrom]

aka, httptoolkit/httptoolkit-server#51

hmmm

no implicit any i think then b/c of this? what do u think?

does it make it harder to fork if it's strict?

[eriktrom]

i would need to think - what is your suggestion?

[eriktrom]

strict

[eriktrom]

@MasterOdin - thanks again for your help today and last weekend

[MasterOdin]

People aren't "building" this library directly per se, but rather that when a consumer project uses tsc with skipLibCheck disabled, tsc will go through all dependencies' definition file and validate that they meet the tsconfig of the base project. There shouldn't be any meaningful change to maintainers of this project itself, other than that writing the definition file is a bit stricter. However, given that the definition file is a simple one at 62 lines, I'd hope that anyone with TS experience wouldn't find the new requirement onerous.

does it make it harder to fork if it's strict?

Only in that people cannot write type definitions that use implicit any, which isn't hard to avoid, so the added difficulty should be near zero in the scheme of things.

[MasterOdin]

I would add that this PR didn't need a new release as it wasn't touching anything end user facing, but I suppose there's no harm in it 🤷

[eriktrom]

i left a comment and then deleted it - yeah right?

[MasterOdin]

skipLibCheck just means that the declaration file for portfinder isn't validated against the consumer project, but the consumer is still validated that it's using whatever declarations were provided correctly, so there's still value in having the definition file.

So like, with skipLibCheck: true, then firebase-tools wouldn't have an error without #143, but like it would throw an error if I were to add:

import * as portfinder from 'portfinder';

portfinder.setBasePort('foo');

complaining about how setBasePort only accepts numbers, not strings.

[eriktrom]

@MasterOdin - check this out when u have a chance httptoolkit/httptoolkit-server#51

---

thanks - yeah - recalling how microsoft pulled this altogether now :)