govuk-frontend icon indicating copy to clipboard operation
govuk-frontend copied to clipboard

Some component JavaScript code is complex and can be difficult to follow

Open vanitabarrett opened this issue 2 years ago • 1 comments

Cause

Some of our component JavaScript is quite complex, for example: there's a lot of DOM manipulation and managing state. We've also iterated component JavaScript over time and in some cases added code to already complex functions, which has led to some functions doing multiple things.

Consequences

As a result, some of our JavaScript functions are difficult to follow and the naming of our functions can be misleading. This makes us less confident when debugging and making changes.

Impact of debt

Low

Reason (impact of debt)

No response

Effort to pay down

Medium

Reason (effort to pay down)

No response

Overall rating

Medium

Reason (overall rating)

This doesn't cause us frequent issues, but it reduces our confidence working on our JavaScript code. I've rated this as a Medium to pay down because it's tied in with adding unit testing/revisiting our testing strategy - ideally, we'd have smaller functions so that we can add unit tests for our JavaScript, but it would also be nice to have better tested JavaScript so we're confident doing our refactoring - a bit of a Catch-22.

There might also be some upskilling if the code is complex due to things like managing state. There may be some research to do into best practice here.

vanitabarrett avatar Jan 20 '22 14:01 vanitabarrett

Would using JSDoc (Wikipedia overview) to annotate our JS be useful? We already use SassDoc, which uses a similar syntax to JSDoc, for our Sass code, so this would seem like a natural companion.

I used JSDoc when writing up the i18n tech spike in addition to inline comments, though other people would need to tell me if this has actually helped them understand what's happening!

querkmachine avatar May 20 '22 14:05 querkmachine

Part of solving the issue could be to add type annotations to our code, which is tracked by #3230.

romaricpascal avatar Jan 31 '23 14:01 romaricpascal

We've made some good progress since this ticket was opened, including introducing JSDoc (as suggested by @querkmachine).

Next we want to take a proper look at the code with a view to refactoring it once we've moved to ES6 classes, which is covered by #2560.

On that basis I think we can close this.

36degrees avatar Apr 17 '23 16:04 36degrees