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

feat: Depreciate toBeInTheDOM with replacement #40

Merged
merged 16 commits into from
Jul 18, 2018
Merged
Show file tree
Hide file tree
Changes from 14 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
72 changes: 34 additions & 38 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ to maintain.
- [Installation](#installation)
- [Usage](#usage)
- [Custom matchers](#custom-matchers)
- [`toBeInTheDOM`](#tobeinthedom)
- [`toBeEmpty`](#tobeempty)
- [`toBeInTheDocument`](#tobeinthedocument)
- [`toContainElement`](#tocontainelement)
- [`toHaveTextContent`](#tohavetextcontent)
- [`toHaveAttribute`](#tohaveattribute)
Expand All @@ -55,6 +55,8 @@ to maintain.
- [`toHaveFocus`](#tohavefocus)
- [`toBeVisible`](#tobevisible)
- [`toBeDisabled`](#tobedisabled)
- [Deprecated matchers](#deprecated-matchers)
- [`toBeInTheDOM`](#tobeinthedom)
- [Inspiration](#inspiration)
- [Other Solutions](#other-solutions)
- [Guiding Principles](#guiding-principles)
Expand Down Expand Up @@ -87,77 +89,61 @@ Alternatively, you can selectively import only the matchers you intend to use,
and extend jest's `expect` yourself:

```javascript
import {toBeInTheDOM, toHaveClass} from 'jest-dom'
import {toBeInTheDocument, toHaveClass} from 'jest-dom'

expect.extend({toBeInTheDOM, toHaveClass})
expect.extend({toBeInTheDocument, toHaveClass})
```

> Note: when using TypeScript, this way of importing matchers won't provide the
> necessary type definitions. More on this [here](https://github.com/gnapse/jest-dom/pull/11#issuecomment-387817459).

## Custom matchers

### `toBeInTheDOM`
### `toBeEmpty`

```typescript
toBeInTheDOM(container?: HTMLElement | SVGElement)
```

This allows you to assert whether an element present in the DOM container or not. If no DOM container is specified it will use the default DOM context.

#### Using the default DOM container

```javascript
// add the custom expect matchers once
import 'jest-dom/extend-expect'

// ...
// <span data-testid="count-value">2</span>
expect(queryByTestId(container, 'count-value')).toBeInTheDOM()
expect(queryByTestId(container, 'count-value1')).not.toBeInTheDOM()
// ...
toBeEmpty()
```

#### Using a specified DOM container
This allows you to assert whether an element has content or not.

```javascript
// add the custom expect matchers once
import 'jest-dom/extend-expect'

// ...
// <span data-testid="ancestor"><span data-testid="descendant"></span></span>
expect(queryByTestId(container, 'descendant')).toBeInTheDOM(
queryByTestId(container, 'ancestor'),
)
expect(queryByTestId(container, 'ancestor')).not.toBeInTheDOM(
queryByTestId(container, 'descendant'),
)
// <span data-testid="not-empty"><span data-testid="empty"></span></span>
expect(queryByTestId(container, 'empty')).toBeEmpty()
expect(queryByTestId(container, 'not-empty')).not.toBeEmpty()
// ...
```

> Note: when using `toBeInTheDOM`, make sure you use a query function
> (like `queryByTestId`) rather than a get function (like `getByTestId`).
> Otherwise the `get*` function could throw an error before your assertion.

### `toBeEmpty`
### `toBeInTheDocument`

```typescript
toBeEmpty()
toBeInTheDocument()
```

This allows you to assert whether an element has content or not.
This allows you to assert whether an element is present in the document or not.

```javascript
// add the custom expect matchers once
import 'jest-dom/extend-expect'

// ...
// <span data-testid="not-empty"><span data-testid="empty"></span></span>
expect(queryByTestId(container, 'empty')).toBeEmpty()
expect(queryByTestId(container, 'not-empty')).not.toBeEmpty()
// document.body.innerHTML = `<span data-testid="html-element"><span>Html Element</span></span><svg data-testid="svg-element"></svg>`

// const htmlElement = document.querySelector('[data-testid="html-element"]')
// const svgElement = document.querySelector('[data-testid="html-element"]')
// const detachedElement = document.createElement('div')

expect(queryByTestId(container, 'count-value').toBeInTheDocument()
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've thought this line was also going to be replaced to use some of the other elements you define above that are indeed in the document.

I'd even insist in changing it merely on the ground that this is referencing a helper function in dom-testing-library, probably since this README was created based on that other library's README.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I am sorry! Great catch!

expect(detacthedElement).not.toBeInTheDocument()
// ...
```

> Note: This will not find detached elements. The element must be added to the document to be found. If you desire to search in a detached element please use: [`toContainElement`](#tocontainelement)

### `toContainElement`

```typescript
Expand Down Expand Up @@ -378,6 +364,16 @@ expect(getByText(container, 'LINK')).not.toBeDisabled()
// ...
```

## Deprecated matchers

### `toBeInTheDOM`

> Note: The differences between `toBeInTheDOM` and `toBeInTheDocument` are significant. Replacing all uses of `toBeInTheDOM` with `toBeInTheDocument` will likely cause unintended consequences in your tests. Please make sure when replacing `toBeInTheDOM` to read through the replacements below to see which use case works better for your needs.

> Please use [`toContainElement`](#tocontainelement) for searching a specific container.

> Please use [`toBeInTheDocument`](#tobeinthedocument) for searching the entire document.

## Inspiration

This whole library was extracted out of Kent C. Dodds' [dom-testing-library][],
Expand Down
4 changes: 4 additions & 0 deletions extend-expect.d.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
declare namespace jest {
interface Matchers<R> {
/**
* @deprecated
*/
toBeInTheDOM(container?: HTMLElement | SVGElement): R
toBeInTheDocument(): R
toBeVisible(): R
toBeEmpty(): R
toBeDisabled(): R
Expand Down
24 changes: 24 additions & 0 deletions src/__tests__/to-be-in-the-document.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
test('.toBeInTheDocument', () => {
document.body.innerHTML = `
<span data-testid="html-element"><span>Html Element</span></span>
<svg data-testid="svg-element"></svg>`

const htmlElement = document.querySelector('[data-testid="html-element"]')
const svgElement = document.querySelector('[data-testid="html-element"]')
const detachedElement = document.createElement('div')
const fakeElement = {thisIsNot: 'an html element'}
const undefinedElement = undefined
const nullElement = null

expect(htmlElement).toBeInTheDocument()
expect(svgElement).toBeInTheDocument()
expect(detachedElement).not.toBeInTheDocument()

// negative test cases wrapped in throwError assertions for coverage.
expect(() => expect(htmlElement).not.toBeInTheDocument()).toThrowError()
expect(() => expect(svgElement).not.toBeInTheDocument()).toThrowError()
expect(() => expect(detachedElement).toBeInTheDocument()).toThrowError()
expect(() => expect(fakeElement).toBeInTheDocument()).toThrowError()
expect(() => expect(undefinedElement).toBeInTheDocument()).toThrowError()
expect(() => expect(nullElement).toBeInTheDocument()).toThrowError()
})
5 changes: 5 additions & 0 deletions src/__tests__/to-be-in-the-dom.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import {render} from './helpers/test-utils'

test('.toBeInTheDOM', () => {
// @deprecated intentionally hiding warnings for test clarity
const spy = jest.spyOn(console, 'warn').mockImplementation(() => {})

const {queryByTestId} = render(`
<span data-testid="count-container">
<span data-testid="count-value"></span>
Expand Down Expand Up @@ -51,4 +54,6 @@ test('.toBeInTheDOM', () => {
expect(() => {
expect(valueElement).toBeInTheDOM(fakeElement)
}).toThrowError()

spy.mockRestore()
})
43 changes: 43 additions & 0 deletions src/__tests__/utils.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import {checkDocumentKey, deprecate} from '../utils'

function matcherMock() {}

test('deprecate', () => {
const spy = jest.spyOn(console, 'warn').mockImplementation(() => {})
const name = 'test'
const replacement = 'test'
const message = `Warning: ${name} has been deprecated and will be removed in future updates.`

deprecate(name, replacement)
expect(spy).toHaveBeenCalledWith(message, replacement)

deprecate(name)
expect(spy).toHaveBeenCalledWith(message, undefined)

spy.mockRestore()
})

test('checkDocumentKey', () => {
const fakeKey = 'fakeKey'
const realKey = 'documentElement'
const badKeyMessage = `${fakeKey} is undefined on document but is required to use ${
matcherMock.name
}.`

const badDocumentMessage = `document is undefined on global but is required to use ${
matcherMock.name
}.`

expect(() =>
checkDocumentKey(document, realKey, matcherMock),
).not.toThrowError()

expect(() => {
checkDocumentKey(document, fakeKey, matcherMock)
}).toThrow(badKeyMessage)

expect(() => {
//eslint-disable-next-line no-undef
checkDocumentKey(undefined, realKey, matcherMock)
}).toThrow(badDocumentMessage)
})
2 changes: 2 additions & 0 deletions src/index.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {toBeInTheDOM} from './to-be-in-the-dom'
import {toBeInTheDocument} from './to-be-in-the-document'
import {toBeEmpty} from './to-be-empty'
import {toContainElement} from './to-contain-element'
import {toHaveTextContent} from './to-have-text-content'
Expand All @@ -11,6 +12,7 @@ import {toBeDisabled} from './to-be-disabled'

export {
toBeInTheDOM,
toBeInTheDocument,
toBeEmpty,
toContainElement,
toHaveTextContent,
Expand Down
25 changes: 25 additions & 0 deletions src/to-be-in-the-document.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import {matcherHint, printReceived} from 'jest-matcher-utils'
import {checkHtmlElement, checkDocumentKey} from './utils'

export function toBeInTheDocument(element) {
checkHtmlElement(element, toBeInTheDocument, this)
checkDocumentKey(global.document, 'documentElement', toBeInTheDocument)

return {
pass: document.documentElement.contains(element),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we check whether document and document.documentElement exist? I know jsdom will have them, but if I accidentally used this matcher in the node environment, it would be nice to get a descriptive error message rather than "cannot read property 'documentElement' of undefined".

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hrm, sounds good, agreed on developer experience. It would be nice to get a clear error. Does the following sound good @jgoz?

document is undefined on global but is required for the toBeInTheDocument matcher.

documentElement is undefined on document but is required for the toBeInTheDocument matcher.

Copy link
Collaborator

Choose a reason for hiding this comment

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

👍 the first message should be good enough for both cases.

message: () => {
return [
matcherHint(
`${this.isNot ? '.not' : ''}.toBeInTheDocument`,
'element',
'',
),
'',
'Received:',
` ${printReceived(
element.hasChildNodes() ? element.cloneNode(false) : element,
)}`,
].join('\n')
},
}
}
7 changes: 6 additions & 1 deletion src/to-be-in-the-dom.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
import {matcherHint, printReceived} from 'jest-matcher-utils'
import {checkHtmlElement} from './utils'
import {checkHtmlElement, deprecate} from './utils'

export function toBeInTheDOM(element, container) {
deprecate(
'toBeInTheDOM',
'Please use toBeInTheDocument for searching the entire document and toContainElement for searching a specific container.',
)

if (element) {
checkHtmlElement(element, toBeInTheDOM, this)
}
Expand Down
44 changes: 43 additions & 1 deletion src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,39 @@ function checkHtmlElement(htmlElement, ...args) {
}
}

class InvalidDocumentError extends Error {
constructor(message, matcherFn) {
super()

/* istanbul ignore next */
if (Error.captureStackTrace) {
Error.captureStackTrace(this, matcherFn)
}

this.message = message
}
}

function checkDocumentKey(document, key, matcherFn) {
if (typeof document === 'undefined') {
throw new InvalidDocumentError(
`document is undefined on global but is required to use ${
matcherFn.name
}.`,
matcherFn,
)
}

if (typeof document[key] === 'undefined') {
throw new InvalidDocumentError(
`${key} is undefined on document but is required to use ${
matcherFn.name
}.`,
matcherFn,
)
}
}

function display(value) {
return typeof value === 'string' ? value : stringify(value)
}
Expand All @@ -66,4 +99,13 @@ function matches(textToMatch, node, matcher) {
}
}

export {checkHtmlElement, getMessage, matches}
function deprecate(name, replacementText) {
// Notify user that they are using deprecated functionality.
// eslint-disable-next-line no-console
console.warn(
`Warning: ${name} has been deprecated and will be removed in future updates.`,
replacementText,
)
}

export {checkDocumentKey, checkHtmlElement, deprecate, getMessage, matches}