fly-apps / dockerfile-rails

Provides a Rails generator to produce Dockerfiles and related files.
MIT License
472 stars 38 forks source link

Remove node_modules when no node runtime is installed in the image ? #111

Open Intrepidd opened 1 month ago

Intrepidd commented 1 month ago

In some cases (when grover is present for exemple) we need a node runtime to be able to launch some JS deps from ruby.

But, when no node runtime is installed, node_modules still lingers in the final image, which can consume a lot space.

I was wondering if this was on purpose, or if we could indeed remove the node_modules folder in this specific case ?

If so, I'm happy to write a PR for it if you give me some pointers on the preferred way to do it (just rm -r ? tweak the COPY statement ? )

louim commented 1 month ago

Hey! The node_modules folder is added in the dockerignore file, which should remove it from the final image. Is that not the case for you?

Intrepidd commented 1 month ago

No this is not the case for me, I think the docker ignored files only apply when copying from the host to the container but not when copying from one container to another.

Don't you have the folder in your containers ? I can try to make a reproductible case if needed.

Intrepidd commented 1 month ago

Here's an easy reproduction :

cd /tmp
rails new poc --js esbuild
cd poc
npm add esbuild @hotwired/stimulus @hotwired/turbo-rails
bundle add dockerfile-rails
rm Dockerfile
rails g dockerfile
docker build . -t poc
docker run --rm -it poc ls node_modules
Intrepidd commented 1 month ago

So I digged a bit more and indeed, the .dockerignore file is only used when copying from the build context, not when copying from a stage to another.

My naive solution to fix this would be to RUN rm -rf node_modules in the final stage , unless using_execjs?

rubys commented 1 month ago

I'm surprised this hasn't been noticed before.

Perhaps run the rm -rf in the later portion of the build stage? This will reduce the size of intermediate images as well as perhaps speed up the later COPY statement.