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

Bug: Non-editable TextInputs are updated via fireEvent.changeText() when wrapped within 2 components down #1261

Closed
ylemberg opened this issue Dec 20, 2022 · 10 comments · Fixed by #1385

Comments

@ylemberg
Copy link

ylemberg commented Dec 20, 2022

Describe the bug

I have an issue very similar to #476. However this reproduces once the <TextInput /> is nested within 2 components.

I created a Github repo to demonstrate this issue.

Expected behavior

A <TextInput/> that is nested within 2 components or more should not have the fireEvent.changeText() invoke the onChangeText() callback and the test in the above repo "should not fire on non-editable SecondTextInputWrapper" should pass

Steps to Reproduce

  1. Clone the repository https://github.com/ylemberg/text-input-spec-example
  2. Run yarn install
  3. Run yarn test

Screenshots

Screen Shot 2022-12-19 at 5 50 27 PM

Versions

"devDependencies": {
    "@babel/core": "^7.18.0",
    "@babel/preset-typescript": "^7.17.12",
    "@babel/runtime": "^7.18.0",
    "@testing-library/jest-native": "3.4.3",
    "@testing-library/react-native": "^11.5.0",
    "@types/jest": "^29.2.4",
    "babel-plugin-module-resolver": "^4.1.0",
    "cross-env": "^7.0.3",
    "jest": "26.6.3",
    "react": "17.0.2",
    "react-native": "0.66.5",
    "react-test-renderer": "^17.0.2",
    "typescript": "^4.2.0"
  }

https://github.com/ylemberg/text-input-spec-example/blob/main/package.json

@mdjastrzebski
Copy link
Member

@ylemberg Thanks for reporting this and providing us with repro repository. RNTL event handling works by trying to simulate how React Native event handling would work and we sometimes have discrepancies as the one you've found.

We will try to diagnose it at some stage but it looks like relatively low priority issue that can be worked around by avoiding firing change text events at non-editable TextInputs.

@ylemberg
Copy link
Author

@mdjastrzebski Thanks for taking a look at the issue.

Avoiding firing a change text event at a non-editable TextInput does not work for my use case unfortunately.

I have a component that has a form, something like

const myComponent = ({firstName, lastName}) => {
  <Form>
      <SecondTextInputWrapper
        editable={false}
        testID={testID}
        onChangeText={onChangeTextMock}
        placeholder={`${firstName}`}
      />
      <SecondTextInputWrapper
        editable={false}
        testID={testID}
        onChangeText={onChangeTextMock}
        placeholder={`${lastName}`}
      />
    ...
  </Form>
}

and would like to test myComponent that the user is unable to edit both <SecondTextInputWrapper/>'s. However as described in the example repo above using fireEvent.changeText() will incorrectly update the TextInputs. Would you have any advice as to a good work around for this situation?

@mdjastrzebski
Copy link
Member

mdjastrzebski commented Dec 30, 2022

If your goal is to verify that user cannot edit either TextInputs then more straightforward way of expressing that requirement would be following assertion:

expect(screen.getByPlaceholder(firstName)).toBeDisabled()
expect(screen.getByPlaceholder(lastName)).toBeDisabled()

This uses toBeDisabled() matcher from Jest Native.

@ylemberg
Copy link
Author

This appears to work for my use case. Needed to add an accessibilityState={{ disabled: true }} to the TextInput but unblocks my scenario. Thanks!

@mdjastrzebski
Copy link
Member

mdjastrzebski commented Dec 30, 2022

toBeDisabled() should actually react to editable={false} set on TextInput: https://github.com/testing-library/jest-native/blob/49c3bbdaa141723c66320ce8c2282d18a7bc9a6a/src/to-be-disabled.ts#L21

Can you post the query & assertion you are using?

@mdjastrzebski
Copy link
Member

@ylemberg You seem to be using quite old version of Jest Native. Recent versions of it support 'editable' prop in 'toBeDisabled()' matcher.

@ylemberg
Copy link
Author

ylemberg commented Jan 3, 2023

@mdjastrzebski ah yes bumping up my "@testing-library/jest-native" version to latest appeared to no longer need the accessibilityState change. Thank you and Happy New Year!

@mdjastrzebski
Copy link
Member

@ylemberg Happy New Year to you too!

@pierrezimmermannbam
Copy link
Collaborator

This one is pretty tricky. What fireEvent does is that it tries to trigger the given event by calling a specific prop on the element. In your case you're using changeText so it will look for onChangeText prop. Since the TextInput is not editable, it will not trigger on the TextInput but then it tries to trigger on the parent. The mechanism of going up the tree is useful namely for touch events, it's what allows to press a button from a Text within it for instance.
Then because your wrapper has a prop onChangeText and it's not a TextInput (in that case we don't check the value of editable prop) it will call that prop, hence your test will fail.

This can't be fixed easily or without introducing breaking changes, so using the toBeDisabled matcher is a very good workaround for now I think and in the future we may think about changing the behavior of fireEvent so that some cases can't happen

@mdjastrzebski
Copy link
Member

This issue has been resolved in v12.1.2 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants