Adding docker support #522
Conversation
dockerizeallthethings
commented
Dec 27, 2017
dockerizeallthethings
commented
- Adding docker image files for both prod and dev use
- Adding instructions to top level README.md to:
- Run a copy of the prod version hosted on hub.docker to try out the project
- Build and run an image that will listen to changes on the host machine (to allow developers to use an IDE on their host machine)
…ple in prod mode as a docker image
…mage with `docker build -f Dockerfile-prod -t <your image here> .` - The Dockerfile is now for development mode. - Adding additional steps for running in development mode, with support for hot reloading.
|
@dockerizeallthethings, thanks for the PR! Could you open an issue and link it to this PR? In general, we like to have a discussion about what goes into the project and an issue is the perfect place to do that. Cheers. |
|
Thanks for the PR! Dumb question: How would we update the Docker image hosted on hub.docker later? |
|
If you have accounts on hub.docker I should be able to add them as
contributors.
Also, you're certainly welcome to host your own version on hub.docker, if
you'd prefer.
Lemme know if you have any other questions, and feel free to send me some
hub.docker usernames to add :).
…--RG
On Sat, Dec 30, 2017 at 7:46 PM, Cory House ***@***.***> wrote:
Thanks for the PR! Dumb question: How would we update the Docker image
hosted on hub.docker later?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#522 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/Ago6YtvXGK21FEUl5djdE8H4Cob7IZ1Aks5tFwOigaJpZM4RNHOH>
.
|
|
Great! I'm |
|
Done!
I also created an issue linked to the PR, as per an request from a Nick
Taylor (to which I can't seem to directly respond).
Happy New Year!
…--RG
On Mon, Jan 1, 2018 at 2:51 PM, Cory House ***@***.***> wrote:
Great! I'm housecor
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#522 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/Ago6YmRCAcaVBWIwfuXZcHnT13lRe4Qqks5tGWFUgaJpZM4RNHOH>
.
|
|
I don't know much about docker so I cannot approve or deny of these changes. I am definitely excited about them though. |
|
I actually do something very similar to this already for a few projects, it helps to alleviate the complexities with managing various versions of nodejs and npm packages on my mac. I mount the root of the project folder into the container so i can then run the build if i wanted. |
|
This is for implementing #529. |
|
I haven't tried this locally yet, will try to take a peek at this tomorrow. Just curious why alpine instead of slim for Node? I haven't done a whole lot of Docker yet. New job's immersing me though 😉 |
|
I also don't know Docker well enough yet to vet this. I'm hoping to dive in soon for work but for now I'm no help on this. |
There was a problem hiding this comment.
I have pulled this PR down to my local machine and ran the docker image/build, and there a few things I noticed.
- you can't interact with the console when the image is running in foreground mode, so that needs some documentation work done I think as well. Things like clearing snapshots, etc don't work, basically anything that requires console/terminal interaction while
npm run start -sis going. - However, once you bind the ports to the host, the app loads fine and HMR and all the features as best as I can tell work just as if you ran this natively with the exception of the item mentioned above.
|
@dockerizeallthethings, sorry that this PR and issue has gone stale. Any ideas for the first point that @mikedevita mentions? |
|
I'm not a huge fan of this for a couple of reasons:
If someone wants to develop using Docker, that should be put back on the developer, this PR is making A LOT of assumptions. For example, I use react-slingshot in a multi-project build, but my react-slingshot project has dependencies on other projects that have to be built first. Therefore, the Dockerfile has to change accordingly. Also, the development Dockerfile assumes that we want to expose port 3000 and 3001, it could be that someone is using docker-compose and Nginx to proxy to 3000 and 3001. In my opinion, this is a good start but not ready to merge. It's trying to accomplish too much. I'm going to FORK react-slingshot this weekend and try to implement some ENV configuration changes and I'll integrate Docker as well. I'm going to attempt to solve :
|
|
If anyone is curious about my Docker comment, check out the PR: #590 |
