pmndrs / react-spring

✌️ A spring physics based React animation library
http://www.react-spring.dev/
MIT License
28.11k stars 1.19k forks source link

React spring is breaking my tests #555

Closed WebDeg-Brian closed 5 years ago

WebDeg-Brian commented 5 years ago

I'm currently using Jest with enzyme to test my components. My animations work fine on the client, no problems are shown in the console. However, when I run my tests, Jest keeps saying the problem comes from react-spring. Here are the screenshots:

https://cdn.discordapp.com/attachments/103696749012467712/547192480832159765/unknown.png https://cdn.discordapp.com/attachments/103696749012467712/547192562851643402/unknown.png

I also confirm that because when I remove all the animations my tests work fine. As soon as I import Spring from react-spring the tests started to break :( I believe this is a strong, and would appreciate if you could fix it as soon as possible. Thank you!

Edit: The problem most likely comes from the renderprops module, since I don't get the problem using hook

Edit again: I've read the this issue on GitHub and tried the workaround but doesn't seem to work. I then also tried to import the cjs file and it works, but it kinda sucks to import a CJS file instead of an actual ESM

drcmda commented 5 years ago

node doesn't understand esm, it cannot use imports. it would make sense to use the cjs for testing. i think this is how you'd do it with any other library, or how do you normally get around es-modules?

jacobrask commented 5 years ago

This should work to make sure Jest picks up the CJS bundles instead of ESM

    "moduleNameMapper": {
      "react-spring": "<rootDir>/node_modules/react-spring/web.cjs",
      "react-spring/renderprops": "<rootDir>/node_modules/react-spring/renderprops.cjs"
    },

The way it normally works is probably that Jest uses the main field in package.json, and Webpack uses module. This works as expected for the main entry point, but not for renderprops since it's not defined in package.json.

WebDeg-Brian commented 5 years ago

@drcmda I'm not quite sure about it, but for all other library it works fine.

WebDeg-Brian commented 5 years ago

@jacobrask Hey, thanks for the solution. I'm quite confused why jest works fine with other libs such as material-ui, but doesn't with react-spring

drcmda commented 5 years ago

Are you sure they serve evergreen-esm, though? Many popular libs (like React) just ship plain commonjs in a flatbundle, which can't be tree-shaken afterwards. If there's something i can do to make the Node story better i would of course gladly do it, i just don't know where the problem is.

edit:

Is this the material-ui you're referring to? https://unpkg.com/material-ui@0.20.2/index.js To me that looks like cjs.

WebDeg-Brian commented 5 years ago

Wait, so import works with both cjs and esm, but require() only works with cjs? I've setup react-spring with jest many times, but after updating this package it doesn't seem to work anymore

WebDeg-Brian commented 5 years ago

@drcmda Nah nevermind, I actually got it now. Is there anything you can do to make cjs the default file instead of esm? Or at least a doc on this would help a lot of people

drcmda commented 5 years ago

require, as i understand it, is commonjs, node's current module standard. es modules define imports via import and exports via export, that's what seems to choke node atm.

I've setup react-spring with jest many times, but after updating this package it doesn't seem to work anymore

What @jacobrask said explains the weird behaviour. Due to hooks we are forced to distribute essentially two libs. Previously Jest would just pick up the main field in package.json.

@jacobrask do you think there is any solution to this?

Otherwise i will make a new section about this in the docs.

WebDeg-Brian commented 5 years ago

@drcmda Shall I just make a pull request for this bit?

drcmda commented 5 years ago

Yes, that would be great! 😄

WebDeg-Brian commented 5 years ago

@drcmda Ok I will have a quick look at the repo and make a PR now

victors1681 commented 5 years ago

@WebDeg-Brian I just have the same issue, and I noticed the thread is resent. But with this issue is closed without any PR related? did you fix it?

victors1681 commented 5 years ago

Never mind the @jacobrask previews solution it works, I just I had to remove my cache loaders to make it works.

jacobrask commented 5 years ago

@drcmda I haven't followed how far the .mjs proposal got, but this problem was basically what .mjs was supposed to solve - .mjs files are ES modules, .js files are commonjs.

Maybe long term the components should be re-implemented as wrappers around the hooks and re-integrated into the main bundle? The different targets will still have the same problem though.

drcmda commented 5 years ago

I thought about wrapping hooks with Spring/Transition/Trail classes, too, it would be the best way forward. next major this could perhaps be one of the bigger changes.

kwrush commented 5 years ago

This should work to make sure Jest picks up the CJS bundles instead of ESM

    "moduleNameMapper": {
      "react-spring": "<rootDir>/node_modules/react-spring/web.cjs",
      "react-spring/renderprops": "<rootDir>/node_modules/react-spring/renderprops.cjs"
    },

The way it normally works is probably that Jest uses the main field in package.json, and Webpack uses module. This works as expected for the main entry point, but not for renderprops since it's not defined in package.json.

In my case, the order of entries matters. Sub entry should be defined before the parent

"moduleNameMapper": {
  "react-spring/renderprops": "<rootDir>/node_modules/react-spring/renderprops.cjs",
  "react-spring": "<rootDir>/node_modules/react-spring/web.cjs"
}
TrebuhD commented 5 years ago

In case you use Create-React-App (which doesn't support moduleNameMapper), reverting to v7.2.10 works.

stevesuyao commented 4 years ago

This should work to make sure Jest picks up the CJS bundles instead of ESM

    "moduleNameMapper": {
      "react-spring": "<rootDir>/node_modules/react-spring/web.cjs",
      "react-spring/renderprops": "<rootDir>/node_modules/react-spring/renderprops.cjs"
    },

The way it normally works is probably that Jest uses the main field in package.json, and Webpack uses module. This works as expected for the main entry point, but not for renderprops since it's not defined in package.json.

In my case, the order of entries matters. Sub entry should be defined before the parent

"moduleNameMapper": {
  "react-spring/renderprops": "<rootDir>/node_modules/react-spring/renderprops.cjs",
  "react-spring": "<rootDir>/node_modules/react-spring/web.cjs"
}

@kwrush You really saved my day! Thx!

javierfuentesm commented 2 years ago

@kwrush this was working fine but now it is failing im getting this :

image

any ideas?