search

justinmc / react-audio-player

A simple React wrapper on the HTML5 audio tag
MIT License
587 stars 103 forks source link

"prop-types" not defined as a dependency. #41

Closed odub closed 6 years ago

odub commented 7 years ago

Just got the following error from webpack, trying to build react-audio-player into a project:

    ERROR in ./~/react-audio-player/dist/bundle.js
    Module not found: Error: Cannot resolve module 'prop-types' in /home/ubuntu/my-project/node_modules/react-audio-player/dist
     @ ./~/react-audio-player/dist/bundle.js 1:4276-4297

This should fix it – prop-types needs to be listed in dependencies as well as in peerDependencies.

odub commented 7 years ago

The error entered here: https://github.com/justinmc/react-audio-player/commit/75f815bcdbb510dbf801ef2bf5a7532a715bc37e

justinmc commented 7 years ago

Hmm, do you have prop-types installed in your application? I thought it should work without errors as a peer dependency but I could be wrong. A fresh create-react-app with this project added seems to work for me.

Maybe I'm getting carried away with the peer dependencies though. Just to check another more stable project, I see that React Router does list it as a dependency like you suggest.

Let me know what you think. Also is a conflict now since I just merged another PR. I think if you just bump the version to 15.6.0 it should be fine.

Oh and I just noticed, I think you meant to put it in dependencies and not devDependencies, right? This would be the first dependencies entry I guess.

Thanks for the contribution!

odub commented 7 years ago

Oops, yes you're right that it should be in dependencies. I'll rebase over the new changes and fix the placement slip.

With create-react-app it'll work because prop-types will be in the node_modules dir from being present at another point in the dependency tree. In a project where it's not there by virtue of another dependency requiring it, you should be able to see the error.

I don't have prop-types in my application because I'm using an older version of React.

Based on the information here, it seems also like the behaviour of peerDependencies changed in version 3. In the more recent versions, it seems like peerDependencies only act to express version compatiblity, rather than specifying that the dependency is required to be downloaded.

odub commented 7 years ago

Ok, done! I also pushed up react in the same way for consistency.

justinmc commented 6 years ago

Ah sorry I forgot to follow up on this. I'll go ahead and merge and will release a new version with this soon.

justinmc commented 6 years ago

Should be released and good to go in v0.6.4!

odub commented 6 years ago

Thanks