-
Notifications
You must be signed in to change notification settings - Fork 1.1k
onChange event not triggering function #175
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
Comments
Weird. Thanks for reporting this. Could you do a little digging to figure out what's going on? |
I'm happy to do that, where would you recommend I start? I have been trying to figure out what is wrong for a few hours now. I'm not sure what to do next. |
When you say you tried using |
I now have the following test: it('handleChange function called', async () => {
const spy = jest.fn();
const { getByLabelText } = render(<CostInput handleChange={spy} />);
let input = getByLabelText('Item Cost');
expect(input.value).toBe('');
fireEvent.change(input, { target: { value: '23' } });
await wait(() => {
expect(spy).toHaveBeenCalledTimes(1);
}); It still works on the code sandbox but not locally. |
I can confirm that I'm having the same problem. I assumed I was doing something incorrectly until I saw this issue. I also attempted to use |
@SZBoatwright what operating system are you using? I am using Windows 10. |
I'm having issues with The following is my test for my component called const onChange = jest.fn();
const { getByTestId } = render(
<Input name="test-input" value="old value" onChange={onChange} />
);
const inputNode = getByTestId('input-node'); // The actual <input>
expect(onChange).toHaveBeenCalledTimes(0);
fireEvent.change(inputNode, { target: { value: 'new value' } });
expect(onChange).toHaveBeenCalledTimes(1); The above test passes. However, if I change the last line to: expect(onChange).toHaveBeenCalledWith({ target: { value: 'new value' } }); Then I get a ton of errors all of the form:
The difference in all the errors is the property. So the above error is for "nativeEvent", then there's one for "target", "currentTarget", "eventPhase", etc. Out of curiosity, I added a |
@themostcolm I'm on a mac with Sierra. I have a desktop with Windows 10 on it that I could try on as well. |
@denchen your issue is likely unrelated. Read the information in the error message link. I'll try to give this a look when I get the chance, but probably not until next week. |
I seem to have a similar problem: I can't trigger onChange handler on my checkbox component.
Store:
My tests:
I also tried changing the attribure by hand and calling fireEvemt afterwards:
The component itself works fine. If I'm doing something wrong, I'd appreciate the help with figuring it out! |
Hey @Enikol, I tried to reproduce your code and it works for me. Is it possible that the |
Hi @Gpx ! Thank you for taking time to dig into this. No, I don't think it's possible, but I tried wrapping it in
Test just fails because of the 5ms timeout, because |
Ok, so you are all probably experiencing slightly different problems. @themostcolm, your issue is that your @SZBoatwright, my guess is you're experiencing the same issue. @denchen, the reason you're getting all those errors is because jest is reading properties from the Synthetic Event that React called your event handler with and that synthetic event has already been returned to the pool (learn more about the synthetic event system here: https://www.youtube.com/watch?v=dRo_egw7tBc&list=PLV5CVI1eNcJi8sor_aQ2AzOeQ3On3suOr). The solution is to not make an assertion about how your event handler was called because that's an implementation detail. Assert on the DOM output, how the your prop function callbacks were called, or how other modules were called. @Enikol, this should probably be documented better, but with checkboxes you don't actually fire Thanks everyone! I'm going to go ahead and close this now :) |
**What**: Added a note about triggering `onChange` event handler on a checkbox to README.md **Why**: Discussed in #175 **Checklist**: <!-- add "N/A" to the end of each line that's irrelevant to your changes --> <!-- to check an item, place an "x" in the box like so: "- [x] Documentation" --> - [x] Documentation - [x] Tests"N/A" - [x] Ready to be merged <!-- In your opinion, is this ready to be merged as soon as it's reviewed? --> - [x] Added myself to contributors table <!-- this is optional, see the contributing guidelines for instructions -->
**What**: Move `CODE_OF_CONDUCT.md` from `other` to root of repo **Why**: `README.md` links to this file and expects to find it in the root. **Checklist**: - [x] Documentation - [x] Tests (N/A) - [x] Ready to be merged <!-- In your opinion, is this ready to be merged as soon as it's reviewed? --> - [x] Added myself to contributors table <!-- this is optional, see the contributing guidelines for instructions -->
It's certainly not an implementation detail when testing a reusable text input component, where the event being propagated is part of the component's interface contract. This is the problem I ran into today when trying to test a custom
|
You could also make a component that uses the API you want to test and test that component instead. That has the benefit of making the tests better suited as documentation of intent. |
@kentcdodds Thanks for the tip, this is basically what we did now. |
@Enikol did you find a problem? I experience same issue with mobx and checkbox field now. |
Hi @ktarasov-clearscale , did you see @kentcdodds answer about using
|
@MatanBobi yes, I've seen that answer. Let's assume I have a scenario with a checkbox input which checked by default. Do you think I should fire a click event to test that scenario? |
@ktarasov-clearscale So let me just understand the scenario, you're talking about an initial value and not a default value.. You're getting your data from the server or something? |
@MatanBobi seems my issue is related to final forms as soon as final forms have own state. No problems with mobx and jest. Just need to construct (find) selectors from wrapper element (provided by mount/render/shallow) to avoid sideeffects. |
react-testing-library
version: 5.0.1react
version: 16.5.0node
version: 9.11.1 or 8.11.1 (tested to see if it made a difference)npm
(oryarn
) version: 5.6.0Relevant code or config:
What you did:
I wrote a test to check that the handleChange function would be called. I also ran the same code on CodeSandbox at https://codesandbox.io/s/jvw4499xzw.
What happened:
The test fails on my local environment which is a CRA application that only has those 2 files in it.
It works on the Code Sandbox without a problem.
Reproduction:
The working Code Sandbox can be found here: https://codesandbox.io/s/jvw4499xzw
The broken repo is found here: https://github.com/themostcolm/react-testing-bug
If possible, please create a repository that reproduces the issue with the
minimal amount of code possible.
Problem description:
It is making my app untestable.
Suggested solution:
I honestly have no idea, I'm sorry. I tried using waitForElement, but it didn't make a difference.
The text was updated successfully, but these errors were encountered: