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

Update createAll types so they don't rely on component's defaults #5496

Merged
merged 3 commits into from
Nov 14, 2024

Conversation

romaricpascal
Copy link
Member

@romaricpascal romaricpascal commented Nov 12, 2024

Use TypeScript's ConstructorParameters to pick the type for the component's configuration from the arguments of its constructor, rather than its defaults property.

Thoughts

This is closer to the actual behaviour of the function, which passes the config as the second argument of the constructor.

Testing in the init.jsdom.test.mjs (as it imports both createAll and components):

  • Completions work OK for components that accept a configuration:
    Screenshot of code editor using createAll to instantiate an Accordion, showing completions for the accordion configuration

  • For components that do not accept configuration, config is typed as any
    Screenshot of code editor using createAll to instantiate a component without config, showing the type any for the second argument of the function

Future work

Because createAll no longer relies on defaults we can set the defaults static property of the components as @protected to clarify the scope of our public API further. Given configuration passed to the constructor gets merged with those defaults, there's little need for them to be accessed outside of the component's class (or its descendents).

Copy link

github-actions bot commented Nov 12, 2024

📋 Stats

File sizes

File Size
dist/govuk-frontend-development.min.css 118.41 KiB
dist/govuk-frontend-development.min.js 42.84 KiB
packages/govuk-frontend/dist/govuk/all.bundle.js 92.78 KiB
packages/govuk-frontend/dist/govuk/all.bundle.mjs 87.16 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.83 KiB
packages/govuk-frontend/dist/govuk/i18n.mjs 5.55 KiB
packages/govuk-frontend/dist/govuk/init.mjs 7.13 KiB

Modules

File Size (bundled) Size (minified)
all.mjs 82.63 KiB 40.37 KiB
accordion.mjs 26.44 KiB 13.33 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 3c4cb8c

Copy link

github-actions bot commented Nov 12, 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 956e54478..73172514b 100644
--- a/packages/govuk-frontend/dist/govuk/all.bundle.js
+++ b/packages/govuk-frontend/dist/govuk/all.bundle.js
@@ -2447,11 +2447,11 @@
    *
    * Any component errors will be caught and logged to the console.
    *
-   * @template {CompatibleClass} T
-   * @param {T} Component - class of the component to create
-   * @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
+   * @template {CompatibleClass} ComponentClass
+   * @param {ComponentClass} Component - class of the component to create
+   * @param {ComponentConfig<ComponentClass>} [config] - Config supplied to component
+   * @param {OnErrorCallback<ComponentClass> | Element | Document | CreateAllOptions<ComponentClass> } [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<ComponentClass>>} - array of instantiated components
    */
   function createAll(Component, config, createAllOptions) {
     let $scope = document;
@@ -2498,7 +2498,7 @@
     }).filter(Boolean);
   }
   /**
-   * @typedef {{new (...args: any[]): any, defaults?: object, moduleName: string}} CompatibleClass
+   * @typedef {{new (...args: any[]): any, moduleName: string}} CompatibleClass
    */
   /**
    * Config for all components via `initAll()`
@@ -2532,23 +2532,27 @@
    * @typedef {keyof Config} ConfigKey
    */
   /**
-   * @template {CompatibleClass} T
+   * @template {CompatibleClass} ComponentClass
+   * @typedef {ConstructorParameters<ComponentClass>[1]} ComponentConfig
+   */
+  /**
+   * @template {CompatibleClass} ComponentClass
    * @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
+   * @property {ComponentClass} [component] - Class of component
+   * @property {ComponentConfig<ComponentClass>} config - Config supplied to component
    */
   /**
-   * @template {CompatibleClass} T
+   * @template {CompatibleClass} ComponentClass
    * @callback OnErrorCallback
    * @param {unknown} error - Thrown error
-   * @param {ErrorContext<T>} context - Object containing the element, component class and configuration
+   * @param {ErrorContext<ComponentClass>} context - Object containing the element, component class and configuration
    */
   /**
-   * @template {CompatibleClass} T
+   * @template {CompatibleClass} ComponentClass
    * @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
+   * @property {OnErrorCallback<ComponentClass>} [onError] - callback function if error throw by component on init
    */
 
   exports.Accordion = Accordion;
