Skip to content

Commit

Permalink
Merge pull request #5358 from alphagov/use-formaterrormessage
Browse files Browse the repository at this point in the history
Make components use `formatErrorMessage` for error messages
  • Loading branch information
romaricpascal authored Oct 4, 2024
2 parents fbb15b2 + 2e1f395 commit ab58769
Show file tree
Hide file tree
Showing 15 changed files with 55 additions and 44 deletions.
2 changes: 1 addition & 1 deletion src/govuk/components/accordion/accordion.puppeteer.test.js
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
2 changes: 1 addition & 1 deletion src/govuk/components/button/button.puppeteer.test.js
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
8 changes: 6 additions & 2 deletions src/govuk/components/character-count/character-count.mjs
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
2 changes: 1 addition & 1 deletion src/govuk/components/header/header.puppeteer.test.js
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
2 changes: 1 addition & 1 deletion src/govuk/components/radios/radios.puppeteer.test.js
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
3 changes: 1 addition & 2 deletions src/govuk/components/skip-link/skip-link.puppeteer.test.js
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
2 changes: 1 addition & 1 deletion src/govuk/components/tabs/tabs.puppeteer.test.js
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 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 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 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

0 comments on commit ab58769

Please sign in to comment.