chore: Add typescript to type check JS Interface via JSDoc
#2611
Conversation
szokeasaurusrex
left a comment
szokeasaurusrex
left a comment
There was a problem hiding this comment.
Thanks Lukas! I have a few questions. Also, I am not sure that the type checker is indeed required following this change. I'll do some experimenting to verify
szokeasaurusrex
left a comment
szokeasaurusrex
left a comment
There was a problem hiding this comment.
Looks good, thank you for adding this!
One final question though (non-blocking for this PR): Does the type checker enforce that everything has types declared (at least, that the public API does)? Or, is it possible that we could expose something which is untyped?
It doesn't enforce that the types are declared publicly (i.e. for library consumers). At least not in the current setup. We could change this eventually though, by telling typescript to not only type-check but also emit type declaration files. Might be a nice follow-up but I don't have time for this today anymore. Opened #2617 to track. Within the codebase, typescript simply infers the types of all non-explicitly annotated variables and exports. So if I have something like module.exports.x = "foo"TS will infer that |
This PR adds
typescriptto the repo to type-check the JS interface of Sentry CLI. I decided to build on top of the already existing JSDoc type annotations but actually enforce them by running TS.This also surfaced an incorrect type annotation which I fixed as well, along with adding some minor annotations that were necessary. In light of #2606, I also had to ignore a type error but we'll fix this in the next major.
Added a type checking job to CI, which IIUC should also be required after #2599.
closes #2604