Open romaricpascal opened 1 year ago
colinrotherham
commented
1 year ago @romaricpascal I've updated the PR and made type checks optional via GitHub Actions
But for all the ECMAScript syntax checks to run I've added @typescript-eslint/parser where needed
36degrees
commented
1 year ago Moving this to blocked. The remaining change is to make the type checking mandatory on CI (#3319), but we don't want to do that until we've run some skill-sharing sessions on types as part of #3052.
36degrees
commented
1 year ago Planning to run a skill sharing session tomorrow (13 April) so should be able to make a decision about this after that.
What
Some work has already been merged to that effect with:
A chain of PRs are also pushing things further in that domain:
Why
We've already started adding some JSDoc blocs, including type annotations, to our components. The most recent push being in 4.4.0, with numerous addition of JSDoc blocs to document component's configuration options (for. ex the Accordion).
Adding these type annotations and ensuring they match our code helps both our users and our team.
For our users
Users get more information about our API
Because we also ship these JSDoc comments in the package, modern editors (which support accessing these comments) will reveal this information as they code.
This information include methods available to our components, what they do, the parameters they expect (and of which type)–including object keys–, and the type of their return values.
Further down the line these blocs of documentation could support publishing the documentation for our JavaScript API, similarly to what we do with Sass.
Users can get confidence they use our API as intended
By providing our JSDoc blocs, users' tooling can check that they use classes,methods or functions that do exist in our API, with the right kind of arguments and treating their results as intended.
This can give them more confidence that things work as intended, without adding a flurry of very narrow unit tests.
This can also let them catch breaking changes that would be hard to spot (for example a method that keeps the same parameters but for which one of them changed), or calls to methods/functions that we are deprecating.
For our team
Documenting our components
This information helps us keep track of the API we intend to ship (as well as internal APIs). This helps better follow what our code does when editing the project.
Ensuring our code matches the documentation
To be most helpful to developers, these comments need to match what our code does. TypeScript automates checking that these two things are in line.
With the information provided in our JSDoc blocs and its built in knowledge of ECMAScript and Web API, it can follows the trail of our code to check:
This calls for documenting not only our public API, but also our internal functions/classes/methods.
Being more confident in how our code works
As TypeScript follows our code, it catches calls to methods with the wrong arguments, calls to methods that wouldn't exist (and crash the script), or forgetting that a variable may be holding a multiple kinds of values that we need to handle differently...
This gives us more confidence that the code we ship works as we intend (similarly to how it helps our users check that they use our library it expects to be used).
Who needs to work on this
Developers
Who needs to review this
Developers
Done when
Following the meeting on Feb 2nd, we've decided to go ahead, so we need to: