Add `onError` to `initAll` config #5276

patrickpatrickpatrick · 2024-08-29T16:12:29Z

What

config parameter of initAll now accepts onError callback
initAll tests updated for onError callback

Why

createAll now supports onError callback, so makes sense for initAll to be able to take onError as a parameter to be passed into createAll and also when initAll errors itself.

Fixes #5267

github-actions · 2024-08-29T16:14:05Z

📋 Stats

File sizes

File	Size
dist/govuk-frontend-development.min.css	112.61 KiB
dist/govuk-frontend-development.min.js	42.2 KiB
packages/govuk-frontend/dist/govuk/all.bundle.js	89.77 KiB
packages/govuk-frontend/dist/govuk/all.bundle.mjs	84.31 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.18 KiB
packages/govuk-frontend/dist/govuk/i18n.mjs	5.55 KiB
packages/govuk-frontend/dist/govuk/init.mjs	6.79 KiB

Modules

File	Size (bundled)	Size (minified)
all.mjs	80.69 KiB	40.12 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 5983f3a

github-actions · 2024-08-29T16:14:28Z

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 dd7574fa1..2ae88f3c2 100644
--- a/packages/govuk-frontend/dist/govuk/govuk-frontend.min.js
+++ b/packages/govuk-frontend/dist/govuk/govuk-frontend.min.js
@@ -1083,7 +1083,9 @@ class Tabs extends GOVUKFrontendComponent {
 
 function initAll(e) {
     var t;
-    if (e = void 0 !== e ? e : {}, !isSupported()) return void console.log(new SupportError);
+    if (e = void 0 !== e ? e : {}, !isSupported()) return void(e.onError ? e.onError(new SupportError, {
+        config: e
+    }) : console.log(new SupportError));
     const n = [
             [Accordion, e.accordion],
             [Button, e.button],
@@ -1098,7 +1100,10 @@ function initAll(e) {
             [SkipLink],
             [Tabs]
         ],
-        i = null != (t = e.scope) ? t : document;
+        i = {
+            scope: null != (t = e.scope) ? t : document,
+            onError: e.onError
+        };
     n.forEach((([e, t]) => {
         createAll(e, t, i)
     }))

Action run for 5983f3a

github-actions · 2024-08-29T16:14:29Z

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 057ec0db2..d5d901b55 100644
--- a/packages/govuk-frontend/dist/govuk/all.bundle.js
+++ b/packages/govuk-frontend/dist/govuk/all.bundle.js
@@ -2356,19 +2356,28 @@
    * Use the `data-module` attributes to find, instantiate and init all of the
    * components provided as part of GOV.UK Frontend.
    *
-   * @param {Config & { scope?: Element }} [config] - Config for all components (with optional scope)
+   * @param {Config & { scope?: Element, onError?: OnErrorCallback<CompatibleClass> }} [config] - Config for all components (with optional scope)
    */
   function initAll(config) {
     var _config$scope;
     config = typeof config !== 'undefined' ? config : {};
     if (!isSupported()) {
-      console.log(new SupportError());
+      if (config.onError) {
+        config.onError(new SupportError(), {
+          config
+        });
+      } else {
+        console.log(new SupportError());
+      }
       return;
     }
     const components = [[Accordion, config.accordion], [Button, config.button], [CharacterCount, config.characterCount], [Checkboxes], [ErrorSummary, config.errorSummary], [ExitThisPage, config.exitThisPage], [Header], [NotificationBanner, config.notificationBanner], [PasswordInput, config.passwordInput], [Radios], [SkipLink], [Tabs]];
-    const $scope = (_config$scope = config.scope) != null ? _config$scope : document;
+    const options = {
+      scope: (_config$scope = config.scope) != null ? _config$scope : document,
+      onError: config.onError
+    };
     components.forEach(([Component, config]) => {
-      createAll(Component, config, $scope);
+      createAll(Component, config, options);
     });
   }
 
@@ -2469,7 +2478,7 @@
    * @template {CompatibleClass} T
    * @typedef {object} ErrorContext
    * @property {Element} [element] - Element used for component module initialisation
-   * @property {T} component - Class of component
+   * @property {T} [component] - Class of component
    * @property {T["defaults"]} config - Config supplied to component
    */
   /**
diff --git a/packages/govuk-frontend/dist/govuk/all.bundle.mjs b/packages/govuk-frontend/dist/govuk/all.bundle.mjs
index 15a7feada..8add055cb 100644
--- a/packages/govuk-frontend/dist/govuk/all.bundle.mjs
+++ b/packages/govuk-frontend/dist/govuk/all.bundle.mjs
@@ -2350,19 +2350,28 @@ Tabs.moduleName = 'govuk-tabs';
  * Use the `data-module` attributes to find, instantiate and init all of the
  * components provided as part of GOV.UK Frontend.
  *
- * @param {Config & { scope?: Element }} [config] - Config for all components (with optional scope)
+ * @param {Config & { scope?: Element, onError?: OnErrorCallback<CompatibleClass> }} [config] - Config for all components (with optional scope)
  */
 function initAll(config) {
   var _config$scope;
   config = typeof config !== 'undefined' ? config : {};
   if (!isSupported()) {
-    console.log(new SupportError());
+    if (config.onError) {
+      config.onError(new SupportError(), {
+        config
+      });
+    } else {
+      console.log(new SupportError());
+    }
     return;
   }
   const components = [[Accordion, config.accordion], [Button, config.button], [CharacterCount, config.characterCount], [Checkboxes], [ErrorSummary, config.errorSummary], [ExitThisPage, config.exitThisPage], [Header], [NotificationBanner, config.notificationBanner], [PasswordInput, config.passwordInput], [Radios], [SkipLink], [Tabs]];
-  const $scope = (_config$scope = config.scope) != null ? _config$scope : document;
+  const options = {
+    scope: (_config$scope = config.scope) != null ? _config$scope : document,
+    onError: config.onError
+  };
   components.forEach(([Component, config]) => {
-    createAll(Component, config, $scope);
+    createAll(Component, config, options);
   });
 }
 
@@ -2463,7 +2472,7 @@ function createAll(Component, config, createAllOptions) {
  * @template {CompatibleClass} T
  * @typedef {object} ErrorContext
  * @property {Element} [element] - Element used for component module initialisation
- * @property {T} component - Class of component
+ * @property {T} [component] - Class of component
  * @property {T["defaults"]} config - Config supplied to component
  */
 /**
diff --git a/packages/govuk-frontend/dist/govuk/init.mjs b/packages/govuk-frontend/dist/govuk/init.mjs
index 2de2329d5..57b9296b7 100644
--- a/packages/govuk-frontend/dist/govuk/init.mjs
+++ b/packages/govuk-frontend/dist/govuk/init.mjs
@@ -19,19 +19,28 @@ import { SupportError } from './errors/index.mjs';
  * Use the `data-module` attributes to find, instantiate and init all of the
  * components provided as part of GOV.UK Frontend.
  *
- * @param {Config & { scope?: Element }} [config] - Config for all components (with optional scope)
+ * @param {Config & { scope?: Element, onError?: OnErrorCallback<CompatibleClass> }} [config] - Config for all components (with optional scope)
  */
 function initAll(config) {
   var _config$scope;
   config = typeof config !== 'undefined' ? config : {};
   if (!isSupported()) {
-    console.log(new SupportError());
+    if (config.onError) {
+      config.onError(new SupportError(), {
+        config
+      });
+    } else {
+      console.log(new SupportError());
+    }
     return;
   }
   const components = [[Accordion, config.accordion], [Button, config.button], [CharacterCount, config.characterCount], [Checkboxes], [ErrorSummary, config.errorSummary], [ExitThisPage, config.exitThisPage], [Header], [NotificationBanner, config.notificationBanner], [PasswordInput, config.passwordInput], [Radios], [SkipLink], [Tabs]];
-  const $scope = (_config$scope = config.scope) != null ? _config$scope : document;
+  const options = {
+    scope: (_config$scope = config.scope) != null ? _config$scope : document,
+    onError: config.onError
+  };
   components.forEach(([Component, config]) => {
-    createAll(Component, config, $scope);
+    createAll(Component, config, options);
   });
 }
 
@@ -132,7 +141,7 @@ function createAll(Component, config, createAllOptions) {
  * @template {CompatibleClass} T
  * @typedef {object} ErrorContext
  * @property {Element} [element] - Element used for component module initialisation
- * @property {T} component - Class of component
+ * @property {T} [component] - Class of component
  * @property {T["defaults"]} config - Config supplied to component
  */
 /**

Action run for 5983f3a

romaricpascal

Seems to flow nicely from initAll to createAll 🙌🏻 There's a couple of little things to tidy up in the test file before we can merge (a couple for a separate PR), but otherwise it's neat.

romaricpascal · 2024-08-29T17:03:18Z

packages/govuk-frontend/src/govuk/init.mjs

@@ -19,14 +19,23 @@ import { SupportError } from './errors/index.mjs'
 * Use the `data-module` attributes to find, instantiate and init all of the
 * components provided as part of GOV.UK Frontend.
 *
- * @param {Config & { scope?: Element }} [config] - Config for all components (with optional scope)
+ * @param {Config & { scope?: Element } & { onError?: OnErrorCallback<CompatibleClass> }} [config] - Config for all components (with optional scope)


suggestion Could this be grouped in a single object with all the non-Config options?

Suggested change

* @param {Config & { scope?: Element } & { onError?: OnErrorCallback<CompatibleClass> }} [config] - Config for all components (with optional scope)

* @param {Config & { scope?: Element, onError?: OnErrorCallback<CompatibleClass> }} [config] - Config for all components (with optional scope)

packages/govuk-frontend/src/govuk/init.mjs

packages/govuk-frontend/src/govuk/init.jsdom.test.mjs

romaricpascal · 2024-08-29T17:16:51Z

packages/govuk-frontend/src/govuk/init.jsdom.test.mjs

+      console.log(error)
+      console.log(context)


One for a separate PR, but this makes me think that one thing we didn't test is that createAll (and initAll) no longer log to the console if an onError is provided. Would be great if we had a way to test this alongside the fact that the callback is called.

If those two console.log calls are only here to support the two toHaveBeenCalledWith later on, it seems toHaveBeenCalledWith supports multiple arguments so those later calls might be doable as one:

expect(errorCallback).toHaveBeenCalledWith( expect.any(Error), expect.objectContaining({ // Whatever we expect }) )

Which would then allow us to check that the function no longer logged with

expect(global.console.log).not.toHaveBeenCalled()

I can consolidate the console.log check! I mean in theory it doesn't need to do any outputting at all. We can just check to see the error callback was called at all and then check to see if there's no error logged (the default behaviour).

Created #5303 to track the update of the tests.

romaricpascal · 2024-08-29T17:20:10Z

packages/govuk-frontend/src/govuk/init.jsdom.test.mjs

+    })
+
+    // Silence warnings in test output, and allow us to 'expect' them
+    jest.spyOn(global.console, 'log').mockImplementation()


nitpick Missed that during the review of createAll, so one for a separate PR as well, but seems jest.spyOn does not clean up after itself and we may need to call jest.restoreAllMocks to ensure the mock is cleaned up for further tests.

Added a small story to track this as we should have a look more widely than the console.log spies for the createAll and initAll tests.

- `config` parameter of `initAll` now accepts `onError` callback - `initAll` tests updated for `onError` callback

romaricpascal

Thanks for adressing the comments 😊 ⛵

patrickpatrickpatrick requested a review from romaricpascal August 29, 2024 16:12

govuk-design-system-ci temporarily deployed to govuk-frontend-pr-5276 August 29, 2024 16:12 Inactive

romaricpascal reviewed Aug 29, 2024

View reviewed changes

patrickpatrickpatrick force-pushed the on-error-init-all branch from 5421f10 to 47cf52f Compare August 30, 2024 09:56

govuk-design-system-ci temporarily deployed to govuk-frontend-pr-5276 August 30, 2024 09:56 Inactive

Add onError to initAll config

5983f3a

- `config` parameter of `initAll` now accepts `onError` callback - `initAll` tests updated for `onError` callback

patrickpatrickpatrick force-pushed the on-error-init-all branch from 47cf52f to 5983f3a Compare September 10, 2024 09:57

govuk-design-system-ci temporarily deployed to govuk-frontend-pr-5276 September 10, 2024 09:58 Inactive

romaricpascal approved these changes Sep 10, 2024

View reviewed changes

patrickpatrickpatrick merged commit 5783a2b into public-js-api Sep 11, 2024
48 checks passed

patrickpatrickpatrick deleted the on-error-init-all branch September 11, 2024 10:03

romaricpascal mentioned this pull request Sep 16, 2024

Add CHANGELOG entries for merged features #5321

Open

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `onError` to `initAll` config #5276

Add `onError` to `initAll` config #5276

patrickpatrickpatrick commented Aug 29, 2024 •

edited

Loading

github-actions bot commented Aug 29, 2024 •

edited

Loading

github-actions bot commented Aug 29, 2024 •

edited

Loading

github-actions bot commented Aug 29, 2024 •

edited

Loading

romaricpascal left a comment

romaricpascal Aug 29, 2024

romaricpascal Aug 29, 2024

patrickpatrickpatrick Aug 30, 2024

romaricpascal Sep 10, 2024

romaricpascal Aug 29, 2024

romaricpascal Sep 10, 2024

romaricpascal left a comment

	* @param {Config & { scope?: Element } & { onError?: OnErrorCallback<CompatibleClass> }} [config] - Config for all components (with optional scope)
	* @param {Config & { scope?: Element, onError?: OnErrorCallback<CompatibleClass> }} [config] - Config for all components (with optional scope)

Add onError to initAll config #5276

Add onError to initAll config #5276

Conversation

patrickpatrickpatrick commented Aug 29, 2024 • edited Loading

What

Why

github-actions bot commented Aug 29, 2024 • edited Loading

📋 Stats

File sizes

Modules

github-actions bot commented Aug 29, 2024 • edited Loading

JavaScript changes to npm package

github-actions bot commented Aug 29, 2024 • edited Loading

Other changes to npm package

romaricpascal left a comment

Choose a reason for hiding this comment

romaricpascal Aug 29, 2024

Choose a reason for hiding this comment

romaricpascal Aug 29, 2024

Choose a reason for hiding this comment

patrickpatrickpatrick Aug 30, 2024

Choose a reason for hiding this comment

romaricpascal Sep 10, 2024

Choose a reason for hiding this comment

romaricpascal Aug 29, 2024

Choose a reason for hiding this comment

romaricpascal Sep 10, 2024

Choose a reason for hiding this comment

romaricpascal left a comment

Choose a reason for hiding this comment

Add `onError` to `initAll` config #5276

Add `onError` to `initAll` config #5276

patrickpatrickpatrick commented Aug 29, 2024 •

edited

Loading

github-actions bot commented Aug 29, 2024 •

edited

Loading

github-actions bot commented Aug 29, 2024 •

edited

Loading

github-actions bot commented Aug 29, 2024 •

edited

Loading