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

Meteor 1.3 build process + user-installed packages #90

Open
jacobdr opened this issue Apr 4, 2016 · 7 comments
Open

Meteor 1.3 build process + user-installed packages #90

jacobdr opened this issue Apr 4, 2016 · 7 comments

Comments

@jacobdr
Copy link

jacobdr commented Apr 4, 2016

I am a little bit confused as to how the new module process works in Meteor 1.3, and after scouring Github and the Meteor forum for a definitive answer, I thought that maybe I could get some clarification here. This stems from a failing build using meteord:onbuild that previously worked using only Atmosphere/meteor packages.

I am trying to understand how exactly user-installed npm modules (of the meteor npm install --save variety) are installed during the meteor build process. My reading of the literature so far seems to suggest that the local node_modules directory is copied over into the build environment, but that seems like it doesnt make sense because it would hurt cross-platform compatibility and mess up any package post-install build scripts that build from source. Also, this seems like a bad idea because it makes the the build context MUCH MUCH larger -- node_modules should ultimately be excluded in both the .gitignore and .dockerignore files.

Anyone have any insight into the best way to combine Docker with Meteor 1.3?

@jacobdr
Copy link
Author

jacobdr commented Apr 5, 2016

Pinging @benjamn to see if he can offer some insight as a core developer. Question: can you describe at a high-level how user-installed node_modules are installed in a Meteor 1.3 project during the build process? Or if you have been Dockerizing meteor at all, any best-practices that you follow?

I would like to add node_modules to my .dockerignore (reduces size of the build context dramatically) and .gitignore files (dependencies should be vendored through package.json npm i), but this breaks the current build process established in this repo -- which seems to be canonical meteor-docker resource.

I saw that you were responsible for many of the commits to the import-scanner, which might be relevant to the conversation.

@jshimko
Copy link
Member

jshimko commented Apr 5, 2016

We need to add an npm install --production right here before the meteor build...

https://github.com/meteorhacks/meteord/blob/master/base/scripts/lib/build_app.sh#L10

That would install your app level NPM packages before building the app (the --production flag will make sure it skips anything in your devDependencies).

Although I have to admit, I'm not sure why Meteor >= 1.3 doesn't do that for you when you run meteor build. I can't think of a context in which that would not be desired (other than the absence of a package.json in the app root).

And you're right, you should definitely .dockerignore the node_modules directory.

@benjamn
Copy link

benjamn commented Apr 5, 2016

For the client, we build a bundle of just what was imported, so the contents of node_modules no longer matter at runtime. I'm guessing your use case is mostly for server-side modules, though, so this paragraph many not be particularly relevant.

On the server, the story is a bit more complicated. During meteor build, the contents of local node_modules directories are copied into bundle/programs/server/npm/node_modules/, so the original node_modules directories can be ignored safely as long as you don't delete them before you run meteor build.

The contents of bundle/programs/server/npm/node_modules/ remain important at runtime, because we attempt to use Node to load modules directly from the file system, to achieve maximal compatibility with Node.

However, if require.resolve fails in pure Node, we fall back to using a bundle that is very similar to what we use on the client, except built for the server (so it follows "main" fields in package.json files instead of the optional "browser" field, for example). So you may be able to get away with deleting/ignoring some package directories from bundle/programs/server/npm/node_modules/, if those packages don't have any binary dependencies, and don't try to examine the filesystem directly by doing things like fs.readFile(path.join(__dirname, ...)). Packages that really need to be run in pure Node are relatively few and far between, so I suspect most packages under bundle/programs/server/npm/node_modules/ could be deleted, though it's difficult to tell automatically which ones are safe to remove.

Given that, you should probably not blindly add node_modules or bundle/programs/server/npm/node_modules/ to your .dockerignore file, but you may be able to selectively ignore some packages that don't really need to be run with Node.

@jshimko
Copy link
Member

jshimko commented Apr 5, 2016

Thanks for elaborating on that @benjamn!

I've been dockerignore'ing my node_modules folder since 1.3 betas came out and I always run the npm install I mentioned above inside the container (pre meteor build). Otherwise, any time I've let NPM dependencies into the container from my OSX dev environment I eventually run into build snags. The scenario I described above is the only way I've tried that seems to reliably work.

Also, I think a container build should be able to work directly from a git clone (where node_modules definitely should not be), so that has always been part of my motivation for those choices too. For example, I have CircleCI build my containers from a git clone. I could obviously run npm install on the CI box before the docker build, but I like having the process happen entirely inside the container so it's happening the exact same way on CI as it is anywhere else I do the build.

Is there a downside to doing this that I'm not seeing/understanding?

@benjamn
Copy link

benjamn commented Apr 5, 2016

We would like to improve the process of (re)(installing/building) npm packages on the server you're deploying to, but we need to work out the details.

Running npm install again with the same package.json file can give you different package versions (and that's assuming there's a package.json file at all). Perhaps something like generating a npm-shrinkwrap.json file and re-running npm install from that would work.

Another complication is the possibility of private (or purely local) packages that can't simply be redownloaded from npm. Private packages hosted by npm just need an NPM_TOKEN environment variable, but I'm not sure what to do about packages not hosted by npm.

None of these problems are unsolvable, but coming up with perfectly general solutions may be tricky.

@markshust
Copy link

@benjamn thanks for all the notes. going to digest this and see if there is a better process.

just an FYI, I wrote a blog post and sample docker image for how I'm dealing with npm install. i wrote this on feb 1 and have been deploying to production many, many times with it and it seems rock solid with absolutely zero issues. perhaps this process can be implemented into meteord/mup.

http://markshust.com/2016/02/01/creating-custom-production-docker-image-meteor-13

@jacobdr
Copy link
Author

jacobdr commented Apr 6, 2016

Really happy to see the progress made on this issue today. Thanks for everyone for chiming in.

@benjamn -- Thanks for your great insights. Obviously a complicated process, and something that generally "just works" for users of Meteor, when in reality there is a lot happening under the covers

@jshimko thanks for the specific suggestion -- I forked meteord and made that change. Will test it out tomorrow and submit a PR with documentation. Would appreciate your help in testing it out if you are available tomorrow.

@markoshust I will also try to incorporate your ideas, especially when it comes to simplifying the build process in general and moving as much of it as possible to the Dockerfile. That declarative style I think ultimately helps with legibility and maintainability.

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

4 participants