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

enzyme-adapter-react-15.4 incompatible with react/react-dom 15.3 #1277

Closed
edmorley opened this issue Oct 18, 2017 · 9 comments
Closed

enzyme-adapter-react-15.4 incompatible with react/react-dom 15.3 #1277

edmorley opened this issue Oct 18, 2017 · 9 comments

Comments

@edmorley
Copy link
Contributor

edmorley commented Oct 18, 2017

Hi!

The Enzyme docs (here) say to use enzyme-adapter-react-15.4 when using React 15.0-15.4.

As of #1234 / #1252, react-addons-test-utils is no longer a peerDependency of enzyme-adapter-react-15.4, but a normal dependency. This means that instead of the user being able to specify a suitable version, the adapter uses the version range 15.0.0-0 - 15.4.x, which resolves to 15.4.2, and so is incompatible with react/react-dom 15.3.x.

For example when using:

  "dependencies": {
    "react": "15.3.2",
    "react-dom": "15.3.2",
    ...
  },
  "devDependencies": {
    "enzyme": "3.1.0",
    "enzyme-adapter-react-15.4": "1.0.2",
    ...
  }

...enzyme-adapter-react-15.4 pulls in react-addons-test-utils 15.4.2, which contains:

module.exports = require('react-dom/lib/ReactTestUtils');

...which doesn't exist in react-dom 15.3.2, and so results in:

Module not found: Error: Can't resolve 'react-dom/lib/ReactTestUtils' in '....\node_modules\react-addons-test-utils'

Compare this to react-addons-test-utils 15.3.2, which contains:

module.exports = require('react/lib/ReactTestUtils');

I would presume the solutions to be either:

  1. Revert the peer dependency change for move react-addons-test-utils to dependencies #1234 / Make react-test-renderer a dependency instead of a peer dependency #1252 for older versions of the adapter.
  2. Create a new enzyme-adapter-react-15.3 that has a reduced version range for react-addons-test-utils.

Many thanks :-)

CC @brucewpaul, @ljharb, @bdwain.

@ljharb
Copy link
Member

ljharb commented Oct 18, 2017

Interesting, that's a consequence that we didn't think of, and the reason that it needs to be a peer dep.

I'm thinking maybe we should revert the changes for all the adapters. @brucewpaul, @bdwain, thoughts?

@bdwain
Copy link
Contributor

bdwain commented Oct 18, 2017

I think 15.4 is a special case and in general within the same major version should be fine. I'm not totally familiar with what happened that required a change for react 15.4, but I don't think this would be an issue for react 16 or 15.5+. I'm not totally sure about 13 or 14 though. But i think it's best to only revert where there's a clear problem, which seems to just be 15.4.

Also, is it possible that the version range for the 15.4 adapter is just wrong? Maybe it should be < 15.4, not <= 15.4. Something seems to have changed between 15.3 and 15.4 that broke 15.4.

@ljharb
Copy link
Member

ljharb commented Oct 18, 2017

@bdwain does that mean you're saying that using a 15.6 test-utils would work with 15.5, for example?

It's certainly possible that we need adapters for all of 15-15.3, 15.4, and 15.5+.

@bdwain
Copy link
Contributor

bdwain commented Oct 18, 2017

react addons test utils 15.6.2 has a peer dependency on "react-dom": "^15.4.2", so as long your version of react dom matches that it should be good.

@ljharb
Copy link
Member

ljharb commented Oct 18, 2017

ok, so are we looking at a specific case where react 15.3 breaks with test utils for 15.4, but that's not true for any other major line?

@bdwain
Copy link
Contributor

bdwain commented Oct 19, 2017

I'll investigate all of these tonight and get back to you.

@bdwain
Copy link
Contributor

bdwain commented Oct 19, 2017

I think I misread v 15.6.2's peer dependency. it's actually ^15.6.2. It looks like they've had a policy of bumping the peer dependency of react-test-renderer to match its version, so that react had to be at least as new. This may not have been a hard requirement, but it still causes npm ls to fail.

I opened an issue about this and it sounds like for version 16 they may be willing to change their policy and leave the peer dependency at ^16.0.0`. facebook/react#11278

I think we will need to revert the change for everything but 16.0, but it sounds like we can leave 16.0 as a regular dependency (they are still on 16.0.0 anyway).

I can open this PR later

@ljharb
Copy link
Member

ljharb commented Oct 20, 2017

Makes sense.

@bdwain
Copy link
Contributor

bdwain commented Oct 25, 2017

#1301 should fix this @edmorley @ljharb

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

No branches or pull requests

3 participants