JedWatson / react-tappable

Tappable component for React
http://jedwatson.github.io/react-tappable/
MIT License
862 stars 90 forks source link

React as peerDependency / Add 0.13 #13

Closed natew closed 9 years ago

natew commented 9 years ago

Couple tweaks here. Peer dependency so it doesn't duplicate react version, and also adding support for the beta version.

But... not sure why but running an npm link with this package.json is giving me this:

0 info it worked if it ends with ok
1 verbose cli [ 'node', '/usr/local/bin/npm', 'link', '--production' ]
2 info using npm@2.5.1
3 info using node@v0.12.0
4 verbose linkPkg /Users/nw/projects/reapp/react-tappable
5 verbose gentlyRm vacuuming /usr/local/lib/node_modules/react-tappable
6 silly gentlyRm purging /usr/local/lib/node_modules/react-tappable
7 silly gentlyRm quitting because other entries in /usr/local/lib/node_modules
8 verbose gentlyRm vacuuming /usr/local/lib/node_modules/react-tappable
9 verbose link build target /usr/local/lib/node_modules/react-tappable
10 verbose install where, what [ '/Users/nw/projects/reapp/react-tappable', [] ]
11 verbose install where, deps [ '/Users/nw/projects/reapp/react-tappable', [] ]
12 verbose stack TypeError: Cannot read property 'react' of undefined
12 verbose stack     at /usr/local/lib/node_modules/npm/lib/install.js:157:33
12 verbose stack     at Array.forEach (native)
12 verbose stack     at /usr/local/lib/node_modules/npm/lib/install.js:156:50
12 verbose stack     at /usr/local/lib/node_modules/npm/lib/install.js:324:22
12 verbose stack     at evalmachine.<anonymous>:265:20
12 verbose stack     at OpenReq.Req.done (/usr/local/lib/node_modules/npm/node_modules/graceful-fs/graceful-fs.js:141:5)
12 verbose stack     at OpenReq.done (/usr/local/lib/node_modules/npm/node_modules/graceful-fs/graceful-fs.js:61:22)
12 verbose stack     at FSReqWrap.oncomplete (evalmachine.<anonymous>:99:15)
13 verbose cwd /Users/nw/projects/reapp/react-tappable
14 error Darwin 14.1.0
15 error argv "node" "/usr/local/bin/npm" "link" "--production"
16 error node v0.12.0
17 error npm  v2.5.1
18 error Cannot read property 'react' of undefined
19 error If you need help, you may report this error at:
19 error     <http://github.com/npm/npm/issues>
20 verbose exit [ 1, true ]
nmn commented 9 years ago

React 0.13 is out so, there should be a better way to do this now.

JedWatson commented 9 years ago

Making React a peerDependency stops it from being installed in the package itself. This is a misunderstanding of how package.json and peerDependencies works.

The solution is to be more lenient with the required version; I'll bump it.

natew commented 9 years ago

But if I understand correctly you wouldn't use react-tappable by itself, you'd import it into another project and use it there. In a sense it's like a plugin in that you wouldn't want a React sub-dependency.

At least that's how I've used it.

JedWatson commented 9 years ago

You would, but it stands alone as well for examples, build, etc.

Using both peerDependencies and devDependencies together would probably solve the problem, but there are other issues with peerDependencies as well. Making the dependency version >=0.12.0 also fixes the issue, with less side effects.

JedWatson commented 9 years ago

One of the side effects is that if React is a peerDependency then npm link will always break. react-tappable will never be able to resolve require("react").

Having said that, npm link will also cause the double-react issue 100% of the time; I haven't figured out a good way around that yet :(

natew commented 9 years ago

Hmm. I don't run tests on a lot of stuff I link in, so maybe I never ran into it. The double react issue is frustrating, I have explicit instructions on avoiding that in some repos.

React-router uses both as well, maybe they have something working better.

natew commented 9 years ago

Actually, looks like react-router does devDependencies and peerDependencies, which may be what does it.