Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add isSupported to createAll #5251

Merged
merged 1 commit into from
Aug 20, 2024
Merged

Conversation

patrickpatrickpatrick
Copy link
Contributor

@patrickpatrickpatrick patrickpatrickpatrick commented Aug 19, 2024

What

  • createAll returns empty array and throws error is isSupported false
  • update existing createAll tests and add test for isSupported false

Why

Prevents createAll running if govuk-frontend not supported.

Fixes #5213

Copy link

github-actions bot commented Aug 19, 2024

📋 Stats

File sizes

File Size
dist/govuk-frontend-development.min.css 112.61 KiB
dist/govuk-frontend-development.min.js 41.89 KiB
packages/govuk-frontend/dist/govuk/all.bundle.js 87.8 KiB
packages/govuk-frontend/dist/govuk/all.bundle.mjs 82.45 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 41.88 KiB
packages/govuk-frontend/dist/govuk/i18n.mjs 5.55 KiB
packages/govuk-frontend/dist/govuk/init.mjs 4.94 KiB

Modules

File Size (bundled) Size (minified)
all.mjs 79.57 KiB 39.84 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 3301745

Copy link

github-actions bot commented Aug 19, 2024

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 9dc0b2147..69492a902 100644
--- a/packages/govuk-frontend/dist/govuk/govuk-frontend.min.js
+++ b/packages/govuk-frontend/dist/govuk/govuk-frontend.min.js
@@ -1106,13 +1106,13 @@ function initAll(e) {
 
 function createAll(e, t, n = document) {
     const i = n.querySelectorAll(`[data-module="${e.moduleName}"]`);
-    return Array.from(i).map((n => {
+    return isSupported() ? Array.from(i).map((n => {
         try {
             return "defaults" in e && void 0 !== t ? new e(n, t) : new e(n)
         } catch (i) {
             return console.log(i), null
         }
-    })).filter(Boolean)
+    })).filter(Boolean) : (console.log(new SupportError), [])
 }
 Tabs.moduleName = "govuk-tabs";
 export {

Action run for 3301745

Copy link

github-actions bot commented Aug 19, 2024

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 fd8a866cd..3b86ffe36 100644
--- a/packages/govuk-frontend/dist/govuk/all.bundle.js
+++ b/packages/govuk-frontend/dist/govuk/all.bundle.js
@@ -2389,6 +2389,10 @@
    */
   function createAll(Component, config, $scope = document) {
     const $elements = $scope.querySelectorAll(`[data-module="${Component.moduleName}"]`);
+    if (!isSupported()) {
+      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);
diff --git a/packages/govuk-frontend/dist/govuk/all.bundle.mjs b/packages/govuk-frontend/dist/govuk/all.bundle.mjs
index 1baaac9d9..8725b363d 100644
--- a/packages/govuk-frontend/dist/govuk/all.bundle.mjs
+++ b/packages/govuk-frontend/dist/govuk/all.bundle.mjs
@@ -2383,6 +2383,10 @@ function initAll(config) {
  */
 function createAll(Component, config, $scope = document) {
   const $elements = $scope.querySelectorAll(`[data-module="${Component.moduleName}"]`);
+  if (!isSupported()) {
+    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);
diff --git a/packages/govuk-frontend/dist/govuk/init.mjs b/packages/govuk-frontend/dist/govuk/init.mjs
index f985eaffb..c8b4ab490 100644
--- a/packages/govuk-frontend/dist/govuk/init.mjs
+++ b/packages/govuk-frontend/dist/govuk/init.mjs
@@ -52,6 +52,10 @@ function initAll(config) {
  */
 function createAll(Component, config, $scope = document) {
   const $elements = $scope.querySelectorAll(`[data-module="${Component.moduleName}"]`);
+  if (!isSupported()) {
+    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);

Action run for 3301745

@romaricpascal romaricpascal changed the base branch from main to public-js-api August 19, 2024 14:22
@romaricpascal
Copy link
Member

@patrickpatrickpatrick I've change the base of the PR to not point to main, so we can work on the JS API without disturbing the Navigation component release.

beforeEach(() => {
document.body.classList.add('govuk-frontend-supported')
})

afterEach(() => {
document.body.innerHTML = ''
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion As JSDOM doesn't get cleaned between the same tests of a given file we should ensure the class gets removed as well.

Suggested change
document.body.innerHTML = ''
document.body.outerHTML = '<body></body>'

Copy link
Member

@romaricpascal romaricpascal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks all neat, I'd just suggest a little more safety in the tests cleanup now we add a class before each of the createAll tests.

- createAll returns empty array and throws error is `isSupported` false
- update existing createAll tests and add test for `isSupported` false
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-5251 August 19, 2024 15:37 Inactive
@patrickpatrickpatrick patrickpatrickpatrick merged commit 69763be into public-js-api Aug 20, 2024
48 checks passed
@patrickpatrickpatrick patrickpatrickpatrick deleted the create-all-check branch August 20, 2024 08:37
@romaricpascal romaricpascal linked an issue Aug 20, 2024 that may be closed by this pull request
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add leading check to verify that GOV.UK Frontend is supported in createAll
3 participants