Data4Democracy / internal-displacement

Studying news events and internal displacement.
43 stars 27 forks source link

Minor cleanup of dependencies #122

Closed domingohui closed 7 years ago

domingohui commented 7 years ago

Not sure why react-router is included twice.

Maybe we can also move react-bootstrap and bootstrap to devDependencies?

wwymak commented 7 years ago

no, react-bootstrap and bootstrap should not be in the dev dependencies as they are for front end styling and not part of development work.

wwymak commented 7 years ago

also seems a bit odd that the original seed repo has stuff like react-router in the devDependencies. They should be in the dependencies instead... I know this is more a prototype thing rather than a full production build but we may as well stick with good coding principles where we can.

So apologies @domingohui but I don't think this PR is the most useful thing to do right now-- if anything we should be moving stuff from devDependencies into dependencies and not the other way round ;)

domingohui commented 7 years ago

Thanks for the input @wwymak ! I thought webpack would bundle all assets including front end styling like bootstrap. So I have the impression that we can just build it with webpack, and FE dependencies don't have to be downloaded in production. Please correct me if I'm wrong!

domingohui commented 7 years ago

Or we need most of the things (excl. testing frameworks) in dependencies because we need to use those for a production build using webpack eventually? In this case, it makes more sense to include everything in dependencies then :D

wwymak commented 7 years ago

Webpack does put in all the stuff that you need in a bundle, but the npm dependencies are not what the client is loading. npm modules specify what you would need in production code, whereas dev dependencies are what would help you develop the app, e.g packages that watch for changes, build tools such as web pack etc. So while the final bootstrap stuff is in a bundle, they should still be in the proper dependencies. (well this is my opinion anyway)

domingohui commented 7 years ago

I see. Guess we will need to clean up the dependencies later on then :)