govuk-frontend
govuk-frontend copied to clipboard
Some component JavaScript code is complex and can be difficult to follow
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.
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!
Part of solving the issue could be to add type annotations to our code, which is tracked by #3230.
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.