[SPIKE] Use TypeScript for the Accordion's defaults immutability

[romaricpascal]

trafficstars

Object.freeze only does a shallow freeze of the object, meaning we have nothing checking our code against something setting one of the keys of i18n. The ReadonlyDeep<T> type from type-fest allows TypeScript to check that even deep properties cannot be updated by our code.

This spike illustrates how we would use the ReadonlyDeep<t> type, using the Accordion component and building on the fact that we define the types for each component's configuration as <COMPONENT_NAME>Config.

[!IMPORTANT] Tests are expected to fail on this spike to demonstrate that TypeScript does indeed pick up on nested properties of the Accordion's defaults getting modified when they shouldn't

Thoughts

Removing Object.freeze

Because Object.freeze only does a shallow freeze, it provide a false sense of safety that deep keys are immutable if you're not aware of its shallowness. Implementing a function to deep freeze an object recursively would mean extra code shipped to the browser for the sake of preventing users' code from modifying a component's defaults.

Leaving these defaults unprotected by removing Object.freeze doesn't seem a massive risk. If anything it allows an undocumented way to set defaults for all the components of a given class, while allowing multiple calls to createAll or initAll to override those defaults with specific configurations for given pages or parts of pages. And it shaves a little bit of code which won't be sent to the browser.

Keeping things readonly in our code

This doesn't mean we cannot check that our code doesn't inadvertently try to update one of the properties in defaults. TypeScript's Readonly<T> type unfortunately works shallowly, just like Object.freeze. However, the type-fest package provides a ReadonlyDeep<T> type that ensures nested objects/Maps/Sets/Arrays are also readonly (and is heavily tested).

The type-fest package doesn't add to our own dependencies as type checking happens at build time, so it only needs to be a devDependency. However, it'll have impact on the @types/govuk-frontend package, for which it's become a dependency (unless there's a system for vendoring it).

[github-actions[bot]]

:clipboard: Stats

File sizes

File	Size
dist/govuk-frontend-development.min.css	118.41 KiB
dist/govuk-frontend-development.min.js	42.98 KiB
packages/govuk-frontend/dist/govuk/all.bundle.js	92.81 KiB
packages/govuk-frontend/dist/govuk/all.bundle.mjs	87.18 KiB
packages/govuk-frontend/dist/govuk/all.mjs	1.18 KiB
packages/govuk-frontend/dist/govuk/govuk-frontend-component.mjs	1.74 KiB
packages/govuk-frontend/dist/govuk/govuk-frontend.min.css	118.4 KiB
packages/govuk-frontend/dist/govuk/govuk-frontend.min.js	42.96 KiB
packages/govuk-frontend/dist/govuk/i18n.mjs	5.55 KiB
packages/govuk-frontend/dist/govuk/init.mjs	6.85 KiB

Modules

File	Size (bundled)	Size (minified)
all.mjs	81.24 KiB	40.48 KiB
accordion.mjs	25.14 KiB	13.44 KiB
button.mjs	8.95 KiB	3.71 KiB
character-count.mjs	24.57 KiB	10.6 KiB
checkboxes.mjs	7.81 KiB	3.42 KiB
error-summary.mjs	10.85 KiB	4.47 KiB
exit-this-page.mjs	20.06 KiB	10.26 KiB
header.mjs	6.46 KiB	3.22 KiB
notification-banner.mjs	9.21 KiB	3.63 KiB
password-input.mjs	18.11 KiB	8.26 KiB
radios.mjs	6.81 KiB	2.98 KiB
service-navigation.mjs	6.44 KiB	3.26 KiB
skip-link.mjs	6.4 KiB	2.76 KiB
tabs.mjs	12.04 KiB	6.67 KiB

View stats and visualisations on the review app

---

Action run for 304294889ec38dc92bebf383ada315c90b09647d

[github-actions[bot]]

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 64b3fda7d..a4a31eec1 100644
--- a/packages/govuk-frontend/dist/govuk/govuk-frontend.min.js
+++ b/packages/govuk-frontend/dist/govuk/govuk-frontend.min.js
@@ -224,6 +224,7 @@ class I18n {
         }
     }
 }
