From 67890f0a3d734569e87facec16ec7c632e2c16d7 Mon Sep 17 00:00:00 2001 From: Patrick Cartlidge Date: Mon, 18 Nov 2024 14:42:10 +0000 Subject: [PATCH 1/4] Remove `GOVUKFrontend` prefix from `Component` - `GOVUKFrontendComponent` is now `Component`. Renamed `govuk-frontend-component.mjs` to `component.mjs` and the associated test. Changed references to `GOVUKFrontendComponent` to `Component` in the test. - Created new `govuk-frontend-component.mjs` that imports `Component` and exports it as `GOVUKFrontendComponent` to avoid a breaking change. --- packages/govuk-frontend/src/govuk/all.mjs | 2 +- ...sdom.test.mjs => component.jsdom.test.mjs} | 8 +- .../govuk-frontend/src/govuk/component.mjs | 113 +++++++++++++++++ .../src/govuk/govuk-frontend-component.mjs | 114 +----------------- .../src/govuk/init.jsdom.test.mjs | 6 +- 5 files changed, 122 insertions(+), 121 deletions(-) rename packages/govuk-frontend/src/govuk/{govuk-frontend-component.jsdom.test.mjs => component.jsdom.test.mjs} (84%) create mode 100644 packages/govuk-frontend/src/govuk/component.mjs diff --git a/packages/govuk-frontend/src/govuk/all.mjs b/packages/govuk-frontend/src/govuk/all.mjs index 684d6984c9..ab808c96d4 100644 --- a/packages/govuk-frontend/src/govuk/all.mjs +++ b/packages/govuk-frontend/src/govuk/all.mjs @@ -14,7 +14,7 @@ export { SkipLink } from './components/skip-link/skip-link.mjs' export { Tabs } from './components/tabs/tabs.mjs' export { initAll, createAll } from './init.mjs' export { isSupported } from './common/index.mjs' -export { GOVUKFrontendComponent as Component } from './govuk-frontend-component.mjs' +export { Component } from './component.mjs' export { ConfigurableComponent } from './common/configuration.mjs' /** diff --git a/packages/govuk-frontend/src/govuk/govuk-frontend-component.jsdom.test.mjs b/packages/govuk-frontend/src/govuk/component.jsdom.test.mjs similarity index 84% rename from packages/govuk-frontend/src/govuk/govuk-frontend-component.jsdom.test.mjs rename to packages/govuk-frontend/src/govuk/component.jsdom.test.mjs index 491981373a..6a97c7b1df 100644 --- a/packages/govuk-frontend/src/govuk/govuk-frontend-component.jsdom.test.mjs +++ b/packages/govuk-frontend/src/govuk/component.jsdom.test.mjs @@ -1,7 +1,7 @@ +import { Component } from './component.mjs' import { SupportError } from './errors/index.mjs' -import { GOVUKFrontendComponent } from './govuk-frontend-component.mjs' -describe('GOVUKFrontendComponent', () => { +describe('Component', () => { describe('checkSupport()', () => { beforeEach(() => { // Jest does not tidy the JSDOM document between tests @@ -10,7 +10,7 @@ describe('GOVUKFrontendComponent', () => { }) describe('default implementation', () => { - class ServiceComponent extends GOVUKFrontendComponent { + class ServiceComponent extends Component { static moduleName = 'app-service-component' } @@ -27,7 +27,7 @@ describe('GOVUKFrontendComponent', () => { describe('when overriden', () => { it('Allows child classes to define their own condition for support', () => { - class ServiceComponent extends GOVUKFrontendComponent { + class ServiceComponent extends Component { static moduleName = 'app-service-component' static checkSupport() { diff --git a/packages/govuk-frontend/src/govuk/component.mjs b/packages/govuk-frontend/src/govuk/component.mjs new file mode 100644 index 0000000000..07f0aac274 --- /dev/null +++ b/packages/govuk-frontend/src/govuk/component.mjs @@ -0,0 +1,113 @@ +import { isInitialised, isSupported } from './common/index.mjs' +import { ElementError, InitError, SupportError } from './errors/index.mjs' + +/** + * Base Component class + * + * Centralises the behaviours shared by our components + * + * @virtual + * @template {Element} [RootElementType=HTMLElement] + */ +export class Component { + /** + * @type {typeof Element} + */ + static elementType = HTMLElement + + // allows Typescript user to work around the lack of types + // in GOVUKFrontend package, Typescript is not aware of $root + // in components that extend GOVUKFrontendComponent + /** + * Returns the root element of the component + * + * @protected + * @returns {RootElementType} - the root element of component + */ + get $root() { + return this._$root + } + + /** + * @protected + * @type {RootElementType} + */ + _$root + + /** + * Constructs a new component, validating that GOV.UK Frontend is supported + * + * @internal + * @param {Element | null} [$root] - HTML element to use for component + */ + constructor($root) { + const childConstructor = /** @type {ChildClassConstructor} */ ( + this.constructor + ) + + // TypeScript does not enforce that inheriting classes will define a `moduleName` + // (even if we add a `@virtual` `static moduleName` property to this class). + // While we trust users to do this correctly, we do a little check to provide them + // a helpful error message. + // + // After this, we'll be sure that `childConstructor` has a `moduleName` + // as expected of the `ChildClassConstructor` we've cast `this.constructor` to. + if (typeof childConstructor.moduleName !== 'string') { + throw new InitError(`\`moduleName\` not defined in component`) + } + + if (!($root instanceof childConstructor.elementType)) { + throw new ElementError({ + element: $root, + component: childConstructor, + identifier: 'Root element (`$root`)', + expectedType: childConstructor.elementType.name + }) + } else { + this._$root = /** @type {RootElementType} */ ($root) + } + + childConstructor.checkSupport() + + this.checkInitialised() + + const moduleName = childConstructor.moduleName + + this.$root.setAttribute(`data-${moduleName}-init`, '') + } + + /** + * Validates whether component is already initialised + * + * @private + * @throws {InitError} when component is already initialised + */ + checkInitialised() { + const constructor = /** @type {ChildClassConstructor} */ (this.constructor) + const moduleName = constructor.moduleName + + if (moduleName && isInitialised(this.$root, moduleName)) { + throw new InitError(constructor) + } + } + + /** + * Validates whether components are supported + * + * @throws {SupportError} when the components are not supported + */ + static checkSupport() { + if (!isSupported()) { + throw new SupportError() + } + } +} + +/** + * @typedef ChildClass + * @property {string} moduleName - The module name that'll be looked for in the DOM when initialising the component + */ + +/** + * @typedef {typeof Component & ChildClass} ChildClassConstructor + */ diff --git a/packages/govuk-frontend/src/govuk/govuk-frontend-component.mjs b/packages/govuk-frontend/src/govuk/govuk-frontend-component.mjs index 2e389ddd9a..0abfd6c6f5 100644 --- a/packages/govuk-frontend/src/govuk/govuk-frontend-component.mjs +++ b/packages/govuk-frontend/src/govuk/govuk-frontend-component.mjs @@ -1,113 +1 @@ -import { isInitialised, isSupported } from './common/index.mjs' -import { ElementError, InitError, SupportError } from './errors/index.mjs' - -/** - * Base Component class - * - * Centralises the behaviours shared by our components - * - * @virtual - * @template {Element} [RootElementType=HTMLElement] - */ -export class GOVUKFrontendComponent { - /** - * @type {typeof Element} - */ - static elementType = HTMLElement - - // allows Typescript user to work around the lack of types - // in GOVUKFrontend package, Typescript is not aware of $root - // in components that extend GOVUKFrontendComponent - /** - * Returns the root element of the component - * - * @protected - * @returns {RootElementType} - the root element of component - */ - get $root() { - return this._$root - } - - /** - * @protected - * @type {RootElementType} - */ - _$root - - /** - * Constructs a new component, validating that GOV.UK Frontend is supported - * - * @internal - * @param {Element | null} [$root] - HTML element to use for component - */ - constructor($root) { - const childConstructor = /** @type {ChildClassConstructor} */ ( - this.constructor - ) - - // TypeScript does not enforce that inheriting classes will define a `moduleName` - // (even if we add a `@virtual` `static moduleName` property to this class). - // While we trust users to do this correctly, we do a little check to provide them - // a helpful error message. - // - // After this, we'll be sure that `childConstructor` has a `moduleName` - // as expected of the `ChildClassConstructor` we've cast `this.constructor` to. - if (typeof childConstructor.moduleName !== 'string') { - throw new InitError(`\`moduleName\` not defined in component`) - } - - if (!($root instanceof childConstructor.elementType)) { - throw new ElementError({ - element: $root, - component: childConstructor, - identifier: 'Root element (`$root`)', - expectedType: childConstructor.elementType.name - }) - } else { - this._$root = /** @type {RootElementType} */ ($root) - } - - childConstructor.checkSupport() - - this.checkInitialised() - - const moduleName = childConstructor.moduleName - - this.$root.setAttribute(`data-${moduleName}-init`, '') - } - - /** - * Validates whether component is already initialised - * - * @private - * @throws {InitError} when component is already initialised - */ - checkInitialised() { - const constructor = /** @type {ChildClassConstructor} */ (this.constructor) - const moduleName = constructor.moduleName - - if (moduleName && isInitialised(this.$root, moduleName)) { - throw new InitError(constructor) - } - } - - /** - * Validates whether components are supported - * - * @throws {SupportError} when the components are not supported - */ - static checkSupport() { - if (!isSupported()) { - throw new SupportError() - } - } -} - -/** - * @typedef ChildClass - * @property {string} moduleName - The module name that'll be looked for in the DOM when initialising the component - */ - -/** - * @typedef {typeof GOVUKFrontendComponent & ChildClass} ChildClassConstructor - */ +export { Component as GOVUKFrontendComponent } from './component.mjs' diff --git a/packages/govuk-frontend/src/govuk/init.jsdom.test.mjs b/packages/govuk-frontend/src/govuk/init.jsdom.test.mjs index b7206342f4..570721fc94 100644 --- a/packages/govuk-frontend/src/govuk/init.jsdom.test.mjs +++ b/packages/govuk-frontend/src/govuk/init.jsdom.test.mjs @@ -4,7 +4,7 @@ import { } from '@govuk-frontend/lib/names' import * as GOVUKFrontend from './all.mjs' -import { GOVUKFrontendComponent } from './govuk-frontend-component.mjs' +import { Component } from './component.mjs' import { initAll, createAll } from './init.mjs' // Annoyingly these don't get hoisted if done in a loop @@ -227,14 +227,14 @@ describe('createAll', () => { document.body.outerHTML = '' }) - class MockComponent extends GOVUKFrontendComponent { + class MockComponent extends Component { constructor(...args) { super(...args) this.args = args } static checkSupport() { - GOVUKFrontendComponent.checkSupport() + Component.checkSupport() } static moduleName = 'mock-component' From b3454c6d746e26f1231bc0a19927c3054e53d1ad Mon Sep 17 00:00:00 2001 From: Patrick Cartlidge Date: Mon, 18 Nov 2024 15:07:27 +0000 Subject: [PATCH 2/4] Update components to import `Component` Components that extended `GOVUKFrontendComponent` now extend `Component` from `component.mjs`. --- .../src/govuk/components/checkboxes/checkboxes.mjs | 4 ++-- .../govuk-frontend/src/govuk/components/header/header.mjs | 4 ++-- .../govuk-frontend/src/govuk/components/radios/radios.mjs | 4 ++-- .../components/service-navigation/service-navigation.mjs | 4 ++-- .../src/govuk/components/skip-link/skip-link.mjs | 6 +++--- packages/govuk-frontend/src/govuk/components/tabs/tabs.mjs | 4 ++-- 6 files changed, 13 insertions(+), 13 deletions(-) diff --git a/packages/govuk-frontend/src/govuk/components/checkboxes/checkboxes.mjs b/packages/govuk-frontend/src/govuk/components/checkboxes/checkboxes.mjs index 36e51e5f5c..a2168d252e 100644 --- a/packages/govuk-frontend/src/govuk/components/checkboxes/checkboxes.mjs +++ b/packages/govuk-frontend/src/govuk/components/checkboxes/checkboxes.mjs @@ -1,12 +1,12 @@ +import { Component } from '../../component.mjs' import { ElementError } from '../../errors/index.mjs' -import { GOVUKFrontendComponent } from '../../govuk-frontend-component.mjs' /** * Checkboxes component * * @preserve */ -export class Checkboxes extends GOVUKFrontendComponent { +export class Checkboxes extends Component { /** @private */ $inputs diff --git a/packages/govuk-frontend/src/govuk/components/header/header.mjs b/packages/govuk-frontend/src/govuk/components/header/header.mjs index 1609f93b13..3ac0177494 100644 --- a/packages/govuk-frontend/src/govuk/components/header/header.mjs +++ b/packages/govuk-frontend/src/govuk/components/header/header.mjs @@ -1,13 +1,13 @@ import { getBreakpoint } from '../../common/index.mjs' +import { Component } from '../../component.mjs' import { ElementError } from '../../errors/index.mjs' -import { GOVUKFrontendComponent } from '../../govuk-frontend-component.mjs' /** * Header component * * @preserve */ -export class Header extends GOVUKFrontendComponent { +export class Header extends Component { /** @private */ $menuButton diff --git a/packages/govuk-frontend/src/govuk/components/radios/radios.mjs b/packages/govuk-frontend/src/govuk/components/radios/radios.mjs index d8ece9a966..d7005a1e7e 100644 --- a/packages/govuk-frontend/src/govuk/components/radios/radios.mjs +++ b/packages/govuk-frontend/src/govuk/components/radios/radios.mjs @@ -1,12 +1,12 @@ +import { Component } from '../../component.mjs' import { ElementError } from '../../errors/index.mjs' -import { GOVUKFrontendComponent } from '../../govuk-frontend-component.mjs' /** * Radios component * * @preserve */ -export class Radios extends GOVUKFrontendComponent { +export class Radios extends Component { /** @private */ $inputs diff --git a/packages/govuk-frontend/src/govuk/components/service-navigation/service-navigation.mjs b/packages/govuk-frontend/src/govuk/components/service-navigation/service-navigation.mjs index 529e14f25c..b96ce04d35 100644 --- a/packages/govuk-frontend/src/govuk/components/service-navigation/service-navigation.mjs +++ b/packages/govuk-frontend/src/govuk/components/service-navigation/service-navigation.mjs @@ -1,13 +1,13 @@ import { getBreakpoint } from '../../common/index.mjs' +import { Component } from '../../component.mjs' import { ElementError } from '../../errors/index.mjs' -import { GOVUKFrontendComponent } from '../../govuk-frontend-component.mjs' /** * Service Navigation component * * @preserve */ -export class ServiceNavigation extends GOVUKFrontendComponent { +export class ServiceNavigation extends Component { /** @private */ $menuButton diff --git a/packages/govuk-frontend/src/govuk/components/skip-link/skip-link.mjs b/packages/govuk-frontend/src/govuk/components/skip-link/skip-link.mjs index 5dd9ce8bf5..2f70283cca 100644 --- a/packages/govuk-frontend/src/govuk/components/skip-link/skip-link.mjs +++ b/packages/govuk-frontend/src/govuk/components/skip-link/skip-link.mjs @@ -1,14 +1,14 @@ import { getFragmentFromUrl, setFocus } from '../../common/index.mjs' +import { Component } from '../../component.mjs' import { ElementError } from '../../errors/index.mjs' -import { GOVUKFrontendComponent } from '../../govuk-frontend-component.mjs' /** * Skip link component * * @preserve - * @augments GOVUKFrontendComponent + * @augments Component */ -export class SkipLink extends GOVUKFrontendComponent { +export class SkipLink extends Component { static elementType = HTMLAnchorElement /** diff --git a/packages/govuk-frontend/src/govuk/components/tabs/tabs.mjs b/packages/govuk-frontend/src/govuk/components/tabs/tabs.mjs index 271a32cadf..993e9152b7 100644 --- a/packages/govuk-frontend/src/govuk/components/tabs/tabs.mjs +++ b/packages/govuk-frontend/src/govuk/components/tabs/tabs.mjs @@ -1,13 +1,13 @@ import { getBreakpoint, getFragmentFromUrl } from '../../common/index.mjs' +import { Component } from '../../component.mjs' import { ElementError } from '../../errors/index.mjs' -import { GOVUKFrontendComponent } from '../../govuk-frontend-component.mjs' /** * Tabs component * * @preserve */ -export class Tabs extends GOVUKFrontendComponent { +export class Tabs extends Component { /** @private */ $tabs From 3b9ace6fa96fe8f4ddae5e2654fe4a3c42903972 Mon Sep 17 00:00:00 2001 From: Patrick Cartlidge Date: Wed, 20 Nov 2024 12:31:31 +0000 Subject: [PATCH 3/4] Update `ConfigurableComponent` to use `Component` --- .../govuk-frontend/src/govuk/common/configuration.mjs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/govuk-frontend/src/govuk/common/configuration.mjs b/packages/govuk-frontend/src/govuk/common/configuration.mjs index 11a6a57d66..63947cd1f6 100644 --- a/packages/govuk-frontend/src/govuk/common/configuration.mjs +++ b/packages/govuk-frontend/src/govuk/common/configuration.mjs @@ -1,5 +1,5 @@ +import { Component } from '../component.mjs' import { ConfigError } from '../errors/index.mjs' -import { GOVUKFrontendComponent } from '../govuk-frontend-component.mjs' import { isObject, formatErrorMessage } from './index.mjs' @@ -13,9 +13,9 @@ export const configOverride = Symbol.for('configOverride') * @virtual * @template {ObjectNested} [ConfigurationType={}] * @template {Element & { dataset: DOMStringMap }} [RootElementType=HTMLElement] - * @augments GOVUKFrontendComponent + * @augments Component */ -export class ConfigurableComponent extends GOVUKFrontendComponent { +export class ConfigurableComponent extends Component { /** * configOverride * @@ -354,5 +354,5 @@ export function extractConfigByNamespace(schema, dataset, namespace) { /** * @template {ObjectNested} [ConfigurationType={}] - * @typedef {typeof GOVUKFrontendComponent & ChildClass} ChildClassConstructor + * @typedef {typeof Component & ChildClass} ChildClassConstructor */ From 95eb0d4bc82b0813b738547f3f0bb2ee2a9bb7a7 Mon Sep 17 00:00:00 2001 From: Romaric Pascal Date: Thu, 21 Nov 2024 12:43:22 +0000 Subject: [PATCH 4/4] Add deprecated JavaScript files to package build Because they're not imported by `all.mjs` deprecated JavaScript files were not included in the package (in which they need to remain until the next breaking release). This adds a module to store the list of deprecated JavaScript files and helper functions that are then used: - in the Gulp configuration, to add a new task building each deprecated file individually - in the Rollup configuration, to prevent the bundled output of the deprecated files as all we want is for them to be in the package as a module Co-authored-by: Patrick Cartlidge Co-authored-by: Brett Kyle --- docs/contributing/managing-change.md | 25 +++++++++++ .../govuk-frontend/rollup.publish.config.mjs | 30 ++++++++----- .../src/govuk/govuk-frontend-component.mjs | 5 +++ .../tasks/config/deprecated-scripts.mjs | 45 +++++++++++++++++++ packages/govuk-frontend/tasks/scripts.mjs | 19 ++++++++ 5 files changed, 112 insertions(+), 12 deletions(-) create mode 100644 packages/govuk-frontend/tasks/config/deprecated-scripts.mjs diff --git a/docs/contributing/managing-change.md b/docs/contributing/managing-change.md index da640ffb08..7c6eb31c2b 100644 --- a/docs/contributing/managing-change.md +++ b/docs/contributing/managing-change.md @@ -122,6 +122,31 @@ If possible, update the mixin or function to maintain the existing functionality } ``` +### Deprecating a JavaScript file being removed + +Removing a JavaScript file may happen because the module is no longer needed, or has been moved to another place in the project. + +Awaiting for the next breaking, the file needs to remain included in the built package, marked as deprecated. + +1. To ensure the file remains in the package, add its path within `src/govuk` to the list of paths in `packages/tasks/config/deprecated-scripts.mjs`. For example: + +```mjs +export const deprecatedFilesPaths = [ + 'govuk-frontend-component.mjs' +] +``` + +This will build the file individually when creating the package, as it is no longer being discovered automatically by Rollup when building `all.mjs`. + +2. To mark the file as deprecated, add the following JSDoc comment at the top of the file: + +```js +/** + * @deprecated - Optionally describe where the file has been moved to or why it's been removed + * @module + */ +``` + ## Renaming things When renaming things, keep the old name available as an alias and mark it as deprecated, following the steps above to [make sure we remember to remove the deprecated feature](#make-sure-we-remember-to-remove-the-deprecated-feature). diff --git a/packages/govuk-frontend/rollup.publish.config.mjs b/packages/govuk-frontend/rollup.publish.config.mjs index 1e7be8ddf4..5b706f51ca 100644 --- a/packages/govuk-frontend/rollup.publish.config.mjs +++ b/packages/govuk-frontend/rollup.publish.config.mjs @@ -3,6 +3,8 @@ import { babel } from '@rollup/plugin-babel' import replace from '@rollup/plugin-replace' import { defineConfig } from 'rollup' +import { isDeprecated } from './tasks/config/deprecated-scripts.mjs' + /** * Rollup config for npm publish */ @@ -28,26 +30,30 @@ export default defineConfig(({ i: input }) => ({ * ECMAScript (ES) module bundles for browser