Annotate our code with types and lint to ensure their proper usage

[romaricpascal]

What

• Add type annotations via JSDoc to our codebase to document the types of data that our components and functions manipulate.
• And linting to ensure our code is consistent with what these types announce.

Some work has already been merged to that effect with:

• https://github.com/alphagov/govuk-frontend/pull/2913
• https://github.com/alphagov/govuk-frontend/pull/2997
• https://github.com/alphagov/govuk-frontend/pull/3102
• https://github.com/alphagov/govuk-frontend/pull/3103

A chain of PRs are also pushing things further in that domain:

• https://github.com/alphagov/govuk-frontend/pull/2987
• https://github.com/alphagov/govuk-frontend/pull/3123
• https://github.com/alphagov/govuk-frontend/pull/3104
• https://github.com/alphagov/govuk-frontend/pull/3319

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:

1. that we're calling function/methods or instantiating classes that do exist...
2. ...passing them the appropriate arguments, of the type they expect...
3. ...and manipulating their results in a way that matches their type (see 1).

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

• [x] Gather remaining reserves about types
• [x] Answer these reserves
• [x] Make decision about going ahead (and record in the decision log?)

Following the meeting on Feb 2nd, we've decided to go ahead, so we need to:

• [x] Review the code changes for potential breaking changes
  • [x] https://github.com/alphagov/govuk-frontend/pull/2987
  • [x] https://github.com/alphagov/govuk-frontend/pull/3123
• [x] Decide how to merge the linting (enable it by default, or have it available to run but optional)
  • [x] https://github.com/alphagov/govuk-frontend/pull/3104
  • [x] https://github.com/alphagov/govuk-frontend/pull/3319
• [x] Communicate to the rest of the devs the introduction of type linting (and resources to help them)
  • Provide examples/references of JSDoc blocs, linting feedback and issues they caught
  • https://github.com/alphagov/govuk-frontend/issues/3060
• [ ] Review need for https://github.com/alphagov/govuk-frontend/issues/2835

[colinrotherham]

@romaricpascal I've updated the PR and made type checks optional via GitHub Actions

• https://github.com/alphagov/govuk-frontend/pull/3104

But for all the ECMAScript syntax checks to run I've added @typescript-eslint/parser where needed

[36degrees]

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]

Planning to run a skill sharing session tomorrow (13 April) so should be able to make a decision about this after that.