Skip to content

Simulate.click on a submit button #54

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

Closed
thchia opened this issue Apr 13, 2018 · 19 comments
Closed

Simulate.click on a submit button #54

thchia opened this issue Apr 13, 2018 · 19 comments
Labels
documentation A docs improvement is needed. help wanted Extra attention is needed

Comments

@thchia
Copy link
Contributor

thchia commented Apr 13, 2018

  • react-testing-library version: 2.1.1
  • node version: 8.9.3
  • npm (or yarn) version: yarn 1.3.2

Relevant code or config

const Component = () => {
  return <form onSubmit={handleSubmit}>
    <button type="submit">Submit</button>
  </form>
}

What you did:

const submitButton = getByText('Submit')

// does not work
Simulate.click(submitButton)

// works
Simulate.submit(submitButton)

What happened:

Submit buttons cannot be clicked using Simulate.click.

Problem description:

This is not a bug as far as Simulate is concerned, but I think that in the spirit of the Guiding Principles, resorting to calling .submit should be discouraged. Users click submit buttons, they do not invoke the submit event.

Certain libraries like react-final-form rely on the submit event to display validation errors. If for example, you have logic to disable a submit button from being clickable, calling Simulate.submit will ignore this and call the submit event anyway. This can result in incorrect assertions.

Suggested solution:

Add a quick caveat in the docs under Simulate. Note that calling

submitButton.dispatchEvent(new MouseEvent('click'))

can solve this issue. I am good to submit a PR if this is deemed worth it.

EDIT: I guess this is technically covered in the fireEvent use case...

@kentcdodds
Copy link
Member

I agree we should add a note in the docs under Simulate. And the suggested solution should be to use fireEvent 👍 Thanks!

@kentcdodds kentcdodds added help wanted Extra attention is needed documentation A docs improvement is needed. labels Apr 13, 2018
@antsmartian
Copy link
Collaborator

antsmartian commented Apr 13, 2018

@kentcdodds I was about to talk about this sometime back... We have form.js line : https://github.com/kentcdodds/react-testing-library/blob/master/src/__tests__/forms.js#L51 where we use submit method from Simulate, may be we need to add comments over there and say this is actually possible via fireEvent API or may be remove simulate and add fireEvent.click.

I was just thinking like does Simulate is really required, provided we have fireEvent in place? May be there is something that Simulate does which our fireEvent can't do? As suggested, we need to add docs on Simulate vs fireEvent and their usages.

@kentcdodds
Copy link
Member

May be there is something that Simulate does which our fireEvent can't do?

Yes, Simulate can work with components which are not rendered into the document, fireEvent must be used on components that are rendered into the document. See more info about this in the README. This wouldn't be a problem with frameworks which do not bind events to the document.

@antsmartian
Copy link
Collaborator

Got it, thanks!

@antsmartian
Copy link
Collaborator

Closing this w.r.t #57

julienw pushed a commit to julienw/react-testing-library that referenced this issue Dec 20, 2018
* Add within API and document it

* improve the `within` API example

* Update README.md
@ajur58
Copy link

ajur58 commented Mar 26, 2019

Correct me if I'm wrong, but JSDOM does not support submitting the form element: jsdom/jsdom#1937

I get the same error as described in the link above when trying to do so.

Am I missing something?

@kentcdodds
Copy link
Member

This is a pretty old issue. Could you open a new one with an example of what you're trying to do?

@ajur58
Copy link

ajur58 commented Mar 26, 2019

Sorry for not being clear enough: I think this is not an issue related to react-testing-library.

I tried to trigger a form submit using fireEvent. I got the error described in the link I posted before (JSDOM not supporting submitting forms), but reading the comments above I was under the impression that forms can be submitted. I'm still not sure if this is the case.

I asked in this thread as I thought other people might end up here when searching for the same issue.

Sidenote: We're in the process of moving from enzyme to react-testing-library, love the new approach! Thanks!

@kentcdodds
Copy link
Member

Glad you're liking react-testing-library @j13l!

Hmmm, this actually works today:

import React from 'react'
import {render, fireEvent} from 'react-testing-library'

test('works', () => {
  const handler = jest.fn(e => e.preventDefault())
  const {getByTestId} = render(
    <form onSubmit={handler} data-testid="form">
      <button type="submit">Submit</button>
    </form>,
  )
  fireEvent.submit(getByTestId('form'))
  expect(handler).toHaveBeenCalledTimes(1)
})

