Validate types with tsc without skipLibCheck option + introduce type declaration checking
Hi Vova, I experienced a number of type errors when I imported the @anycable/web package into my codebase, but didn't notice the errors when I pulled down anycable-client and ran yarn tsc. Realized it was because skipLibCheck in tsconfig.json had skipped checking the type declaration files (*.d.ts) in this repo.
- fixed all the errors by disabling the skipLibCheck flag
- still need skipLibCheck for check-dts to pass due to errors from
AbortControllerandAbortSignal, so added it back to tsconfig.json after I was done - added a separate typecheck with
tscusing a different config (noskiplibcheck_tsconfig.json) that will check the type declarations in this repo
Great! Thanks!
Could you please take a look at CI failures?
@palkan is there a way for me to kick off another CI run, or can only you do that?
is there a way for me to kick off another CI run, or can only you do that?
It's only me for first-time PRs; as soon as we get this merged, all your subsequent PRs will run automatically.
Looks like we need to add some configuration to ignore node_modules/**/* and **/error.ts files from type checking.
~Hmmm, I had trouble trying to repro the type failures on the CI run because my local tests can't get that far in the process due to import issues. I think the type failures on CI stem from the check-dts step.~
~That didn't fail before because skipLibCheck was included.~
~Because we only care about the outputs from the packages directory, I propose we change the check to check-dts 'packages/**/*'. What do you think?~
I can repro now with yarn check-dts.
Those type failures are caused by swapping the position of the [Parameters<ActionsType[A]>[0]] type (e0224d2e). I'm assuming that the behavior modeled in channel/errors.ts is correct and my changes were wrong. But I also don't think the existing declaration was correct because there was a type error prior to the swap:
✖ packages/core/channel/index.d.ts:73:21: Type error TS2344
Type 'ActionsType[A]' does not satisfy the constraint '(...args: any) => any'.
~I'll try to look into that error more this afternoon, but if you have any spare time on your end, I'd love a second set of eyes on fixing this declaration:~ https://github.com/anycable/anycable-client/blob/9512487dec3780dd1d5125537479b6e392010d3b/packages/core/channel/index.d.ts#L67
Fixed the declaration with the second-to-last commit.
~We'll also need to figure out a way to ignore the globals.d.ts AbortController errors.~
Seems the above is common enough in check-nts that their FAQ addresses it by adding skipLibCheck 😕 :
https://github.com/ai/check-dts/blob/7020f61159798fbe19d2a1e38aadd75ab0c7ccc5/README.md#i-am-getting-an-error-from-node-types-how-do-i-skip-node_modules
@palkan can I please trouble you to kick off another CI run?
Also, the last commit with introducing a secondary typescript config file may be a bit controversial. I wish the logic was inverted where check-dts could take in an explicit option to skip declaration files, and tsc could just rely on the default tsconfig.json file.
Thanks!
Upgrading @types/node fixed the problem with check-dts, so we don't need a separate config.