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

Documentation update for testing React Native using jsdom #1873

Merged

Conversation

shawnaxsom
Copy link
Contributor

As discussed in #1375, I was also unable to get React Native testing working with Jest/Enzyme with the current documentation instructions.

I see that a React Native adapter is still being discussed in #1436, but I found that some users have had success with jsdom. I was able to get both react-native-mock-renderer and jsdom working, but react-native-mock-renderer didn't result in very clear snapshots when using Jest snapshot testing.

I have therefore provided some instructions here for how I was able to get React Native testing working with jsdom, and I added some examples from the React Native boilerplate project where it is currently working. I decided to leave in a few helpful tips of other issues I encountered along the way as well.


## Loading an emulated DOM with JSDOM

To use enzyme's `deep` until a React Native adapter exists, an emulated DOM must be loaded.
Copy link
Member

Choose a reason for hiding this comment

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

what is "deep"? Do you mean mount?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes mount, good catch, I'll update that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This has been updated to mount

docs/guides/react-native.md Outdated Show resolved Hide resolved

function copyProps(src, target) {
const props = Object.getOwnPropertyNames(src)
.filter(prop => typeof target[prop] === "undefined")
Copy link
Member

Choose a reason for hiding this comment

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

let's use single quotes here, so the JS would pass eslint

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've updated all the double quotes in the page to single quotes

const { window } = jsdom;

function copyProps(src, target) {
const props = Object.getOwnPropertyNames(src)
Copy link
Member

Choose a reason for hiding this comment

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

is Object.getOwnPropertyDescriptors not an option?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This particular function comes from the JSDOM setup documentation: https://airbnb.io/enzyme/docs/guides/jsdom.html

I kept it consistent with that page, copying the setup.js part there but adding some other settings below for the adapter and special handling some console warnings.

I can look into getOwnPropertyDescriptors but probably would want to update it on both pages.

What do you suggest?

Copy link
Member

Choose a reason for hiding this comment

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

Let's update to use it; it's cleaner anyways.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do we want to support es5?

I updated for now to:

function copyProps(src, target) {
  Object.defineProperties(target, {
    ...Object.getOwnPropertyDescriptors(src),
    ...Object.getOwnPropertyDescriptors(target)
  });
}

I think that does what we want cleanly, but VSCode is complaining:

[ts] Property 'getOwnPropertyDescriptors' does not exist on type 'ObjectConstructor'. Did you mean 'getOwnPropertyDescriptor'?
lib.es5.d.ts(172, 5): 'getOwnPropertyDescriptor' is declared here.

Because Object.getOwnPropertyDescriptors isn't in the es5 spec and unsupported also for IE:

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/getOwnPropertyDescriptors

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let me know whether we should keep as is to support es5 / IE, or whether the updated code I have above with getOwnPropertyDescriptors is preferred.

Copy link
Member

Choose a reason for hiding this comment

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

gOPDs is polyfillable, so it should work just fine. https://www.npmjs.com/package/object.getownpropertydescriptors

The spread can be transpiled to Object.assign which is also polyfillable https://www.npmjs.com/package/object.assign

so i think that update works great

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good, I updated copyProps here and in two other places in the codebase:

docs/guides/jsdom.md
packages/enzyme-example-mocha/test/.setup.js

@ljharb ljharb added the docs label Oct 22, 2018
@ljharb ljharb force-pushed the feature/react-native-documentation branch from fe5b22d to ba7675e Compare October 25, 2018 06:06
@jpaas
Copy link

jpaas commented Oct 26, 2018

Hey @shawnaxsom I'm trying out what you have in the docs, but I'm getting Cannot find module 'StyleSheet' from 'react-native-implementation.js'

@shawnaxsom
Copy link
Contributor Author

@jpaas Is the code public so I can see? Are you importing StyleSheet anywhere? StyleSheet should come from "react-native" instead of "react-native-implementation" generally.

Take a look at the boilerplate code we have here to compare:

https://github.com/ProminentEdge/mobile-boilerplate

@jpaas
Copy link

jpaas commented Oct 26, 2018

@shawnaxsom no sorry its a private repo. I did have a look at your repo. My setup-tests.js is identical. Even though I'm using typescript I wasn't able to get your jest.config.js to work. This is my jest.config.js that works for my non-enzyme tests:

module.exports = {
  preset: "ts-jest",
  setupTestFrameworkScriptFile: "<rootDir>/setupTests.js",
  transform: {
    // Required to work around this problem: https://github.com/facebook/react-native/issues/19859
    "^.+\\.(js|tx|tsx)$": "<rootDir>/node_modules/react-native/jest/preprocessor.js",
  },
}

@shawnaxsom
Copy link
Contributor Author

@jpaas Have you compared versions in package.json? What version of "react-native" are you on?

@shawnaxsom
Copy link
Contributor Author

@jpaas Also make sure you have this in package.json:

https://stackoverflow.com/questions/47269957/cannot-find-module-from-react-native-implementation-js

"jest": {
    "preset": "react-native"
},

@jpaas
Copy link

jpaas commented Oct 26, 2018

@shawnaxsom I'm using react-native 0.57.4 & react 16.6.0-alpha.8af6728 vs your 0.57.2 and react 16.5.0. I tried rolling back to your version, but no difference.

I tried that jest preset in the package.json but it didn't help, I suspect because my jest.config.js overrides it with the ts-jest preset.

When I try to use your jest.config.js, I get the error Couldn't find preset "module:metro-react-native-babel-preset" relative to my root dir`

@shawn-pe
Copy link

shawn-pe commented Oct 26, 2018 via email

@jpaas
Copy link

jpaas commented Oct 26, 2018

@shawnaxsom I am using metro-react-native-babel-preset as my preset in my .babelrc, so I imagine there's something about your jest.config.js that is preventing it from being resolved.

@shawnaxsom
Copy link
Contributor Author

shawnaxsom commented Oct 26, 2018 via email

@ljharb ljharb force-pushed the feature/react-native-documentation branch 2 times, most recently from 55ea48f to 69869e0 Compare November 1, 2018 17:33
@ljharb ljharb merged commit 69869e0 into enzymejs:master Nov 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants