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

Make components use formatErrorMessage for error messages #5358

Merged
merged 3 commits into from
Oct 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -725,7 +725,7 @@ describe('/components/accordion', () => {
).rejects.toMatchObject({
name: 'InitError',
message:
'Root element (`$root`) already initialised (`govuk-accordion`)'
'govuk-accordion: Root element (`$root`) already initialised'
})
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,7 @@ describe('/components/button', () => {
})
).rejects.toMatchObject({
name: 'InitError',
message: 'Root element (`$root`) already initialised (`govuk-button`)'
message: 'govuk-button: Root element (`$root`) already initialised'
})
})

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
import { closestAttributeValue } from '../../common/closest-attribute-value.mjs'
import { mergeConfigs, validateConfig } from '../../common/index.mjs'
import {
formatErrorMessage,
mergeConfigs,
validateConfig
} from '../../common/index.mjs'
import { normaliseDataset } from '../../common/normalise-dataset.mjs'
import { ConfigError, ElementError } from '../../errors/index.mjs'
import { GOVUKFrontendComponent } from '../../govuk-frontend-component.mjs'
Expand Down Expand Up @@ -115,7 +119,7 @@ export class CharacterCount extends GOVUKFrontendComponent {
// Check for valid config
const errors = validateConfig(CharacterCount.schema, this.config)
if (errors[0]) {
throw new ConfigError(`Character count: ${errors[0]}`)
throw new ConfigError(formatErrorMessage(CharacterCount, errors[0]))
}

this.i18n = new I18n(this.config.i18n, {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -825,7 +825,7 @@ describe('Character count', () => {
).rejects.toMatchObject({
name: 'InitError',
message:
'Root element (`$root`) already initialised (`govuk-character-count`)'
'govuk-character-count: Root element (`$root`) already initialised'
})
})

Expand Down Expand Up @@ -931,7 +931,7 @@ describe('Character count', () => {
cause: {
name: 'ConfigError',
message:
'Character count: Either "maxlength" or "maxwords" must be provided'
'govuk-character-count: Either "maxlength" or "maxwords" must be provided'
}
})
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ describe('Checkboxes', () => {
).rejects.toMatchObject({
name: 'InitError',
message:
'Root element (`$root`) already initialised (`govuk-checkboxes`)'
'govuk-checkboxes: Root element (`$root`) already initialised'
})
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ describe('Error Summary', () => {
).rejects.toMatchObject({
name: 'InitError',
message:
'Root element (`$root`) already initialised (`govuk-error-summary`)'
'govuk-error-summary: Root element (`$root`) already initialised'
})
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ describe('/components/exit-this-page', () => {
).rejects.toMatchObject({
name: 'InitError',
message:
'Root element (`$root`) already initialised (`govuk-exit-this-page`)'
'govuk-exit-this-page: Root element (`$root`) already initialised'
})
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ describe('Header navigation', () => {
})
).rejects.toMatchObject({
name: 'InitError',
message: 'Root element (`$root`) already initialised (`govuk-header`)'
message: 'govuk-header: Root element (`$root`) already initialised'
})
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ describe('Notification banner', () => {
).rejects.toMatchObject({
name: 'InitError',
message:
'Root element (`$root`) already initialised (`govuk-notification-banner`)'
'govuk-notification-banner: Root element (`$root`) already initialised'
})
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ describe('Radios', () => {
})
).rejects.toMatchObject({
name: 'InitError',
message: 'Root element (`$root`) already initialised (`govuk-radios`)'
message: 'govuk-radios: Root element (`$root`) already initialised'
})
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,8 +139,7 @@ describe('Skip Link', () => {
})
).rejects.toMatchObject({
name: 'InitError',
message:
'Root element (`$root`) already initialised (`govuk-skip-link`)'
message: 'govuk-skip-link: Root element (`$root`) already initialised'
})
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ describe('/components/tabs', () => {
})
).rejects.toMatchObject({
name: 'InitError',
message: 'Root element (`$root`) already initialised (`govuk-tabs`)'
message: 'govuk-tabs: Root element (`$root`) already initialised'
})
})