diff --git a/packages/govuk-frontend/dist/govuk/all.bundle.mjs b/packages/govuk-frontend/dist/govuk/all.bundle.mjs
index b56f12f83..188b6100c 100644
--- a/packages/govuk-frontend/dist/govuk/all.bundle.mjs
+++ b/packages/govuk-frontend/dist/govuk/all.bundle.mjs
@@ -2441,11 +2441,11 @@ function initAll(config) {
  *
  * Any component errors will be caught and logged to the console.
  *
- * @template {CompatibleClass} T
- * @param {T} Component - class of the component to create
- * @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
+ * @template {CompatibleClass} ComponentClass
+ * @param {ComponentClass} Component - class of the component to create
+ * @param {ComponentConfig<ComponentClass>} [config] - Config supplied to component
+ * @param {OnErrorCallback<ComponentClass> | Element | Document | CreateAllOptions<ComponentClass> } [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<ComponentClass>>} - array of instantiated components
  */
 function createAll(Component, config, createAllOptions) {
   let $scope = document;
@@ -2492,7 +2492,7 @@ function createAll(Component, config, createAllOptions) {
   }).filter(Boolean);
 }
 /**
- * @typedef {{new (...args: any[]): any, defaults?: object, moduleName: string}} CompatibleClass
+ * @typedef {{new (...args: any[]): any, moduleName: string}} CompatibleClass
  */
 /**
  * Config for all components via `initAll()`
@@ -2526,23 +2526,27 @@ function createAll(Component, config, createAllOptions) {
  * @typedef {keyof Config} ConfigKey
  */
 /**
- * @template {CompatibleClass} T
+ * @template {CompatibleClass} ComponentClass
+ * @typedef {ConstructorParameters<ComponentClass>[1]} ComponentConfig
+ */
+/**
+ * @template {CompatibleClass} ComponentClass
  * @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
+ * @property {ComponentClass} [component] - Class of component
+ * @property {ComponentConfig<ComponentClass>} config - Config supplied to component
  */
 /**
- * @template {CompatibleClass} T
+ * @template {CompatibleClass} ComponentClass
  * @callback OnErrorCallback
  * @param {unknown} error - Thrown error
- * @param {ErrorContext<T>} context - Object containing the element, component class and configuration
+ * @param {ErrorContext<ComponentClass>} context - Object containing the element, component class and configuration
  */
 /**
- * @template {CompatibleClass} T
+ * @template {CompatibleClass} ComponentClass
  * @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
+ * @property {OnErrorCallback<ComponentClass>} [onError] - callback function if error throw by component on init
  */
 
 export { Accordion, Button, CharacterCount, Checkboxes, GOVUKFrontendComponent as Component, ErrorSummary, ExitThisPage, Header, NotificationBanner, PasswordInput, Radios, ServiceNavigation, SkipLink, Tabs, createAll, initAll, isSupported, version };
diff --git a/packages/govuk-frontend/dist/govuk/init.mjs b/packages/govuk-frontend/dist/govuk/init.mjs
index 9b087239b..7bc77bdab 100644
--- a/packages/govuk-frontend/dist/govuk/init.mjs
+++ b/packages/govuk-frontend/dist/govuk/init.mjs
@@ -54,11 +54,11 @@ function initAll(config) {
  *
  * Any component errors will be caught and logged to the console.
  *
- * @template {CompatibleClass} T
- * @param {T} Component - class of the component to create
- * @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
+ * @template {CompatibleClass} ComponentClass
+ * @param {ComponentClass} Component - class of the component to create
+ * @param {ComponentConfig<ComponentClass>} [config] - Config supplied to component
+ * @param {OnErrorCallback<ComponentClass> | Element | Document | CreateAllOptions<ComponentClass> } [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<ComponentClass>>} - array of instantiated components
  */
 function createAll(Component, config, createAllOptions) {
   let $scope = document;
@@ -105,7 +105,7 @@ function createAll(Component, config, createAllOptions) {
   }).filter(Boolean);
 }
 /**
- * @typedef {{new (...args: any[]): any, defaults?: object, moduleName: string}} CompatibleClass
+ * @typedef {{new (...args: any[]): any, moduleName: string}} CompatibleClass
  */
 /**
  * Config for all components via `initAll()`
@@ -139,23 +139,27 @@ function createAll(Component, config, createAllOptions) {
  * @typedef {keyof Config} ConfigKey
  */
 /**
- * @template {CompatibleClass} T
+ * @template {CompatibleClass} ComponentClass
+ * @typedef {ConstructorParameters<ComponentClass>[1]} ComponentConfig
+ */
+/**
+ * @template {CompatibleClass} ComponentClass
  * @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
+ * @property {ComponentClass} [component] - Class of component
+ * @property {ComponentConfig<ComponentClass>} config - Config supplied to component
  */
 /**
- * @template {CompatibleClass} T
+ * @template {CompatibleClass} ComponentClass
  * @callback OnErrorCallback
  * @param {unknown} error - Thrown error
- * @param {ErrorContext<T>} context - Object containing the element, component class and configuration
+ * @param {ErrorContext<ComponentClass>} context - Object containing the element, component class and configuration
  */
 /**
- * @template {CompatibleClass} T
+ * @template {CompatibleClass} ComponentClass
  * @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
+ * @property {OnErrorCallback<ComponentClass>} [onError] - callback function if error throw by component on init
  */
 
 export { createAll, initAll };

Action run for 3c4cb8c

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-5496 November 12, 2024 18:53 Inactive
Copy link
Contributor

@domoscargin domoscargin left a comment

Choose a reason for hiding this comment

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

This looks grand to me!

@romaricpascal romaricpascal merged commit a657b0e into main Nov 14, 2024
50 checks passed
@romaricpascal romaricpascal deleted the createall-type-no-defaults branch November 14, 2024 15:15
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.

[Stretch] Investigate if we can use config instead of defaults in createAll to get the configuration type
3 participants