+var t;
 I18n.pluralRulesMap = {
     arabic: ["ar"],
     chinese: ["my", "zh", "id", "ja", "jv", "ko", "ms", "th", "vi"],
@@ -377,7 +378,7 @@ class Accordion extends GOVUKFrontendComponentConfigurable {
         return t.classList.add("govuk-visually-hidden", this.sectionHeadingDividerClass), t.textContent = ", ", t
     }
 }
-Accordion.moduleName = "govuk-accordion", Accordion.defaults = Object.freeze({
+Accordion.moduleName = "govuk-accordion", Accordion.defaults = {
     i18n: {
         hideAllSections: "Hide all sections",
         hideSection: "Hide",
@@ -387,7 +388,7 @@ Accordion.moduleName = "govuk-accordion", Accordion.defaults = Object.freeze({
         showSectionAriaLabel: "Show this section"
     },
     rememberExpanded: !0
-}), Accordion.schema = Object.freeze({
+}, Accordion.schema = Object.freeze({
     properties: {
         i18n: {
             type: "object"
@@ -396,7 +397,7 @@ Accordion.moduleName = "govuk-accordion", Accordion.defaults = Object.freeze({
             type: "boolean"
         }
     }
-});
+}), Accordion.defaults.rememberExpanded = !1, null != (t = Accordion.defaults.i18n) && t.hideSection && (Accordion.defaults.i18n.hideSection = "Some new string");
 class Button extends GOVUKFrontendComponentConfigurable {
     constructor(t, e = {}) {
         super(t, e), this.debounceFormSubmitTimer = null, this.$root.addEventListener("keydown", (t => this.handleKeyDown(t))), this.$root.addEventListener("click", (t => this.debounce(t)))

---

Action run for 304294889ec38dc92bebf383ada315c90b09647d

[github-actions[bot]]

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 956e54478..224d68eae 100644
--- a/packages/govuk-frontend/dist/govuk/all.bundle.js
+++ b/packages/govuk-frontend/dist/govuk/all.bundle.js
@@ -530,6 +530,8 @@
     }
   };
 
+  var _Accordion$defaults$i;
+
   /**
    * Accordion component
    *
@@ -810,6 +812,32 @@
       return $punctuationEl;
     }
   }
+  Accordion.moduleName = 'govuk-accordion';
+  Accordion.defaults = {
+    i18n: {
+      hideAllSections: 'Hide all sections',
+      hideSection: 'Hide',
+      hideSectionAriaLabel: 'Hide this section',
+      showAllSections: 'Show all sections',
+      showSection: 'Show',
+      showSectionAriaLabel: 'Show this section'
+    },
+    rememberExpanded: true
+  };
+  Accordion.schema = Object.freeze({
+    properties: {
+      i18n: {
+        type: 'object'
+      },
+      rememberExpanded: {
+        type: 'boolean'
+      }
+    }
+  });
+  Accordion.defaults.rememberExpanded = false;
+  if ((_Accordion$defaults$i = Accordion.defaults.i18n) != null && _Accordion$defaults$i.hideSection) {
+    Accordion.defaults.i18n.hideSection = 'Some new string';
+  }
 
   /**
    * Accordion config
@@ -847,28 +875,11 @@
   /**
    * @typedef {import('../../common/configuration.mjs').Schema} Schema
    */
-  Accordion.moduleName = 'govuk-accordion';
-  Accordion.defaults = Object.freeze({
-    i18n: {
-      hideAllSections: 'Hide all sections',
-      hideSection: 'Hide',
-      hideSectionAriaLabel: 'Hide this section',
-      showAllSections: 'Show all sections',
-      showSection: 'Show',
-      showSectionAriaLabel: 'Show this section'
-    },
-    rememberExpanded: true
-  });
-  Accordion.schema = Object.freeze({
-    properties: {
-      i18n: {
-        type: 'object'
-      },
-      rememberExpanded: {
-        type: 'boolean'
-      }
-    }
-  });
+
+  /**
+   * @template T
+   * @typedef {import('type-fest').ReadonlyDeep<T>} ReadonlyDeep<T>
+   */
 
   const DEBOUNCE_TIMEOUT_IN_SECONDS = 1;
 
diff --git a/packages/govuk-frontend/dist/govuk/all.bundle.mjs b/packages/govuk-frontend/dist/govuk/all.bundle.mjs
index b56f12f83..94fe8a0d6 100644
--- a/packages/govuk-frontend/dist/govuk/all.bundle.mjs
+++ b/packages/govuk-frontend/dist/govuk/all.bundle.mjs
@@ -524,6 +524,8 @@ I18n.pluralRules = {
   }
 };
 
+var _Accordion$defaults$i;
+
 /**
  * Accordion component
  *
@@ -804,6 +806,32 @@ class Accordion extends GOVUKFrontendComponentConfigurable {
     return $punctuationEl;
   }
 }
+Accordion.moduleName = 'govuk-accordion';
+Accordion.defaults = {
+  i18n: {
+    hideAllSections: 'Hide all sections',
+    hideSection: 'Hide',
+    hideSectionAriaLabel: 'Hide this section',
+    showAllSections: 'Show all sections',
+    showSection: 'Show',
+    showSectionAriaLabel: 'Show this section'
+  },
+  rememberExpanded: true
+};
+Accordion.schema = Object.freeze({
+  properties: {
+    i18n: {
+      type: 'object'
+    },
+    rememberExpanded: {
+      type: 'boolean'
+    }
+  }
+});
+Accordion.defaults.rememberExpanded = false;
+if ((_Accordion$defaults$i = Accordion.defaults.i18n) != null && _Accordion$defaults$i.hideSection) {
+  Accordion.defaults.i18n.hideSection = 'Some new string';
+}
 
 /**
  * Accordion config
@@ -841,28 +869,11 @@ class Accordion extends GOVUKFrontendComponentConfigurable {
 /**
  * @typedef {import('../../common/configuration.mjs').Schema} Schema
  */
-Accordion.moduleName = 'govuk-accordion';
-Accordion.defaults = Object.freeze({
-  i18n: {
-    hideAllSections: 'Hide all sections',
-    hideSection: 'Hide',
-    hideSectionAriaLabel: 'Hide this section',
-    showAllSections: 'Show all sections',
-    showSection: 'Show',
-    showSectionAriaLabel: 'Show this section'
-  },
-  rememberExpanded: true
-});
-Accordion.schema = Object.freeze({
-  properties: {
-    i18n: {
-      type: 'object'
-    },
-    rememberExpanded: {
-      type: 'boolean'
-    }
-  }
-});
+
+/**
+ * @template T
+ * @typedef {import('type-fest').ReadonlyDeep<T>} ReadonlyDeep<T>
+ */
 
 const DEBOUNCE_TIMEOUT_IN_SECONDS = 1;
 
diff --git a/packages/govuk-frontend/dist/govuk/components/accordion/accordion.bundle.js b/packages/govuk-frontend/dist/govuk/components/accordion/accordion.bundle.js
index 7eb946c6d..e2f49db2c 100644
--- a/packages/govuk-frontend/dist/govuk/components/accordion/accordion.bundle.js
+++ b/packages/govuk-frontend/dist/govuk/components/accordion/accordion.bundle.js
@@ -470,6 +470,8 @@
     }
   };
 
+  var _Accordion$defaults$i;
+
   /**
    * Accordion component
    *
@@ -750,6 +752,32 @@
       return $punctuationEl;
     }
   }
+  Accordion.moduleName = 'govuk-accordion';
+  Accordion.defaults = {
+    i18n: {
+      hideAllSections: 'Hide all sections',
+      hideSection: 'Hide',
+      hideSectionAriaLabel: 'Hide this section',
+      showAllSections: 'Show all sections',
+      showSection: 'Show',
+      showSectionAriaLabel: 'Show this section'
+    },
+    rememberExpanded: true
+  };
+  Accordion.schema = Object.freeze({
+    properties: {
+      i18n: {
+        type: 'object'
+      },
+      rememberExpanded: {
+        type: 'boolean'
+      }
+    }
+  });
+  Accordion.defaults.rememberExpanded = false;
+  if ((_Accordion$defaults$i = Accordion.defaults.i18n) != null && _Accordion$defaults$i.hideSection) {
+    Accordion.defaults.i18n.hideSection = 'Some new string';
+  }
 
   /**
    * Accordion config
@@ -787,28 +815,11 @@
   /**
    * @typedef {import('../../common/configuration.mjs').Schema} Schema
    */
-  Accordion.moduleName = 'govuk-accordion';
-  Accordion.defaults = Object.freeze({
-    i18n: {
-      hideAllSections: 'Hide all sections',
-      hideSection: 'Hide',
-      hideSectionAriaLabel: 'Hide this section',
-      showAllSections: 'Show all sections',
-      showSection: 'Show',
-      showSectionAriaLabel: 'Show this section'
-    },
-    rememberExpanded: true
-  });
-  Accordion.schema = Object.freeze({
-    properties: {
-      i18n: {
-        type: 'object'
-      },
-      rememberExpanded: {
-        type: 'boolean'
-      }
-    }
-  });
+
+  /**
+   * @template T
+   * @typedef {import('type-fest').ReadonlyDeep<T>} ReadonlyDeep<T>
+   */
 
   exports.Accordion = Accordion;
 
diff --git a/packages/govuk-frontend/dist/govuk/components/accordion/accordion.bundle.mjs b/packages/govuk-frontend/dist/govuk/components/accordion/accordion.bundle.mjs
index dbb20e68a..486d050a3 100644
--- a/packages/govuk-frontend/dist/govuk/components/accordion/accordion.bundle.mjs
+++ b/packages/govuk-frontend/dist/govuk/components/accordion/accordion.bundle.mjs
@@ -464,6 +464,8 @@ I18n.pluralRules = {
   }
 };
 
