alphagov / govuk-frontend

GOV.UK Frontend contains the code you need to start building a user interface for government platforms and services.
https://frontend.design-system.service.gov.uk/
MIT License
1.17k stars 320 forks source link

Add `onError` to `createAll` #5252

Closed patrickpatrickpatrick closed 1 month ago

patrickpatrickpatrick commented 1 month ago

What

Why

Provide better support for defining custom components by allowing users to execute custom callback if the components fail to initialise.

Fixes #5212

github-actions[bot] commented 1 month ago

:clipboard: Stats

File sizes

File Size
dist/govuk-frontend-development.min.css 112.61 KiB
dist/govuk-frontend-development.min.js 42.12 KiB
packages/govuk-frontend/dist/govuk/all.bundle.js 89.55 KiB
packages/govuk-frontend/dist/govuk/all.bundle.mjs 84.1 KiB
packages/govuk-frontend/dist/govuk/all.mjs 1.01 KiB
packages/govuk-frontend/dist/govuk/govuk-frontend-component.mjs 359 B
packages/govuk-frontend/dist/govuk/govuk-frontend.min.css 112.6 KiB
packages/govuk-frontend/dist/govuk/govuk-frontend.min.js 42.11 KiB
packages/govuk-frontend/dist/govuk/i18n.mjs 5.55 KiB
packages/govuk-frontend/dist/govuk/init.mjs 6.59 KiB

Modules

File Size (bundled) Size (minified)
all.mjs 80.49 KiB 40.06 KiB
accordion.mjs 23.83 KiB 12.39 KiB
button.mjs 6.31 KiB 2.69 KiB
character-count.mjs 22.73 KiB 9.92 KiB
checkboxes.mjs 6.16 KiB 2.83 KiB
error-summary.mjs 8.22 KiB 3.46 KiB
exit-this-page.mjs 17.43 KiB 9.26 KiB
header.mjs 4.79 KiB 2.6 KiB
notification-banner.mjs 6.59 KiB 2.62 KiB
password-input.mjs 15.48 KiB 7.25 KiB
radios.mjs 5.16 KiB 2.38 KiB
skip-link.mjs 4.72 KiB 2.18 KiB
tabs.mjs 10.38 KiB 6.06 KiB

View stats and visualisations on the review app


Action run for b771200a337e0a7646130f281bb24947176e53ec

github-actions[bot] commented 1 month ago

JavaScript changes to npm package

diff --git a/packages/govuk-frontend/dist/govuk/govuk-frontend.min.js b/packages/govuk-frontend/dist/govuk/govuk-frontend.min.js
index 69492a902..dd7574fa1 100644
--- a/packages/govuk-frontend/dist/govuk/govuk-frontend.min.js
+++ b/packages/govuk-frontend/dist/govuk/govuk-frontend.min.js
@@ -1104,15 +1104,26 @@ function initAll(e) {
     }))
 }

