Skip to content

Missing peer dependency on react-dom #1424

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

Closed
pieh opened this issue Oct 14, 2019 · 4 comments
Closed

Missing peer dependency on react-dom #1424

pieh opened this issue Oct 14, 2019 · 4 comments

Comments

@pieh
Copy link

pieh commented Oct 14, 2019

This package import some utilities from react-dom:

export { unstable_batchedUpdates } from 'react-dom'

But react-dom is not specified as dependency or peerDependency

react-redux/package.json

Lines 38 to 49 in 2297944

"peerDependencies": {
"react": "^16.8.3",
"redux": "^2.0.0 || ^3.0.0 || ^4.0.0-0"
},
"dependencies": {
"@babel/runtime": "^7.5.5",
"hoist-non-react-statics": "^3.3.0",
"invariant": "^2.2.4",
"loose-envify": "^1.4.0",
"prop-types": "^15.7.2",
"react-is": "^16.9.0"
},

This cause Cannot file module 'react-dom' errors when using it with Ink (react for cli/terminal).

Installing react-dom does fix the issue (bit weird to install it for terminal app, but what can you do :) ). If react-dom is being used, it should be either dependency (probably not the way here) or peerDependency so package managers warn about missing dependency before we run into issues like that.

Issue discovered in gatsbyjs/gatsby#18603

@timdorr
Copy link
Member

timdorr commented Oct 14, 2019

See these PRs:

#1261
#1282
#1371
#1366

@timdorr timdorr closed this as completed Oct 14, 2019
@pieh
Copy link
Author

pieh commented Oct 14, 2019

That's unfortunate :/ but also understandable. Too bad package.json doesn't have some way to conditionally declare dependencies.

Given amount if PRs and issues about this, what do you think about putting some context around that import in source code to guard against PRs and issues about this in the future? :)

In any case - thanks for quick response!

@markerikson
Copy link
Contributor

Heh. Yeah, a "DON'T MESS WITH THIS!" comment might not be a bad idea :)

@timdorr
Copy link
Member

timdorr commented Oct 15, 2019

Too bad package.json doesn't have some way to conditionally declare dependencies.

They do. It's peerDependenciesMeta and it's in all the latest package managers, but is too new to rely on (unlike browsers, we can't polyfill npm 😄).

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