Add Dockerfile for client + fixes - #24
Conversation
WalkthroughThis pull request introduces Docker containerization infrastructure for the client application, including a multi-stage Dockerfile build process, nginx web server configuration, environment configuration examples, and a corresponding build script in the root package.json. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
package.json (1)
9-11: Optionally wire common VITE_ build args into the script*The script correctly builds from the monorepo root against
apps/client/Dockerfile. For smoother DX, you might consider passing the commonVITE_API_URL,VITE_APP_URL, andVITE_BETTER_AUTH_URLvalues here via--build-argsobun run docker:build:clientproduces a fully configured image without extra flags, mirroring the values in.env.example.apps/client/nginx.conf (1)
29-34: Make /health response type explicitlytext/plainIf you want
/healthto reliably returntext/plain(rather than nginx’s default MIME type), consider usingdefault_typeinstead ofadd_header Content-Type:location /health { - access_log off; - return 200 "healthy\n"; - add_header Content-Type text/plain; + access_log off; + default_type text/plain; + return 200 "healthy\n"; }This makes the intent explicit and avoids relying on header overrides for the content type.
apps/client/Dockerfile (1)
7-11: Consider pinning the Bun base image/tag and hardening package installUsing
oven/bun:latestplusapt-getties you to both the upstream image’s distro choice and whatever “latest” becomes over time. If the base image ever switches away from a Debian-like distro,apt-getwill break; and unpinned “latest” can yield non-reproducible builds.You may want to:
- Pin to a specific Bun tag that you test against (e.g., a versioned Debian-based image).
- Add
--no-install-recommendsto theapt-get installto keep the base layer small.For example:
-FROM oven/bun:latest AS base +FROM oven/bun:<known-good-tag> AS base ... -RUN apt-get update -y && apt-get install -y openssl && rm -rf /var/lib/apt/lists/* +RUN apt-get update -y \ + && apt-get install -y --no-install-recommends openssl \ + && rm -rf /var/lib/apt/lists/*
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
.gitignore(1 hunks)apps/client/.env.example(1 hunks)apps/client/Dockerfile(1 hunks)apps/client/nginx.conf(1 hunks)package.json(1 hunks)
🔇 Additional comments (2)
.gitignore (1)
31-32: Fix typo in comment.Line 31 has a minor grammatical error: "dont" should be "don't".
-# We dont need generated Prisma files +# We don't need generated Prisma files prisma/generated/Otherwise, the addition correctly excludes Prisma-generated artifacts from version control, which aligns with the PR objective.
apps/client/.env.example (1)
1-8: Env example is clear and consistentExample values and comments are consistent with the Vite/Docker build-time model and provide sensible local defaults; no changes needed here.
| # Set build-time environment variables (can be overridden during build) | ||
| ARG VITE_API_URL | ||
| ARG VITE_APP_URL | ||
| ARG VITE_BETTER_AUTH_URL | ||
|
|
||
| ENV VITE_API_URL=${VITE_API_URL} | ||
| ENV VITE_APP_URL=${VITE_APP_URL} | ||
| ENV VITE_BETTER_AUTH_URL=${VITE_BETTER_AUTH_URL} | ||
|
|
There was a problem hiding this comment.
Provide defaults for VITE_ build ARGs to avoid empty config in builds*
Right now the ARG values default to empty, so running docker build ... (or bun run docker:build:client) without explicit --build-arg flags will bake empty VITE_API_URL, VITE_APP_URL, and VITE_BETTER_AUTH_URL into the client build.
You can make the image usable out of the box by giving these ARGs sane defaults that match .env.example, while still allowing overrides:
-# Set build-time environment variables (can be overridden during build)
-ARG VITE_API_URL
-ARG VITE_APP_URL
-ARG VITE_BETTER_AUTH_URL
-
-ENV VITE_API_URL=${VITE_API_URL}
-ENV VITE_APP_URL=${VITE_APP_URL}
-ENV VITE_BETTER_AUTH_URL=${VITE_BETTER_AUTH_URL}
+# Set build-time environment variables (can be overridden during build)
+ARG VITE_API_URL=http://localhost:9999
+ARG VITE_APP_URL=http://localhost:80
+ARG VITE_BETTER_AUTH_URL=http://localhost:9999/api/auth
+
+ENV VITE_API_URL=${VITE_API_URL}
+ENV VITE_APP_URL=${VITE_APP_URL}
+ENV VITE_BETTER_AUTH_URL=${VITE_BETTER_AUTH_URL}This matches the documented defaults and reduces surprises when building locally.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Set build-time environment variables (can be overridden during build) | |
| ARG VITE_API_URL | |
| ARG VITE_APP_URL | |
| ARG VITE_BETTER_AUTH_URL | |
| ENV VITE_API_URL=${VITE_API_URL} | |
| ENV VITE_APP_URL=${VITE_APP_URL} | |
| ENV VITE_BETTER_AUTH_URL=${VITE_BETTER_AUTH_URL} | |
| # Set build-time environment variables (can be overridden during build) | |
| ARG VITE_API_URL=http://localhost:9999 | |
| ARG VITE_APP_URL=http://localhost:80 | |
| ARG VITE_BETTER_AUTH_URL=http://localhost:9999/api/auth | |
| ENV VITE_API_URL=${VITE_API_URL} | |
| ENV VITE_APP_URL=${VITE_APP_URL} | |
| ENV VITE_BETTER_AUTH_URL=${VITE_BETTER_AUTH_URL} | |
🤖 Prompt for AI Agents
In apps/client/Dockerfile around lines 29–37, the ARGs VITE_API_URL,
VITE_APP_URL and VITE_BETTER_AUTH_URL are declared without defaults so builds
without --build-arg produce empty baked-in values; update each ARG declaration
to include the sensible default values from .env.example (for example the
documented local/dev defaults) so the ENV lines continue to pick up those ARGs
while still allowing overrides via --build-arg.
|
Thanks for the PR! Will have a look asap |
You can try it out with
bun run docker:build:clientcommand.Note: if you serve the client with a reverse proxy, probably gzip is not required in
nginx.conf, but on the reverse proxy.Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.