-function createAll(e, t, n = document) {
-    const i = n.querySelectorAll(`[data-module="${e.moduleName}"]`);
-    return isSupported() ? Array.from(i).map((n => {
+function createAll(e, t, n) {
+    let i, s = document;
+    var o;
+    "object" == typeof n && (s = null != (o = n.scope) ? o : s, i = n.onError);
+    "function" == typeof n && (i = n), n instanceof HTMLElement && (s = n);
+    const r = s.querySelectorAll(`[data-module="${e.moduleName}"]`);
+    return isSupported() ? Array.from(r).map((n => {
         try {
             return "defaults" in e && void 0 !== t ? new e(n, t) : new e(n)
-        } catch (i) {
-            return console.log(i), null
+        } catch (s) {
+            return i && s instanceof Error ? i(s, {
+                element: n,
+                component: e,
+                config: t
+            }) : console.log(s), null
         }
-    })).filter(Boolean) : (console.log(new SupportError), [])
+    })).filter(Boolean) : (i ? i(new SupportError, {
+        component: e,
+        config: t
+    }) : console.log(new SupportError), [])
 }
 Tabs.moduleName = "govuk-tabs";
 export {

Action run for b771200a337e0a7646130f281bb24947176e53ec

github-actions[bot] commented 1 month ago

Other changes to npm package

diff --git a/packages/govuk-frontend/dist/govuk/all.bundle.js b/packages/govuk-frontend/dist/govuk/all.bundle.js
index 3b86ffe36..057ec0db2 100644
--- a/packages/govuk-frontend/dist/govuk/all.bundle.js
+++ b/packages/govuk-frontend/dist/govuk/all.bundle.js
@@ -2383,21 +2383,50 @@
    *
    * @template {CompatibleClass} T
    * @param {T} Component - class of the component to create
-   * @param {T["defaults"]} [config] - config for the component
-   * @param {Element|Document} [$scope] - scope of the document to search within
+   * @param {T["defaults"]} [config] - Config supplied to component
+   * @param {OnErrorCallback<T> | Element | Document | CreateAllOptions<T> } [createAllOptions] - options for createAll including scope of the document to search within and callback function if error throw by component on init
    * @returns {Array<InstanceType<T>>} - array of instantiated components
    */
-  function createAll(Component, config, $scope = document) {
+  function createAll(Component, config, createAllOptions) {
+    let $scope = document;
+    let onError;
+    if (typeof createAllOptions === 'object') {
+      var _createAllOptions$sco;
+      createAllOptions = createAllOptions;
+      $scope = (_createAllOptions$sco = createAllOptions.scope) != null ? _createAllOptions$sco : $scope;
+      onError = createAllOptions.onError;
+    }
+    if (typeof createAllOptions === 'function') {
+      onError = createAllOptions;
+    }
+    if (createAllOptions instanceof HTMLElement) {
+      $scope = createAllOptions;
+    }
     const $elements = $scope.querySelectorAll(`[data-module="${Component.moduleName}"]`);
     if (!isSupported()) {
-      console.log(new SupportError());
+      if (onError) {
+        onError(new SupportError(), {
+          component: Component,
+          config
+        });
+      } else {
+        console.log(new SupportError());
+      }
       return [];
     }
     return Array.from($elements).map($element => {
       try {
         return 'defaults' in Component && typeof config !== 'undefined' ? new Component($element, config) : new Component($element);
       } catch (error) {
-        console.log(error);
+        if (onError && error instanceof Error) {
+          onError(error, {
+            element: $element,
+            component: Component,
+            config
+          });
+        } else {
+          console.log(error);
+        }
         return null;
       }
     }).filter(Boolean);
@@ -2436,6 +2465,25 @@
    *
    * @typedef {keyof Config} ConfigKey
    */
+  /**
+   * @template {CompatibleClass} T
+   * @typedef {object} ErrorContext
+   * @property {Element} [element] - Element used for component module initialisation
+   * @property {T} component - Class of component
+   * @property {T["defaults"]} config - Config supplied to component
+   */
+  /**
+   * @template {CompatibleClass} T
+   * @callback OnErrorCallback
+   * @param {Error} error - Thrown error
+   * @param {ErrorContext<T>} context - Object containing the element, component class and configuration
+   */
+  /**
+   * @template {CompatibleClass} T
+   * @typedef {object} CreateAllOptions
+   * @property {Element | Document} [scope] - scope of the document to search within
+   * @property {OnErrorCallback<T>} [onError] - callback function if error throw by component on init
+   */

   exports.Accordion = Accordion;
   exports.Button = Button;
diff --git a/packages/govuk-frontend/dist/govuk/all.bundle.mjs b/packages/govuk-frontend/dist/govuk/all.bundle.mjs
index 8725b363d..15a7feada 100644
--- a/packages/govuk-frontend/dist/govuk/all.bundle.mjs
+++ b/packages/govuk-frontend/dist/govuk/all.bundle.mjs
@@ -2377,21 +2377,50 @@ function initAll(config) {
  *
  * @template {CompatibleClass} T
  * @param {T} Component - class of the component to create
- * @param {T["defaults"]} [config] - config for the component
- * @param {Element|Document} [$scope] - scope of the document to search within
+ * @param {T["defaults"]} [config] - Config supplied to component
+ * @param {OnErrorCallback<T> | Element | Document | CreateAllOptions<T> } [createAllOptions] - options for createAll including scope of the document to search within and callback function if error throw by component on init
  * @returns {Array<InstanceType<T>>} - array of instantiated components
  */
-function createAll(Component, config, $scope = document) {
+function createAll(Component, config, createAllOptions) {
+  let $scope = document;
+  let onError;
+  if (typeof createAllOptions === 'object') {
+    var _createAllOptions$sco;
+    createAllOptions = createAllOptions;
+    $scope = (_createAllOptions$sco = createAllOptions.scope) != null ? _createAllOptions$sco : $scope;
+    onError = createAllOptions.onError;
+  }
+  if (typeof createAllOptions === 'function') {
+    onError = createAllOptions;
+  }
+  if (createAllOptions instanceof HTMLElement) {
+    $scope = createAllOptions;
+  }
   const $elements = $scope.querySelectorAll(`[data-module="${Component.moduleName}"]`);
   if (!isSupported()) {
-    console.log(new SupportError());
+    if (onError) {
+      onError(new SupportError(), {
+        component: Component,
+        config
+      });
+    } else {
+      console.log(new SupportError());
+    }
     return [];
   }
   return Array.from($elements).map($element => {
     try {
       return 'defaults' in Component && typeof config !== 'undefined' ? new Component($element, config) : new Component($element);
     } catch (error) {
-      console.log(error);
+      if (onError && error instanceof Error) {
+        onError(error, {
+          element: $element,
+          component: Component,
+          config
+        });
+      } else {
+        console.log(error);
+      }
       return null;
     }
   }).filter(Boolean);
@@ -2430,6 +2459,25 @@ function createAll(Component, config, $scope = document) {
  *
  * @typedef {keyof Config} ConfigKey
  */
+/**
+ * @template {CompatibleClass} T
+ * @typedef {object} ErrorContext
+ * @property {Element} [element] - Element used for component module initialisation
+ * @property {T} component - Class of component
+ * @property {T["defaults"]} config - Config supplied to component
+ */
+/**
+ * @template {CompatibleClass} T
+ * @callback OnErrorCallback
+ * @param {Error} error - Thrown error
+ * @param {ErrorContext<T>} context - Object containing the element, component class and configuration
+ */
+/**
+ * @template {CompatibleClass} T
+ * @typedef {object} CreateAllOptions
+ * @property {Element | Document} [scope] - scope of the document to search within
+ * @property {OnErrorCallback<T>} [onError] - callback function if error throw by component on init
+ */

 export { Accordion, Button, CharacterCount, Checkboxes, ErrorSummary, ExitThisPage, Header, NotificationBanner, PasswordInput, Radios, SkipLink, Tabs, createAll, initAll, isSupported, version };
 //# sourceMappingURL=all.bundle.mjs.map
diff --git a/packages/govuk-frontend/dist/govuk/init.mjs b/packages/govuk-frontend/dist/govuk/init.mjs
index c8b4ab490..2de2329d5 100644
--- a/packages/govuk-frontend/dist/govuk/init.mjs
+++ b/packages/govuk-frontend/dist/govuk/init.mjs
@@ -46,21 +46,50 @@ function initAll(config) {
  *
  * @template {CompatibleClass} T
  * @param {T} Component - class of the component to create
- * @param {T["defaults"]} [config] - config for the component
- * @param {Element|Document} [$scope] - scope of the document to search within
+ * @param {T["defaults"]} [config] - Config supplied to component
+ * @param {OnErrorCallback<T> | Element | Document | CreateAllOptions<T> } [createAllOptions] - options for createAll including scope of the document to search within and callback function if error throw by component on init
  * @returns {Array<InstanceType<T>>} - array of instantiated components
  */
-function createAll(Component, config, $scope = document) {
+function createAll(Component, config, createAllOptions) {
+  let $scope = document;
+  let onError;
+  if (typeof createAllOptions === 'object') {
+    var _createAllOptions$sco;
+    createAllOptions = createAllOptions;
+    $scope = (_createAllOptions$sco = createAllOptions.scope) != null ? _createAllOptions$sco : $scope;
+    onError = createAllOptions.onError;
+  }
+  if (typeof createAllOptions === 'function') {
+    onError = createAllOptions;
+  }
+  if (createAllOptions instanceof HTMLElement) {
+    $scope = createAllOptions;
+  }
   const $elements = $scope.querySelectorAll(`[data-module="${Component.moduleName}"]`);
   if (!isSupported()) {
-    console.log(new SupportError());
+    if (onError) {
+      onError(new SupportError(), {
+        component: Component,
+        config
+      });
+    } else {
+      console.log(new SupportError());
+    }
     return [];
   }
   return Array.from($elements).map($element => {
     try {
       return 'defaults' in Component && typeof config !== 'undefined' ? new Component($element, config) : new Component($element);
     } catch (error) {
-      console.log(error);
+      if (onError && error instanceof Error) {
+        onError(error, {
+          element: $element,
+          component: Component,
+          config
+        });
+      } else {
+        console.log(error);
+      }
       return null;
     }
   }).filter(Boolean);
@@ -99,6 +128,25 @@ function createAll(Component, config, $scope = document) {
  *
  * @typedef {keyof Config} ConfigKey
  */
+/**
+ * @template {CompatibleClass} T
+ * @typedef {object} ErrorContext
+ * @property {Element} [element] - Element used for component module initialisation
+ * @property {T} component - Class of component
+ * @property {T["defaults"]} config - Config supplied to component
+ */
+/**
+ * @template {CompatibleClass} T
+ * @callback OnErrorCallback
+ * @param {Error} error - Thrown error
+ * @param {ErrorContext<T>} context - Object containing the element, component class and configuration
+ */
+/**
+ * @template {CompatibleClass} T
+ * @typedef {object} CreateAllOptions
+ * @property {Element | Document} [scope] - scope of the document to search within
+ * @property {OnErrorCallback<T>} [onError] - callback function if error throw by component on init
+ */

 export { createAll, initAll };
 //# sourceMappingURL=init.mjs.map

Action run for b771200a337e0a7646130f281bb24947176e53ec

romaricpascal commented 1 month ago

@patrickpatrickpatrick I've changed the PR base to public-js-api on GitHub so we don't merge on main accidentally, but I think it'll need a little rebase to get the check for isSupported inside createAll.

What are your thoughts on whether the check should also trigger a call to onError, as well? For consistency, it feels like we should trigger a call as well, and if people initialise multiple components with multiple createAll calls likely receiving the same error handler, they can use isSupported to not call createAll and avoid the noise 🤔

patrickpatrickpatrick commented 1 month ago

@patrickpatrickpatrick I've changed the PR base to public-js-api on GitHub so we don't merge on main accidentally, but I think it'll need a little rebase to get the check for isSupported inside createAll.

What are your thoughts on whether the check should also trigger a call to onError, as well? For consistency, it feels like we should trigger a call as well, and if people initialise multiple components with multiple createAll calls likely receiving the same error handler, they can use isSupported to not call createAll and avoid the noise 🤔

That makes sense, have added this!