Skip to content

feat: Update docker file#1225

Open
Assem-Uber wants to merge 3 commits intocadence-workflow:masterfrom
Assem-Uber:feature/update-docker-image
Open

feat: Update docker file#1225
Assem-Uber wants to merge 3 commits intocadence-workflow:masterfrom
Assem-Uber:feature/update-docker-image

Conversation

@Assem-Uber
Copy link
Copy Markdown
Contributor

Summary

The previous Alpine-based image was unreliable for some IDE dev containers. This change moves to Debian Bookworm slim, adds ca-certificates so install-idl (git over HTTPS) works inside the image, and defers npm to after the container starts so a failed install no longer blocks the dev container from coming up. Image size may grow slightly, though still in acceptable range.

Changes

  • Dev containers / IDE compatibility: Base image and layout adjusted so dev containers work consistently across environments that struggled with the old image.
  • ca-certificates + git: Installed via apt-get so TLS verification succeeds when install-idl runs in-container.
  • Dev stage: No longer runs npm ci / dependency install as part of the image build; npm can be run after start, avoiding failed installs blocking container startup.
  • Build: Removed the extra npm run build step before build-standalone (standalone path is sufficient).
  • Docker ENV: Updated to the modern ENV KEY=value form to clear the old ENV format warning.

Testing

  • Open the repo in a dev container in the IDEs that previously failed; confirm the container starts even before npm install / npm ci.
  • Check size change:
Screenshot 2026-03-27 at 17 30 30 - After start, run npm ci (or your usual install) and confirm npm run install-idl completes without TLS/git errors. - Docker build default/production target and smoke-test the running app. - Confirm Docker build logs no longer show the ENV deprecation/warning (screenshot for old warning): Screenshot 2026-03-27 at 18 47 19

@gitar-bot
Copy link
Copy Markdown

gitar-bot bot commented Mar 30, 2026

Code Review ✅ Approved 3 resolved / 3 findings

Docker configuration updates optimize image size by removing apt-get cache and improve build clarity with environment variable fixes and dependency installation in the dev stage. All previously identified issues have been resolved.

✅ 3 resolved
Bug: Dev stage has no node_modules and no install step

📄 Dockerfile:3-8
The dev stage copies the source tree (COPY . .) but never runs npm ci or installs dependencies. The old Dockerfile copied node_modules from a deps stage. Now the dev container will start with no node_modules at all — the app cannot run or be linted/typechecked until the developer manually runs npm ci and npm run install-idl after container start.

The PR description says this is intentional ("defers npm to after the container starts so a failed install no longer blocks the dev container from coming up"), so this may be acceptable. However, it means the dev container is non-functional out of the box — there's no CMD, entrypoint script, or README hint that tells the developer what to run. Consider at minimum adding a comment or a simple entrypoint script that runs npm ci && npm run install-idl on first start.

Quality: ENV NEXT_TELEMETRY_DISABLED uses deprecated format

📄 Dockerfile:24 📄 Dockerfile:37
Line 24 still uses the old ENV KEY VALUE syntax (ENV NEXT_TELEMETRY_DISABLED 1), which triggers a BuildKit deprecation warning. The PR description mentions fixing this warning, and line 37 has the same pattern. Both should use the modern ENV KEY=VALUE form.

Quality: apt-get cache not cleaned, bloating image layers

📄 Dockerfile:5-6 📄 Dockerfile:14-15
Both the dev and builder stages run apt-get update && apt-get install but never clean up the package lists. This leaves the /var/lib/apt/lists cache in the layer, adding ~30-40 MB to each stage. The builder stage is particularly impactful because it feeds the production runner stage indirectly (though only via COPY, so the layer bloat stays in builder itself). For the dev stage, which is used as a dev container, the extra size persists in the running image.

This is a standard Dockerfile best practice for Debian-based images.

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

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

Successfully merging this pull request may close these issues.

1 participant