Enable type declaration guards #3123

colinrotherham · 2022-12-19T21:38:54Z

Add instanceof checks where necessary to unblock type declaration checks in:

Add (optional) type declaration checks in app, src #3104

colinrotherham · 2023-02-06T20:15:04Z

src/govuk/components/character-count/character-count.mjs

    return this
  }

  var $textarea = $module.querySelector('.govuk-js-character-count')
-  if (!$textarea) {
+  if (!($textarea instanceof HTMLTextAreaElement)) {


Do we want to allow text inputs?

I'd say yes, just as a safety to not break things if someone mounted the component on a markup with an <input> instead of <textarea>. I don't think we officially support it (given our template uses a textarea), but it does work.

I'm saying yes with the assumption that it's a matter of turning that if into if (!($textarea instanceof HTMLTextAreaElement || $textarea instanceof HTMLInputElement)), though. If it's more complex than that, I think we can stick to <textarea> and treat it in its own block of work for properly supporting input in the CharacterCount (both at JS and template level).

Kapture.2023-02-02.at.15.22.00.mp4

Yeah this is where it gets tricky

File upload, checkbox and radio inputs are also HTMLInputElement

But in terms of guarding the .value property your example works great 👍

These are the following places we've gone further than HTMLElement:

Character count needs <textarea> or <input>
lint-types ./src/govuk/components/character-count/character-count.mjs#L65

Checkboxes need <input> with type="checkbox"
lint-types ./src/govuk/components/checkboxes/checkboxes.mjs#L179

Radios need <input> with type="radio"
lint-types ./src/govuk/components/radios/radios.mjs#L129

Details needs <details> to avoid polyfill
lint-types ./src/govuk/components/details/details.mjs#L40

Error summary needs <a> for links and <input> (checkbox/radio) for legend .scrollIntoView()
lint-types ./src/govuk/components/error-summary/error-summary.mjs#L114
lint-types ./src/govuk/components/error-summary/error-summary.mjs#L185

Skip link needs <a>
lint-types ./src/govuk/components/skip-link/skip-link.mjs#L14

Tabs need <a>
lint-types ./src/govuk/components/tabs/tabs.mjs#L270

romaricpascal

I'd say we're good to go on that one, as I don't think any of these changes would be breaking. @36degrees I would appreciate a second look, though, in case we missed a potential breaking case when going through the changes with Colin.

Review component JSDoc types against module and element checks We now prefer `{Element}` type to `{HTMLElement}` to maintain compatibility with `querySelector` but we continue to use `instanceof` to guard types Co-Authored-By: Romaric <romaric.pascal@digital.cabinet-office.gov.uk>

Where empty objects are initially assigned (or merged) the compiler needs to know the final “shape” up front In editors and IDEs this also helps populate tooltip messages during loops etc

colinrotherham added javascript tech debt Frontend squad labels Dec 19, 2022

colinrotherham linked an issue Dec 19, 2022 that may be closed by this pull request

Some component JavaScript is complex and difficult to follow #2513

Closed

govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3123 December 19, 2022 21:39 Inactive

This was referenced Dec 19, 2022

Various JSDoc + type checking fixes #2987

Merged

Configure ESLint to require JSDoc @params etc #3103

Merged

Add (optional) type declaration checks in app, src #3104

Merged

Ensure JSDoc blocks are complete #3102

Merged

colinrotherham force-pushed the fix-types branch from 1477fce to ab7b33d Compare January 9, 2023 21:41

colinrotherham force-pushed the fix-types-instanceof branch from 2992838 to cd85627 Compare January 9, 2023 21:41

colinrotherham force-pushed the fix-types branch from ab7b33d to 09c6ab5 Compare January 11, 2023 15:23

colinrotherham force-pushed the fix-types-instanceof branch from cd85627 to 39f7ac4 Compare January 11, 2023 15:23

colinrotherham force-pushed the fix-types branch from 09c6ab5 to 66911f4 Compare January 11, 2023 19:50

colinrotherham force-pushed the fix-types-instanceof branch from 39f7ac4 to 0988f2c Compare January 11, 2023 19:50

colinrotherham force-pushed the fix-types branch from 66911f4 to 4596100 Compare January 11, 2023 21:03

colinrotherham force-pushed the fix-types-instanceof branch from 0988f2c to d9dea28 Compare January 11, 2023 21:05

colinrotherham force-pushed the fix-types branch from 4596100 to 11224b3 Compare January 16, 2023 14:33

colinrotherham force-pushed the fix-types-instanceof branch from d9dea28 to 455a866 Compare January 16, 2023 14:33

govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3123 January 16, 2023 16:08 Inactive

colinrotherham force-pushed the fix-types branch from 11224b3 to 111ce51 Compare January 18, 2023 15:53

colinrotherham force-pushed the fix-types-instanceof branch from 455a866 to 23abf6b Compare January 18, 2023 15:53

govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3123 January 18, 2023 15:54 Inactive

colinrotherham force-pushed the fix-types branch from 111ce51 to e992320 Compare January 18, 2023 16:45

colinrotherham force-pushed the fix-types-instanceof branch from 23abf6b to 39b22e6 Compare January 18, 2023 16:46

govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3123 January 18, 2023 16:46 Inactive

colinrotherham force-pushed the fix-types branch from e992320 to 7b328b9 Compare January 19, 2023 10:28

colinrotherham force-pushed the fix-types-instanceof branch from 39b22e6 to 349545f Compare January 19, 2023 10:28

romaricpascal linked an issue Jan 31, 2023 that may be closed by this pull request

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

Open

11 tasks

romaricpascal removed a link to an issue Jan 31, 2023

Some component JavaScript is complex and difficult to follow #2513

Closed

colinrotherham force-pushed the fix-types branch from dde0f63 to dbf3b90 Compare February 2, 2023 12:10

colinrotherham force-pushed the fix-types-instanceof branch from c233445 to 68464b9 Compare February 2, 2023 12:10

govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3123 February 2, 2023 12:10 Inactive

kellylee-gds removed the Frontend squad label Feb 2, 2023

colinrotherham force-pushed the fix-types-instanceof branch from 68464b9 to ac7c1c3 Compare February 3, 2023 08:13

govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3123 February 3, 2023 08:14 Inactive

colinrotherham force-pushed the fix-types branch from dbf3b90 to 0718c67 Compare February 6, 2023 17:19

colinrotherham force-pushed the fix-types-instanceof branch from ac7c1c3 to 70bcc52 Compare February 6, 2023 17:20

govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3123 February 6, 2023 17:20 Inactive

Base automatically changed from fix-types to main February 6, 2023 17:25

colinrotherham force-pushed the fix-types-instanceof branch from 70bcc52 to da882a1 Compare February 6, 2023 17:45

govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3123 February 6, 2023 17:45 Inactive

colinrotherham commented Feb 6, 2023

View reviewed changes

colinrotherham marked this pull request as ready for review February 6, 2023 20:15

colinrotherham force-pushed the fix-types-instanceof branch from da882a1 to 11d1f13 Compare February 8, 2023 11:08

govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3123 February 8, 2023 11:08 Inactive

romaricpascal approved these changes Feb 9, 2023

View reviewed changes

36degrees approved these changes Feb 9, 2023

View reviewed changes

colinrotherham and others added 3 commits February 10, 2023 09:25

Improve JSDoc checks with inline @type

0d8c0d6

Where empty objects are initially assigned (or merged) the compiler needs to know the final “shape” up front In editors and IDEs this also helps populate tooltip messages during loops etc

Qualify query selectors to infer HTMLAnchorElement type

5c134f9

colinrotherham force-pushed the fix-types-instanceof branch from 11d1f13 to 5c134f9 Compare February 10, 2023 09:25

govuk-design-system-ci temporarily deployed to govuk-frontend-pr-3123 February 10, 2023 09:25 Inactive

colinrotherham merged commit 916e3d9 into main Feb 10, 2023

colinrotherham deleted the fix-types-instanceof branch February 10, 2023 10:06

colinrotherham added assurance consistency and removed tech debt labels Feb 15, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable type declaration guards #3123

Enable type declaration guards #3123

colinrotherham commented Dec 19, 2022 •

edited by romaricpascal

Loading

colinrotherham Feb 6, 2023

romaricpascal Feb 8, 2023

colinrotherham Feb 8, 2023

colinrotherham Feb 8, 2023

romaricpascal left a comment

Enable type declaration guards #3123

Enable type declaration guards #3123

Conversation

colinrotherham commented Dec 19, 2022 • edited by romaricpascal Loading

colinrotherham Feb 6, 2023

Choose a reason for hiding this comment

romaricpascal Feb 8, 2023

Choose a reason for hiding this comment

colinrotherham Feb 8, 2023

Choose a reason for hiding this comment

colinrotherham Feb 8, 2023

Choose a reason for hiding this comment

romaricpascal left a comment

Choose a reason for hiding this comment

colinrotherham commented Dec 19, 2022 •

edited by romaricpascal

Loading