Skip to content
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

Should I not .gitignore the node_modules directory? #93

Closed
krishamoud opened this issue Apr 12, 2016 · 5 comments
Closed

Should I not .gitignore the node_modules directory? #93

krishamoud opened this issue Apr 12, 2016 · 5 comments

Comments

@krishamoud
Copy link

I just started using meteor 1.3 and I built an app here https://github.com/krishamoud/asteroidbelt which is gitignoreing node_modules and the settings.json file. When I run meteord locally with the node_modules already installed then everything is fine and it builds the same as it always has. When I do a remote build it fails and tells me that there are unresolved imports. Specifically react.

Is this expected behavior?

@jshimko
Copy link
Member

jshimko commented Apr 12, 2016

That is unfortunately the expected behavior with the current version of meteord. It's because npm install is not being run before meteor build inside the container (needs to happen right here). If you don't already have the node_modules folder in your source, it won't end up being created in the container. It's not a problem if you build locally from your dev setup, but a build from a fresh git clone will not work.

This has been discussed recently and I personally think we should be adding npm install --production to the build script. But not everyone agrees, so I guess everyone is just going to have to do what works for their project. In my case, I forked meteord and added that line to the build script because I need CircleCI to be able to build my images from a git clone. I also could have added an npm install to my CI config right before the docker build, but the whole point of Docker (to me) is to have builds work the exact same way regardless of where they're run.

If you want what the changes described above, feel free to use my repo (meteor-one-three branch)
https://github.com/jshimko/meteord/tree/meteor-one-three

Or you can just use the image I built from that by adding this to your Dockerfile...

FROM jshimko/meteord:onbuild

(devbuild and binbuild will work too)

And to answer your original question... no, you definitely don't want to commit node_modules to your repo. Add FROM jshimko/meteord:onbuild to your Dockerfile and you won't have to. ;)

@krishamoud
Copy link
Author

Great! Thanks for the reply! I'll definitely be using your image for my project. Is it backwards compatible with meteor version pre-1.3?

@jshimko
Copy link
Member

jshimko commented Apr 12, 2016

Well, it depends. The only functionality change is the npm install, so it shouldn't effect anything other than that. However, running npm install in a directory without a package.json will definitely throw an error (which will likely tank the build). So that said, you could just continue using the official meteord for Meteor versions before 1.3 that don't make use of app level NPM packages.

Now that you mention it, I could've made that conditional so it only runs when a package.json is present. Give me a few and I'll update it.

@jshimko
Copy link
Member

jshimko commented Apr 12, 2016

All set. Now it'll only run npm install if your app has a package.json in the root.

@krishamoud
Copy link
Author

Thanks for the quick replies! This is great!

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

2 participants