+var _Accordion$defaults$i;
+
 /**
  * Accordion component
  *
@@ -744,6 +746,32 @@ class Accordion extends GOVUKFrontendComponentConfigurable {
     return $punctuationEl;
   }
 }
+Accordion.moduleName = 'govuk-accordion';
+Accordion.defaults = {
+  i18n: {
+    hideAllSections: 'Hide all sections',
+    hideSection: 'Hide',
+    hideSectionAriaLabel: 'Hide this section',
+    showAllSections: 'Show all sections',
+    showSection: 'Show',
+    showSectionAriaLabel: 'Show this section'
+  },
+  rememberExpanded: true
+};
+Accordion.schema = Object.freeze({
+  properties: {
+    i18n: {
+      type: 'object'
+    },
+    rememberExpanded: {
+      type: 'boolean'
+    }
+  }
+});
+Accordion.defaults.rememberExpanded = false;
+if ((_Accordion$defaults$i = Accordion.defaults.i18n) != null && _Accordion$defaults$i.hideSection) {
+  Accordion.defaults.i18n.hideSection = 'Some new string';
+}
 
 /**
  * Accordion config
@@ -781,28 +809,11 @@ class Accordion extends GOVUKFrontendComponentConfigurable {
 /**
  * @typedef {import('../../common/configuration.mjs').Schema} Schema
  */
