Skip to content

feat(config): add customGetElementError config option #452

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

Merged
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
5 changes: 5 additions & 0 deletions src/__tests__/__snapshots__/get-by-errors.js.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`getByLabelText query will throw the custom error returned by config.getElementError 1`] = `"My custom error: Unable to find a label with the text of: TEST QUERY"`;

exports[`getByText query will throw the custom error returned by config.getElementError 1`] = `"My custom error: Unable to find an element with the text: TEST QUERY. This could be because the text is broken up by multiple elements. In this case, you can provide a function for your text matcher to make your matcher more flexible."`;
20 changes: 20 additions & 0 deletions src/__tests__/get-by-errors.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,13 @@
import cases from 'jest-in-case'
import {screen} from '../'
import {configure, getConfig} from '../config'
import {render} from './helpers/test-utils'

const originalConfig = getConfig()
beforeEach(() => {
configure(originalConfig)
})

cases(
'getBy* queries throw an error when there are multiple elements returned',
({name, query, html}) => {
Expand Down Expand Up @@ -39,6 +46,19 @@ cases(
},
)

test.each([['getByText'], ['getByLabelText']])(
'%s query will throw the custom error returned by config.getElementError',
query => {
const getElementError = jest.fn(
(message, _container) => new Error(`My custom error: ${message}`),
)
configure({getElementError})
document.body.innerHTML = '<div>Hello</div>'
expect(() => screen[query]('TEST QUERY')).toThrowErrorMatchingSnapshot()
Copy link
Member

Choose a reason for hiding this comment

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

I would prefer tests that specifically look for the presence/absence of the error message (not a fan of snapshot tests personally) but I'm not gonna block for it. I'll defer to @kentcdodds or someone else on that.

Copy link
Contributor Author

@aw-davidson aw-davidson Feb 17, 2020

Choose a reason for hiding this comment

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

Something like this?

const div = '<div>Hello</div>'
document.body.innerHTML = div
try {
    screen[query]('TEST QUERY'))
} catch (err) {
    expect(err.message.includes(div)).toBe(showLogOnFail)
}

expect(getElementError).toBeCalledTimes(1)
},
)

cases(
'queryBy* queries throw an error when there are multiple elements returned',
({name, query, html}) => {
Expand Down
9 changes: 9 additions & 0 deletions src/config.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import {prettyDOM} from './pretty-dom'

// It would be cleaner for this to live inside './queries', but
// other parts of the code assume that all exports from
// './queries' are query functions.
Expand All @@ -14,6 +16,13 @@ let config = {
asyncWrapper: cb => cb(),
// default value for the `hidden` option in `ByRole` queries
defaultHidden: false,

// called when getBy* queries fail. (message, container) => Error
getElementError(message, container) {
return new Error(
[message, prettyDOM(container)].filter(Boolean).join('\n\n'),
)
},
}

export function configure(newConfig) {
Expand Down
6 changes: 3 additions & 3 deletions src/queries/label-text.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import {getConfig} from '../config'
import {
fuzzyMatches,
matches,
makeNormalizer,
getElementError,
queryAllByAttribute,
makeFindQuery,
makeSingleQuery,
Expand Down Expand Up @@ -104,12 +104,12 @@ function getAllByLabelText(container, text, ...rest) {
if (!els.length) {
const labels = queryAllLabelsByText(container, text, ...rest)
if (labels.length) {
throw getElementError(
throw getConfig().getElementError(
`Found a label with the text of: ${text}, however no form control was found associated to that label. Make sure you're using the "for" attribute or "aria-labelledby" attribute correctly.`,
container,
)
} else {
throw getElementError(
throw getConfig().getElementError(
`Unable to find a label with the text of: ${text}`,
container,
)
Expand Down
14 changes: 6 additions & 8 deletions src/query-helpers.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,9 @@
import {prettyDOM} from './pretty-dom'
import {fuzzyMatches, matches, makeNormalizer} from './matches'
import {waitForElement} from './wait-for-element'

function getElementError(message, container) {
return new Error([message, prettyDOM(container)].filter(Boolean).join('\n\n'))
}
import {getConfig} from './config'

function getMultipleElementsFoundError(message, container) {
return getElementError(
return getConfig().getElementError(
`${message}\n\n(If this is intentional, then use the \`*AllBy*\` variant of the query (like \`queryAllByText\`, \`getAllByText\`, or \`findAllByText\`)).`,
container,
)
Expand Down Expand Up @@ -59,7 +55,10 @@ function makeGetAllQuery(allQuery, getMissingError) {
return (container, ...args) => {
const els = allQuery(container, ...args)
if (!els.length) {
throw getElementError(getMissingError(container, ...args), container)
throw getConfig().getElementError(
getMissingError(container, ...args),
container,
)
}
return els
}
Expand All @@ -86,7 +85,6 @@ function buildQueries(queryAllBy, getMultipleError, getMissingError) {
}

export {
getElementError,
getMultipleElementsFoundError,
queryAllByAttribute,
queryByAttribute,
Expand Down