Ensure JSDoc blocks are complete

[colinrotherham]

This PR completes a review of all JSDoc comments for completeness

All blocks are now documented with descriptions, parameters and optional return (or throws) types

/**
 * Example method
 *
 * @param {Element} $header - Section header
 * @param {number} index - Section index
 * @returns {Accordion} Accordion component
 */
Accordion.prototype.example = function ($header, index) {
  var $example1 = $header.querySelector('.' + this.example1Class)
  var $example2 = $header.querySelector('.' + this.example2Class)

  // Code goes here
  return this
}

Which continues work we started for v4.4.0:

• Enable ESLint JSDoc checks #2913
• Various JSDoc + type checking fixes (for v4.4.0 only) #2997

To assist with code reviews I've split these changes into:

1. Ensure JSDoc blocks are complete #3102
2. Configure ESLint to require JSDoc @params etc #3103
3. Various JSDoc + type checking fixes #2987
4. Enable type declaration guards #3123
5. Add (optional) type declaration checks in app, src #3104

Resolves #2513

[romaricpascal]

Looks pretty neat! Thanks for going over those 🙌🏻 Just spotted a little type issue with the config I think 😄

[romaricpascal]

issue I don't think that's the right type here. The configuration values may be numbers (in the character count) or booleans (in the button), so we also return Object<string, any>. That returned any won't be an object as things are flattened, but I don't think that's something we need to specify.

Suggested change

	* @returns {Object<string, string>} Flattened object with dot-separated keys
	* @returns {Object<string, any>} Flattened object with dot-separated keys

[colinrotherham]

Great spot, and it's found another potentially crashing bug too

We're trusting that translation "strings" in I18n() are always strings
https://github.com/alphagov/govuk-frontend/blob/main/src/govuk/i18n.mjs#L46

I might try and use unknown (checks needed) instead of any (doesn't matter)

[colinrotherham]

Yep all works with Object<string, unknown> 👍

I've removed the TranslationsFlattened type as it was fibbing 😬

/**
 * Translated messages (flattened)
 *
 * @private
 * @typedef {Object<string, string> | {}} TranslationsFlattened
 */

You'll see some extra commits in the other PR now to check for string/number/boolean values:

• Various JSDoc + type checking fixes #2987

[romaricpascal]

issue Same issue as before with the values of the object

Suggested change

	* @returns {Object<string, string>} Flattened object with dot-separated key namespace removed
	* @returns {Object<string, any>} Flattened object with dot-separated key namespace removed

[colinrotherham]

Fixed as above 👍

[romaricpascal]

question From what I gathered, this only changes the label of "this" in the first line when hovering this inside the function. Does it bring some technical info (eg. to TypeScript when type checking) or is it just informative when hovering/in JSDoc?

@class seems to already do the important lifting of typing this to an accordion and we have the header of the documentation to remind us of that if we drop it.

Completions also work OK without it:

[colinrotherham]

Yeah I think it came from a brief time I was trying to run in strict mode

Accordion() // this === window
new Accordion() // this === instanceof Accordion

Where inside our constructor function we'd technically need to add:

function Accordion ($module, config) {
  if (!(this instanceof Accordion)) {
    return new Accordion($module, config);
  }
}

☝️ Old trick to detect the missing new and returns an instance of itself

Shall I get rid? Won't need it once we switch to classes anyway

[romaricpascal]

I'd be keen to drop the @this then, yeah, to not carry its limited value 😄

(not sure if you were proposing to introduce the check for use as a function rather than a constructor, but given that'd expand our API to allow function right before we'd contract it back by switching to classes, I don't feel it's necessary either).

[colinrotherham]

Nah too much overhead. Compiler was right but we can live with it for now

I've removed @this from JSDoc and updated the coding standards to match

[romaricpascal]

Thanks for making the updates 😄 Looks good to go 🚀

[colinrotherham]

Removed as config objects from extractConfigByNamespace() have mixed values

(Not just strings)