-Accordion.moduleName = 'govuk-accordion';
-Accordion.defaults = Object.freeze({
-  i18n: {
-    hideAllSections: 'Hide all sections',
-    hideSection: 'Hide',
-    hideSectionAriaLabel: 'Hide this section',
-    showAllSections: 'Show all sections',
-    showSection: 'Show',
-    showSectionAriaLabel: 'Show this section'
-  },
-  rememberExpanded: true
-});
-Accordion.schema = Object.freeze({
-  properties: {
-    i18n: {
-      type: 'object'
-    },
-    rememberExpanded: {
-      type: 'boolean'
-    }
-  }
-});
+
+/**
+ * @template T
+ * @typedef {import('type-fest').ReadonlyDeep<T>} ReadonlyDeep<T>
+ */
 
 export { Accordion };
 //# sourceMappingURL=accordion.bundle.mjs.map
diff --git a/packages/govuk-frontend/dist/govuk/components/accordion/accordion.mjs b/packages/govuk-frontend/dist/govuk/components/accordion/accordion.mjs
index 4cd678923..9c66c463d 100644
--- a/packages/govuk-frontend/dist/govuk/components/accordion/accordion.mjs
+++ b/packages/govuk-frontend/dist/govuk/components/accordion/accordion.mjs
@@ -2,6 +2,8 @@ import { GOVUKFrontendComponentConfigurable } from '../../common/configuration.m
 import { ElementError } from '../../errors/index.mjs';
 import { I18n } from '../../i18n.mjs';
 