Though I would actually recommend not firing a submit event on a form, but instead do what the user would do and click on the submit button:

import React from 'react'
import {render, fireEvent} from 'react-testing-library'

test('works', () => {
  const handler = jest.fn(e => e.preventDefault())
  const {getByText} = render(
    <form onSubmit={handler}>
      <button type="submit">Submit</button>
    </form>,
  )
  fireEvent.click(getByText(/submit/i))
  expect(handler).toHaveBeenCalledTimes(1)
})

Note: That in this case, having my submit handler call preventDefault() was key to avoiding the warning you're talking about :)

I hope that's helpful!

@ajur58
Copy link

ajur58 commented Mar 26, 2019

Yes, it works!!

I thought submitting via click is breaking due to the JSDOM error.

Turns out it was something completely different: react-intl library injects a span for translations leading to <button type="submit"><span>submit</span></button>. Firing a click event on the span does not trigger a form submit in the tests, but it does in the browser. Is this desired behaviour?

Thanks a lot for your help @kentcdodds 👏

@kentcdodds
Copy link
Member

Actually, I think it's ok to put a span inside a button, but firing a click event on the span should bubble up to the button anyway, so it should still work.

@viniciusavieira
Copy link
Contributor

viniciusavieira commented Mar 26, 2019

Actually it works if you use this aproach:

test('works', () => {
    const handler = jest.fn(e => e.preventDefault())
    const {container} = render(
      <form onSubmit={handler}>
        <button type="submit" data-testid="submit"><span>Submit</span></button>
      </form>,
    )
    
    fireEvent.click(getByTestId(container, 'submit'))
    expect(handler).toHaveBeenCalledTimes(1)
  })

But using the example with getByText this same example fails, and this is probably related to the extra span in the button, since the return of the text match should be the <span>Submit</span>, and the span does not have a submit property. Not sure if is intentional, but we could update the docs about react-intl and prevent people of running into this.

test('does not works', () => {
    const handler = jest.fn(e => e.preventDefault())
    const {container} = render(
      <form onSubmit={handler}>
        <button type="submit" data-testid="submit"><span>Submit</span></button>
      </form>,
    )
    
    fireEvent.click(getByText(container, /submit/i))
    expect(handler).toHaveBeenCalledTimes(1)
  })

@kentcdodds
Copy link
Member

Ok, so this should work:

test('works', () => {
  const handler = jest.fn(e => e.preventDefault())
  const {getByText} = render(
    <form onSubmit={handler}>
      <button type="submit">
        <span>Submit</span>
      </button>
    </form>,
  )

  fireEvent.click(getByText(/submit/i))
  expect(handler).toHaveBeenCalledTimes(1)
})

If it's not working it's probably because your version of jsdom is old. I believe this is a bug that was fixed in jsdom@14.

If you cannot upgrade, then I recommend this approach:

test('works', () => {
  const handler = jest.fn(e => e.preventDefault())
  const {getByText} = render(
    <form onSubmit={handler}>
      <button type="submit">
        <span>Submit</span>
      </button>
    </form>,
  )

  fireEvent.click(getByText(/submit/i).closest('button'))
  expect(handler).toHaveBeenCalledTimes(1)
})

@ajur58
Copy link

ajur58 commented Mar 26, 2019

You're right.

Just set up a CodeSandbox example, but bubbling does work as intended.

However, in our case if I render the label with a span, it does not bubble up. Running on latest Jest, JSDOM & co. Ran out of ideas why this could happen, might look into it again later. Thanks!

@kentcdodds
Copy link
Member

The problem is that jest-environment-jsdom depends on a very old version of jsdom :-(

If you use https://www.npmjs.com/package/jest-environment-jsdom-fourteen you'll be set 👍

Maybe this should be documented, or maybe we can help jest upgrade jsdom...

@kentcdodds
Copy link
Member

Looks like the reason jest is using an old version of jsdom is for Node 4 support. Node 4 will reach End-of-life at the end of April. So hopefully in the next few months we should get an updated version out of the box.

@mikelyons
Copy link

mikelyons commented Apr 14, 2019

I wonder if this would explain having a similar issue with event bubbling in codesandbox's jest environment.

@vospascal
Copy link

I see now even jsdom is on 15 .... the old version in jest is sad :(

@alexreardon
Copy link

You can use the container to find the form element:

const {container, fireEvent} = render(<App/>)

const form = container.querySelector('form');

fireEvent.submit(form);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation A docs improvement is needed. help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

8 participants