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

add fireEvent from dom-testing-library #48

Merged
merged 15 commits into from
Apr 10, 2018
74 changes: 64 additions & 10 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ facilitate testing implementation details). Read more about this in
* [Installation](#installation)
* [Usage](#usage)
* [`render`](#render)
* [`renderIntoDocument`](#renderintodocument)
* [`clearDocument`](#cleardocument)
* [`Simulate`](#simulate)
* [`wait`](#wait)
* [`fireEvent(node: HTMLElement, event: Event)`](#fireeventnode-htmlelement-event-event)
Expand Down Expand Up @@ -265,6 +267,27 @@ const usernameInputElement = getByTestId('username-input')
> Learn more about `data-testid`s from the blog post
> ["Making your UI tests resilient to change"][data-testid-blog-post]

### `renderIntoDocument`

Render into `document.body`. Should be used with [clearDocument](#cleardocument)

```javascript
renderIntoDocument(<div>)
```

### `clearDocument`

Clears the `document.body`. Good for preventing memory leaks. Should be used with [renderIntoDocument](#renderintodocument)

```javascript
afterEach(clearDocument)

test('renders into document', () => {
renderIntoDocument(<div>)
// ...
})
```

### `Simulate`

This is simply a re-export from the `Simulate` utility from
Expand Down Expand Up @@ -318,24 +341,55 @@ intervals.

Fire DOM events.

React attaches an event handler on the `document` and handles some DOM events via event delegation (events bubbling up from a `target` to an ancestor). Because of this, your `node` must be in the `document.body` for `fireEvent` to work with React. You can render into the document using the [renderIntoDocument](#renderintodocument) utility. This is an alternative to simulating Synthetic React Events via [Simulate](#simulate). The benefit of using `fireEvent` over `Simulate` is that you are testing real DOM events instead of Synthetic Events. This aligns better with [the Guiding Principles](#guiding-principles).

```javascript
// <button>Submit</button>
fireEvent(
getElementByText('Submit'),
new MouseEvent('click', {
bubbles: true,
cancelable: true,
}),
)
import { renderIntoDocument, clearDocument, render, fireEvent }

// don't forget to clean up the document.body
afterEach(clearDocument)

test('clicks submit button', () => {
const spy = jest.fn();
const { unmount, getByText } render(<button onClick={spy}>Submit</button>)
Copy link
Member

Choose a reason for hiding this comment

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

The = is missing in this assignment.

Also, shouldn't this example be using renderIntoDocument instead of render?


fireEvent(
getByText('Submit'),
new MouseEvent('click', {
bubbles: true, // click events must bubble for React to see it
cancelable: true,
})
)

// don't forget to unmount component so componentWillUnmount can clean up subscriptions
unmount();

expect(spy).toHaveBeenCalledTimes(1);
});
```

#### `fireEvent[eventName](node: HTMLElement, eventInit)`
Copy link
Member

Choose a reason for hiding this comment

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

Could you update this to be:

#### `fireEvent[eventName](node: HTMLElement, eventProperties: Object)`


Convenience methods for firing DOM events. Look [here](./src/events.js) for full list.
Copy link
Member

Choose a reason for hiding this comment

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

Could you update this to say:

Convenience methods for firing DOM events. Check out
[dom-testing-library/src/events.js](https://github.com/kentcdodds/dom-testing-library/blob/master/src/events.js)
for a full list as well as default `eventProperties`.


```javascript
Copy link
Member

Choose a reason for hiding this comment

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

Please update this example to the same in dom-testing-library:

// <button>Submit</button>
const rightClick = {button: 2}
fireEvent.click(getElementByText('Submit'), rightClick)
// default `button` property for click events is set to `0` which is a left click.

// <button>Submit</button>
fireEvent.click(getElementByText('Submit'))
import { renderIntoDocument, clearDocument, render, fireEvent }

// don't forget to clean up the document.body
afterEach(clearDocument)
Copy link
Member

Choose a reason for hiding this comment

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


test('clicks submit button', () => {
const spy = jest.fn();
const { unmount, getByText } render(<button onClick={spy}>Submit</button>)

// click will bubble for React to see it
fireEvent.click(getByText('Submit'))

// don't forget to unmount component so componentWillUnmount can clean up subscriptions
Copy link
Member

Choose a reason for hiding this comment

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

Is this totally required for react to do its job? I'm not certain it's necessary 🤔

If it is then perhaps our clearDocument should actually be called cleanup and do that job for people. If it's required for everyone to do it, and it's not confusing or complicated for us to do it in the library, then we should do it in the library.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ok we can do it in the library, but i think this is even more reason to have cleanup be in an afterEach hook so if a componentWillUnmount throws an error it will be associated with the correct test.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done but kept beforeEach in examples

Copy link
Member

Choose a reason for hiding this comment

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

ok we can do it in the library, but i think this is even more reason to have cleanup be in an afterEach hook so if a componentWillUnmount throws an error it will be associated with the correct test.

This makes a lot of sense. Sorry, but could you change it back? 😅

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

haha np, will change it back

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

unmount();

expect(spy).toHaveBeenCalledTimes(1);
});
```

## `TextMatch`
Expand Down