Use NODE_ENV ARG to control NPM install and tests

[orangejulius]

By using the Dockerfile ARG directive, we can set an ENV variable and control it via the --build-arg parameter to docker build.

Using this we can configure the Dockerfile to skip installing NPM dev dependencies and running unit tests.

In my testing this reduces the size of the Placeholder docker images from 290MB to 200MB, which is pretty huge! It reduces the time to build the image, reduces our surface area for security issues, and seems like an all around win.

[missinglink]

I actually like the idea of running tests when building the image, simply because they ensure that the environment is valid.

Eg. If I dev a feature on my mac, all the tests pass, I generate the image successfully and push it to dockerhub, we later find out that the docker image was missing a dynamically linked C lib and so it doesn't work as expected.

Totally fine with removing the dev dependencies 👍

[missinglink]

We can avoid downloading and installing the 50MB+ phantomjs package by pinning it to jshint@2.9.5

[orangejulius]

We need the devDependencies to run the tests though.

The workflow I had in mind was one where only the production images skip the tests/devDependencies. Since we always merge master->staging->production, there would be equivalent[1] images with/without the devDependencies/tests run for any change.

This ensures there exists a minimal, fast, secure image for production use, but also that there is a dev image for deeper exploration.

[1] The commit hash isn't identical right now, because we generate merge commits when going from master->staging and staging->production. With a little work we could do away with that, and then it would be very clear that two images are identical except for missing dev dependencies

[missinglink]

So the issue we are trying to solve here is to not include dev dependencies in the docker image?
There is a separate/linked discussion of whether we want to run the tests inside the docker build.

I would tend towards something that is very simple, and having different environments during dev and production doesn't sound like a good idea to me.

If the problem we are trying to solve is the size of the dev dependencies then the simplest solution without compromising any functionality and also not not adding any complexity to the workflow is simply:

npm i && npm t && npm prune --production

ref: https://docs.npmjs.com/cli/prune

[orangejulius]

That's true, that would be simple. A downside there however, is we can't use our current system where NPM install is not needed at all when only code, but not package.json would change.

This would make local development significantly slower: every rebuild of an image locally would have to pay the cost of npm install (with dev dependencies) each time. This can easily take 30-60 seconds.

As it stands, with only code changed, the time to build a new docker image is approximately equal to the time to run npm test, which is often only a few seconds.

I also think there is value in having docker images with testing tools, as well as value having docker images that are as lean as possible. Having both lets us optimize for different situations.

[missinglink]

jshint/jshint#3316

[orangejulius]

It seems that once the jshint issue (referenced above) is resolved, and we roll out the changes to remove semantic-release from devDependencies (pelias/api#1187), this may not matter as much, as the total size of the devDependencies might not be much larger than installing only dependencies.