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 mocks for RNGH #493

Merged
merged 6 commits into from
Apr 29, 2019
Merged

Add mocks for RNGH #493

merged 6 commits into from
Apr 29, 2019

Conversation

osdnk
Copy link
Contributor

@osdnk osdnk commented Mar 4, 2019

Motivation

Need to provide some mocks for RNGH in order to make it workable with jest.

Changes

  1. Remove need of mocking PlatformConstants. Thanks @tsapeta! Now I check whether PlatformConstants exists before usage.
  2. Add mocks of RNGH module.
  3. Also abstract RNGH module for separated file for easier mocking.

@@ -0,0 +1,13 @@
export default {
Copy link

@ice-chillios ice-chillios Mar 5, 2019

Choose a reason for hiding this comment

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

I've also find out that you may need to mock those:

jest.mock('NativeModules', () => ({
  UIManager: {
    RCTView: () => {}, // or 'View'
  },
  PlatformConstants: {
    forceTouchAvailable: false,
  },
}))

I'm not sure but it may help others :)

Choose a reason for hiding this comment

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

Oh, nevermind. You've changed implementation of GestureHandler

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Certainly I do not need PlatformConstants. See the changes and you'll figure out why :).
Why do you think I do need RCTView?

Choose a reason for hiding this comment

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

Oh, sorry. This seems to be obsolete. I've had this one in past and thought it's still necessary.

Copy link
Member

@kmagiera kmagiera left a comment

Choose a reason for hiding this comment

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

Looks good. Can you just comment on how you tested this change? E.g. share a project with jest setup the way it picks up the correct mocks for GH

package.json Outdated Show resolved Hide resolved
Co-Authored-By: osdnk <micosa97@gmail.com>
@osdnk
Copy link
Contributor Author

osdnk commented Mar 7, 2019

I'll share project later. I don't want to have it done in Example because of really strange config, which makes it more difficult and less universal.

@osdnk
Copy link
Contributor Author

osdnk commented Mar 7, 2019

@ice-chillios
Copy link

@osdnk As you see there it has been necessary to pass the tests. That's why I mentioned this one :)

@osdnk
Copy link
Contributor Author

osdnk commented Mar 7, 2019

@dsznajder
Yes, but https://youtu.be/HG7I4oniOyA?t=115

I see that people are doing it, but I don't want to make it matter of faith. I suppose it was important it previous version because of this:
35c57a0#diff-56d925ec2aada9e1b3b613fe5b782837

But since this commit I see it does not have any impact.

@ice-chillios
Copy link

I can confirm that also. It does not make any difference with 1.1.0 and 1.0.17 so I'm pretty sure that your mock is 100% correct ;)

@ice-chillios
Copy link

@osdnk I find out that with 1.1.0 and RN 0.59.1 I got an error:
TypeError: Cannot read property 'setJSResponder' of undefined
caused by this line

It seems to be necessary to mock UIManager with an empty object or sth.

@jkadamczyk jkadamczyk merged commit baf2ba7 into master Apr 29, 2019
@@ -33,6 +34,8 @@
"PlatformConstants.web.js",
"react-native-gesture-handler.d.ts",
"README.md",
"RNGestureHandlerModule.js",
"jestConfig.js",
Copy link

@FireyFly FireyFly Apr 30, 2019

Choose a reason for hiding this comment

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

This should be jestSetup.js, I think? Which probably explains why the jestSetup.js appears to be missing from the 1.2.0 release.

(Also, thanks for this PR/adding this mock--I incidentally ran into issues trying to mock react-native-gesture-handler yesterday, so it landed with quite convenient timing!)

janicduplessis pushed a commit to janicduplessis/react-native-gesture-handler that referenced this pull request Feb 16, 2020
* Add basic test

* cleanup

* Directions -> Direction

* Revert changes in package.json

* Update package.json

Co-Authored-By: osdnk <micosa97@gmail.com>

* add a note to getting started
@jakub-gonet jakub-gonet deleted the config-jest branch July 13, 2020 12:42
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 this pull request may close these issues.

5 participants