Update coding standards with JSDoc changes etc

Also includes:

1. Checks for `$module` on instantiation
2. Checks for selectors on init

We prefer `{Element}` type to `{HTMLElement}` etc to maintain compatibility with `querySelector`

Author: colinrotherham

docs/contributing/coding-standards/js.md

@@ -5,25 +5,45 @@
 JavaScript files have the same name as the component's folder name. Test files have a `.test` suffix placed before the file extension.
 
 ```
-checkboxes
-├── checkboxes.mjs
-└── checkboxes.test.js
+component
+├── component.mjs
+└── component.test.js
 ```
 
 ## Skeleton
 
 ```js
-import { nodeListForEach } from '../vendor/common.mjs'
+import '../../vendor/polyfills/Element.mjs'
 
-function Checkboxes ($module) {
-  // code goes here
+/**
+ * Component name
+ *
+ * @class
+ * @param {Element} $module - HTML element to use for component
+ */
+function Example ($module) {
+  if (!$module) {
+    return this
+  }
+
+  this.$module = $module
+
+  // Code goes here
 }
 
-Checkboxes.prototype.init = function () {
-  // code goes here
+/**
+ * Initialise component
+ */
+Example.prototype.init = function () {
+  // Check that required elements are present
+  if (!this.$module) {
+    return
+  }
+
+  // Code goes here
 }
 
-export default Checkboxes
+export default Example
 ```
 
 ## Use data attributes to initialise component JavaScript
@@ -48,15 +68,15 @@ Use `/** ... */` for multi-line comments. Include a description, and specify typ
 
 ```js
 /**
-* Get the nearest ancestor element of a node that matches a given tag name
-* @param {object} node element
-* @param {string} match tag name (e.g. div)
-* @return {object} ancestor element
-*/
-
-function (node, match) {
-  // code goes here
-  return ancestor
+ * Get the first descendent (child) of an HTML element that matches a given tag name
+ *
+ * @param {Element} $element - HTML element
+ * @param {string} tagName - Tag name (for example 'div')
+ * @returns {Element} Ancestor element
+ */
+function ($element, tagName) {
+  // Code goes here
+  return $element.querySelector(tagName)
 }
 ```
 
@@ -73,52 +93,54 @@ Use the prototype design pattern to structure your code.
 Create a constructor and define any variables that the object needs.
 
 ```js
-function Checkboxes ($module) {
-  // code goes here
+function Example ($module) {
+  // Code goes here
 }
 ```
 
 Assign methods to the prototype object. Do not overwrite the prototype with a new object as this makes inheritance impossible.
 
 ```js
-// bad
-Checkboxes.prototype = {
+// Bad
+Example.prototype = {
   init: function () {
-    // code goes here
+    // Code goes here
   }
 }
 
-// good
-Checkboxes.prototype.init = function () {
-  // code goes here
+// Good
+Example.prototype.init = function () {
+  // Code goes here
 }
 ```
 
 When initialising an object, use the `new` keyword.
 
 ```js
-// bad
-var myCheckbox = Checkbox().init()
+// Bad
+var myExample = Example()
 
-// good
-var myCheckbox = new Checkbox().init()
+// Good
+var myExample = new Example()
 ```
 
 ## Modules
 
-Use ES6 modules (`import`/`export`) over a non-standard module system. You can always transpile to your preferred module system.
+Use ECMAScript modules (`import`/`export`) over CommonJS and other formats. You can always transpile to your preferred module system.
 
 ```js
-import { nodeListForEach } from '../vendor/common.mjs'
-// code goes here
-export default Checkboxes
+import { closestAttributeValue } from '../common/index.mjs'
+
+// Code goes here
+export function exampleHelper1 () {}
+export function exampleHelper2 () {}
 ```
 
-Avoid using wildcard (`import * as nodeListForEach`) imports.
+You must specify the file extension when using the import keyword.
 
-You must specify the file extension for a file when importing it.
+Avoid using namespace imports (`import * as namespace`) in code transpiled to CommonJS (or AMD) bundled code as this can prevent "tree shaking" optimisations.
 
-Use default export over named export.
+Prefer named exports over default exports to avoid compatibility issues with transpiler "synthetic default" as discussed in: https://github.com/alphagov/govuk-frontend/issues/2829
 
 ## Polyfilling
 

src/govuk/all.mjs

@@ -104,7 +104,7 @@ export {
  * Config for all components
  *
  * @typedef {object} Config
- * @property {HTMLElement} [scope=document] - Scope to query for components
+ * @property {Element} [scope=document] - Scope to query for components
  * @property {import('./components/accordion/accordion.mjs').AccordionConfig} [accordion] - Accordion config
  * @property {import('./components/button/button.mjs').ButtonConfig} [button] - Button config
  * @property {import('./components/character-count/character-count.mjs').CharacterCountConfig} [characterCount] - Character Count config

src/govuk/common/closest-attribute-value.mjs

@@ -3,7 +3,7 @@ import '../vendor/polyfills/Element/prototype/closest.mjs'
 /**
  * Returns the value of the given attribute closest to the given element (including itself)
  *
- * @param {HTMLElement} $element - The element to start walking the DOM tree up
+ * @param {Element} $element - The element to start walking the DOM tree up
  * @param {string} attributeName - The name of the attribute
  * @returns {string | null} Attribute value
  */

src/govuk/common/index.mjs

@@ -135,10 +135,13 @@ export function extractConfigByNamespace (configObject, namespace) {
   if (!configObject || typeof configObject !== 'object') {
     throw new Error('Provide a `configObject` of type "object".')
   }
+
   if (!namespace || typeof namespace !== 'string') {
     throw new Error('Provide a `namespace` of type "string" to filter the `configObject` by.')
   }
+
   var newObject = {}
+
   for (var key in configObject) {
     // Split the key into parts, using . as our namespace separator
     var keyParts = key.split('.')

src/govuk/components/accordion/accordion.mjs

@@ -38,10 +38,14 @@ var ACCORDION_TRANSLATIONS = {
  * attribute, which also provides accessibility.
  *
  * @class
- * @param {HTMLElement} $module - HTML element to use for accordion
+ * @param {Element} $module - HTML element to use for accordion
  * @param {AccordionConfig} [config] - Accordion config
  */
 function Accordion ($module, config) {
+  if (!$module) {
+    return this
+  }
+
   this.$module = $module
 
   var defaultConfig = {
@@ -79,16 +83,21 @@ function Accordion ($module, config) {
   this.sectionSummaryFocusClass = 'govuk-accordion__section-summary-focus'
   this.sectionContentClass = 'govuk-accordion__section-content'
 
-  this.$sections = this.$module.querySelectorAll('.' + this.sectionClass)
+  var $sections = this.$module.querySelectorAll('.' + this.sectionClass)
+  if (!$sections.length) {
+    return this
+  }
+
+  this.$sections = $sections
   this.browserSupportsSessionStorage = helper.checkForSessionStorage()
 }
 
 /**
  * Initialise component
  */
 Accordion.prototype.init = function () {
-  // Check for module
-  if (!this.$module) {
+  // Check that required elements are present
+  if (!this.$module || !this.$sections) {
     return
   }
 
@@ -145,6 +154,9 @@ Accordion.prototype.initSectionHeaders = function () {
   // Loop through sections
   nodeListForEach($sections, function ($section, i) {
     var $header = $section.querySelector('.' + $component.sectionHeaderClass)
+    if (!$header) {
+      return
+    }
 
     // Set header attributes
     $component.constructHeaderMarkup($header, i)
@@ -162,14 +174,18 @@ Accordion.prototype.initSectionHeaders = function () {
 /**
  * Construct section header
  *
- * @param {HTMLElement} $header - Section header
+ * @param {Element} $header - Section header
  * @param {number} index - Section index
  */
 Accordion.prototype.constructHeaderMarkup = function ($header, index) {
   var $span = $header.querySelector('.' + this.sectionButtonClass)
   var $heading = $header.querySelector('.' + this.sectionHeadingClass)
   var $summary = $header.querySelector('.' + this.sectionSummaryClass)
 
+  if (!$span || !$heading) {
+    return
+  }
+
   // Create a button element that will replace the '.govuk-accordion__section-button' span
   var $button = document.createElement('button')
   $button.setAttribute('type', 'button')
@@ -227,7 +243,7 @@ Accordion.prototype.constructHeaderMarkup = function ($header, index) {
   $button.appendChild(this.getButtonPunctuationEl())
 
   // If summary content exists add to DOM in correct order
-  if (typeof ($summary) !== 'undefined' && $summary !== null) {
+  if ($summary) {
     // Create a new `span` element and copy the summary line content from the original `div` to the
     // new `span`
     // This is because the summary line text is now inside a button element, which can only contain
@@ -267,7 +283,13 @@ Accordion.prototype.constructHeaderMarkup = function ($header, index) {
  * @param {Event} event - Generic event
  */
 Accordion.prototype.onBeforeMatch = function (event) {
-  var $section = event.target.closest('.' + this.sectionClass)
+  var $fragment = event.target
+  if (!$fragment) {
+    return
+  }
+
+  // Handle when fragment is inside section
+  var $section = $fragment.closest('.' + this.sectionClass)
   if ($section) {
     this.setExpanded(true, $section)
   }
@@ -276,7 +298,7 @@ Accordion.prototype.onBeforeMatch = function (event) {
 /**
  * When section toggled, set and store state
  *
- * @param {HTMLElement} $section - Section element
+ * @param {Element} $section - Section element
  */
 Accordion.prototype.onSectionToggle = function ($section) {
   var expanded = this.isExpanded($section)
@@ -309,14 +331,21 @@ Accordion.prototype.onShowOrHideAllToggle = function () {
  * Set section attributes when opened/closed
  *
  * @param {boolean} expanded - Section expanded
- * @param {HTMLElement} $section - Section element
+ * @param {Element} $section - Section element
  */
 Accordion.prototype.setExpanded = function (expanded, $section) {
   var $showHideIcon = $section.querySelector('.' + this.upChevronIconClass)
   var $showHideText = $section.querySelector('.' + this.sectionShowHideTextClass)
   var $button = $section.querySelector('.' + this.sectionButtonClass)
   var $content = $section.querySelector('.' + this.sectionContentClass)
 
+  if (!$showHideIcon ||
+    !$showHideText ||
+    !$button ||
+    !$content) {
+    return
+  }
+
   var newButtonText = expanded
     ? this.i18n.t('hideSection')
     : this.i18n.t('showSection')
@@ -368,7 +397,7 @@ Accordion.prototype.setExpanded = function (expanded, $section) {
 /**
  * Get state of section
  *
- * @param {HTMLElement} $section - Section element
+ * @param {Element} $section - Section element
  * @returns {boolean} True if expanded
  */
 Accordion.prototype.isExpanded = function ($section) {
@@ -434,7 +463,7 @@ var helper = {
 /**
  * Set the state of the accordions in sessionStorage
  *
- * @param {HTMLElement} $section - Section element
+ * @param {Element} $section - Section element
  */
 Accordion.prototype.storeState = function ($section) {
   if (this.browserSupportsSessionStorage) {
@@ -458,7 +487,7 @@ Accordion.prototype.storeState = function ($section) {
 /**
  * Read the state of the accordions from sessionStorage
  *
- * @param {HTMLElement} $section - Section element
+ * @param {Element} $section - Section element
  */
 Accordion.prototype.setInitialState = function ($section) {
   if (this.browserSupportsSessionStorage) {
@@ -482,7 +511,7 @@ Accordion.prototype.setInitialState = function ($section) {
  * into thematic chunks.
  * See https://github.com/alphagov/govuk-frontend/issues/2327#issuecomment-922957442
  *
- * @returns {HTMLElement} DOM element
+ * @returns {Element} DOM element
  */
 Accordion.prototype.getButtonPunctuationEl = function () {
   var $punctuationEl = document.createElement('span')

src/govuk/components/button/button.mjs

@@ -12,7 +12,7 @@ var DEBOUNCE_TIMEOUT_IN_SECONDS = 1
  * JavaScript enhancements for the Button component
  *
  * @class
- * @param {HTMLElement} $module - HTML element to use for button
+ * @param {Element} $module - HTML element to use for button
  * @param {ButtonConfig} [config] - Button config
  */
 function Button ($module, config) {
@@ -26,6 +26,7 @@ function Button ($module, config) {
   var defaultConfig = {
     preventDoubleClick: false
   }
+
   this.config = mergeConfigs(
     defaultConfig,
     config || {},
@@ -37,6 +38,7 @@ function Button ($module, config) {
  * Initialise component
  */
 Button.prototype.init = function () {
+  // Check that required elements are present
   if (!this.$module) {
     return
   }
@@ -58,7 +60,13 @@ Button.prototype.init = function () {
 Button.prototype.handleKeyDown = function (event) {
   var $target = event.target
 
-  if ($target.getAttribute('role') === 'button' && event.keyCode === KEY_SPACE) {
+  // Handle space bar only
+  if (event.keyCode !== KEY_SPACE) {
+    return
+  }
+
+  // Handle elements with [role="button"] only
+  if ($target.getAttribute('role') === 'button') {
     event.preventDefault() // prevent the page from scrolling
     $target.click()
   }