Improve CI to detect bad internal typing in the project

[oscard0m]

Is it possible (maybe in a different PR) to add test coverage to prevent this breaking change in the future?

Totally - so a quick solution would be to just do npm run build as part of our PR checks, to ensure building is always successful, but we can also use @typescript-eslint/explicit-module-boundary-types to ensure all exports are completely typed.

That's something I think should be addressed in another PR.

I can create an issue for this too, and we discuss if npm run build as a Check Run would be the solution to go. Thumbs up if you are ok with it

Originally posted by @oscard0m in #422 (comment)

[oscard0m]

Let's keep the discussion here. @G-Rath

@gr2m @wolfy1339, would this npm run build extra check be a good solution here?

[gr2m]

I don't mind adding npm run build to the test CI script. But isn't jest doing a build internally anyway before running tests?

[G-Rath]

I believe it'll be because ts-jest probably doesn't generate declaration files, so it's just compiling into JS.

I think we should be able to just do a tsc --noEmit --declaration (or something - I can figure out the exact command later when I've got the time) to confirm things are working.

[oscard0m]

Our CI is actually running this npm run validate:ts

Would that be as easy as adding this? #429

---

Still, I would advocate for a more auto-maintainable solution if possible.

[G-Rath]

Our CI is actually running this npm run validate:ts

I've not had the time yet to compare what that's actually doing (and not doing).

Would that be as easy as adding this? #429

That PR (as it is currently) isn't really testing anything aside from that the properties exist, which we know already because we're using TypeScript.

---

Just to be clear: our current setup is already testing for the majority of things - the error that was missed (and that lead to this issue) is a very specific and rare one that only occurs when you're generating declarations that inline an aliased type.

(fwiw this is actually the first time I've ever hit this error)

My understanding on why it doesn't happen elsewhere is that import & export are actually "just" keywords to TS at typecheck time, so its "allowed" to use a type that's not exported by nature of type-checking Javascript doesn't follow the same rules on what it can and cannot access.

The reasoning behind this is because otherwise TS would be like ESLint which errors anytime you have invalid syntax - so TS would just give up on type checking a non-exported type (which'd probably actually prevent it suggesting imports too, now that I think about it).

I just want to make sure we're not making a mountain out of a molehill :)

[G-Rath]

There is also a chance this is actually a bug in TS just FYI - some of my research when fixing the error turned up a number of GH issues that were marked as bugs that were fixed in later versions (of TS v2 specifically)

[oscard0m]

I'll wait for your research when you have time. No worries :)

[wolfy1339]

@G-Rath do you have any update for this issue?

[gr2m]

I'll close due to inactivity, but please feel free to reopen