Expand Down
16 changes: 6 additions & 10 deletions packages/govuk-frontend/src/govuk/errors/index.jsdom.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -56,25 +56,21 @@ describe('errors', () => {

describe('InitError', () => {
it('is an instance of GOVUKFrontendError', () => {
expect(new InitError('govuk-accordion')).toBeInstanceOf(
GOVUKFrontendError
)
expect(new InitError(Accordion)).toBeInstanceOf(GOVUKFrontendError)
})

it('has its own name set', () => {
expect(new InitError('govuk-accordion').name).toBe('InitError')
expect(new InitError(Accordion).name).toBe('InitError')
})

it('provides feedback for modules already initialised', () => {
expect(new InitError('govuk-accordion').message).toBe(
'Root element (`$root`) already initialised (`govuk-accordion`)'
expect(new InitError(Accordion).message).toBe(
'govuk-accordion: Root element (`$root`) already initialised'
)
})

it('provides feedback when no module name is provided', () => {
expect(new InitError(undefined, 'Accordion').message).toBe(
'moduleName not defined in component (`Accordion`)'
)
it('allows a custom message to be provided', () => {
expect(new InitError('custom message').message).toBe('custom message')
})
})

Expand Down
25 changes: 15 additions & 10 deletions packages/govuk-frontend/src/govuk/errors/index.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -107,17 +107,18 @@ export class InitError extends GOVUKFrontendError {

/**
* @internal
* @param {string|undefined} moduleName - name of the component module
* @param {string} [className] - name of the component module
* @param {ComponentWithModuleName | string} componentOrMessage - name of the component module
*/
constructor(moduleName, className) {
let errorText = `moduleName not defined in component (\`${className}\`)`
constructor(componentOrMessage) {
const message =
typeof componentOrMessage === 'string'
? componentOrMessage
: formatErrorMessage(
componentOrMessage,
`Root element (\`$root\`) already initialised`
)

if (typeof moduleName === 'string') {
errorText = `Root element (\`$root\`) already initialised (\`${moduleName}\`)`
}

super(errorText)
super(message)
}
}

Expand All @@ -129,5 +130,9 @@ export class InitError extends GOVUKFrontendError {
* @property {string} identifier - An identifier that'll let the user understand which element has an error. This is whatever makes the most sense
* @property {Element | null} [element] - The element in error
* @property {string} [expectedType] - The type that was expected for the identifier
* @property {import('../common/index.mjs').ComponentWithModuleName} component - Component throwing the error
* @property {ComponentWithModuleName} component - Component throwing the error
*/

/**
* @typedef {import('../common/index.mjs').ComponentWithModuleName} ComponentWithModuleName
*/
25 changes: 16 additions & 9 deletions packages/govuk-frontend/src/govuk/govuk-frontend-component.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,24 @@ export class GOVUKFrontendComponent {
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`)
}

childConstructor.checkSupport()

this.checkInitialised($root)

const moduleName = childConstructor.moduleName

if (typeof moduleName === 'string') {
moduleName && $root?.setAttribute(`data-${moduleName}-init`, '')
} else {
throw new InitError(moduleName)
}
$root?.setAttribute(`data-${moduleName}-init`, '')
}

/**
Expand All @@ -41,11 +48,11 @@ export class GOVUKFrontendComponent {
* @throws {InitError} when component is already initialised
*/
checkInitialised($root) {
const moduleName = /** @type {ChildClassConstructor} */ (this.constructor)
.moduleName
const constructor = /** @type {ChildClassConstructor} */ (this.constructor)
const moduleName = constructor.moduleName

if ($root && moduleName && isInitialised($root, moduleName)) {
throw new InitError(moduleName)
throw new InitError(constructor)
}
}

Expand All @@ -63,7 +70,7 @@ export class GOVUKFrontendComponent {

/**
* @typedef ChildClass
* @property {string} [moduleName] - The module name that'll be looked for in the DOM when initialising the component
* @property {string} moduleName - The module name that'll be looked for in the DOM when initialising the component
*/

/**
Expand Down