+var _Accordion$defaults$i;
+
 /**
  * Accordion component
  *
@@ -282,6 +284,32 @@ class Accordion extends GOVUKFrontendComponentConfigurable {
     return $punctuationEl;
   }
 }
+Accordion.moduleName = 'govuk-accordion';
+Accordion.defaults = {
+  i18n: {
+    hideAllSections: 'Hide all sections',
+    hideSection: 'Hide',
+    hideSectionAriaLabel: 'Hide this section',
+    showAllSections: 'Show all sections',
+    showSection: 'Show',
+    showSectionAriaLabel: 'Show this section'
+  },
+  rememberExpanded: true
+};
+Accordion.schema = Object.freeze({
+  properties: {
+    i18n: {
+      type: 'object'
+    },
+    rememberExpanded: {
+      type: 'boolean'
+    }
+  }
+});
+Accordion.defaults.rememberExpanded = false;
+if ((_Accordion$defaults$i = Accordion.defaults.i18n) != null && _Accordion$defaults$i.hideSection) {
+  Accordion.defaults.i18n.hideSection = 'Some new string';
+}
 
 /**
  * Accordion config
@@ -319,28 +347,11 @@ class Accordion extends GOVUKFrontendComponentConfigurable {
 /**
  * @typedef {import('../../common/configuration.mjs').Schema} Schema
  */
-Accordion.moduleName = 'govuk-accordion';
-Accordion.defaults = Object.freeze({
-  i18n: {
-    hideAllSections: 'Hide all sections',
-    hideSection: 'Hide',
-    hideSectionAriaLabel: 'Hide this section',
-    showAllSections: 'Show all sections',
-    showSection: 'Show',
-    showSectionAriaLabel: 'Show this section'
-  },
-  rememberExpanded: true
-});
-Accordion.schema = Object.freeze({
-  properties: {
-    i18n: {
-      type: 'object'
-    },
-    rememberExpanded: {
-      type: 'boolean'
-    }
-  }
-});
+
+/**
+ * @template T
+ * @typedef {import('type-fest').ReadonlyDeep<T>} ReadonlyDeep<T>
+ */
 
 export { Accordion };
 //# sourceMappingURL=accordion.mjs.map

---

Action run for 304294889ec38dc92bebf383ada315c90b09647d

[domoscargin]

This feels like a sensible approach - I think we definitely want to make sure we're testing for read-onliness internally. I'm less worried about the impact on @types/govuk-frontend, tbh.

[romaricpascal]

I think we definitely want to make sure we're testing for read-onliness internally

I know your comment now dates from a while back, but do you remember what you meant by this? Was this about having checks when our code is built or having checks happening when code runs in the browser?

[romaricpascal]

For the former, TypeScript is taking care of checking that 🥳

For the later, we could add some defensiveness (for example, wrapping the default config in a Proxy that throws if you use a setter). I'd like us to check if users need that defensiveness as it'd be another low level part for us to maintain (and Kbs sent to browsers, though that probably won't be much).

[domoscargin]

Pretty sure I was just reacting positively to this spike! I think we had some discussions about whether, if we dropped the Object.freeze, we needed to do anything else to enforce read-onliness, so this was just me saying doing it with typechecking is fine!