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

Throw ElementError when the module is not of the expected type #4104

Merged
merged 7 commits into from
Sep 18, 2023

Conversation

romaricpascal
Copy link
Member

@romaricpascal romaricpascal commented Aug 18, 2023

Make each component throw a TypeError when the $module it receives is not of the type it needs to function properly.

Closes #4035.

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-4104 August 18, 2023 14:03 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-4104 August 18, 2023 14:14 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-4104 August 18, 2023 15:29 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-4104 August 18, 2023 16:11 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-4104 August 18, 2023 16:12 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-4104 August 18, 2023 16:18 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-4104 August 18, 2023 16:22 Inactive
@romaricpascal romaricpascal marked this pull request as ready for review August 18, 2023 16:28
@domoscargin
Copy link
Contributor

domoscargin commented Aug 24, 2023

I agree that the generic types approach seems neater and less prone to error here.

I guess we could also make the moduleType getter @abstract and force child classes to implement it, but I don't think that really solves much - just delegates more stuff to the child class instead of abstracting it into the base class.

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-4104 August 24, 2023 16:06 Inactive
@github-actions
Copy link

github-actions bot commented Aug 24, 2023

📋 Stats

File sizes

File Size
dist/govuk-frontend-4.6.0.min.css 118.07 KiB
dist/govuk-frontend-4.6.0.min.js 47.1 KiB
dist/govuk-frontend-ie8-4.6.0.min.css 79.27 KiB
packages/govuk-frontend/dist/govuk/all.bundle.js 75.59 KiB
packages/govuk-frontend/dist/govuk/all.bundle.mjs 71.01 KiB
packages/govuk-frontend/dist/govuk/all.mjs 3.84 KiB
packages/govuk-frontend/dist/govuk/govuk-frontend-component.mjs 359 B
packages/govuk-frontend/dist/govuk/govuk-frontend.min.js 36.79 KiB
packages/govuk-frontend/dist/govuk/i18n.mjs 5.83 KiB

Modules

File Size
all.mjs 67.11 KiB
components/accordion/accordion.mjs 21.78 KiB
components/button/button.mjs 4.58 KiB
components/character-count/character-count.mjs 21.52 KiB
components/checkboxes/checkboxes.mjs 5.28 KiB
components/error-summary/error-summary.mjs 5.88 KiB
components/exit-this-page/exit-this-page.mjs 16.34 KiB
components/header/header.mjs 3.21 KiB
components/notification-banner/notification-banner.mjs 4.42 KiB
components/radios/radios.mjs 4.29 KiB
components/skip-link/skip-link.mjs 3.63 KiB
components/tabs/tabs.mjs 8.95 KiB

View stats and visualisations on the review app


Action run for 655af8f

@romaricpascal
Copy link
Member Author

romaricpascal commented Sep 7, 2023

@colinrotherham @domoscargin Ready for re-review, simplified implementation by replacing the return this with throwing errors 😊

@romaricpascal
Copy link
Member Author

romaricpascal commented Sep 7, 2023

Actually no, let me drop that first commit that's not relevant anymore. Sorry 😔 All sorted!

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-4104 September 7, 2023 12:48 Inactive
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.

Throw an error if JavaScript components receive the wrong type of $module
6 participants