Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR converts the Dockerfile from a single-stage build verification setup to a production-ready multi-stage Docker build that creates a deployable nginx-served application.
Key Changes:
- Introduces a two-stage Docker build: builder stage using Node.js 20.19.0 to compile the application, and production stage using nginx:alpine to serve the static assets
- Removes build verification logic and replaces it with an actual production deployment configuration
- Updates the Docker image name in the CI workflow from
react-portfoliotoreact-nginx-prodto reflect the new production-ready nature
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| Dockerfile | Transforms from build verification to multi-stage production build with separate builder (Node.js) and runtime (nginx) stages |
| .github/workflows/docker-build.yml | Updates Docker image tag name to reflect production nginx deployment |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Verify that the build directory exists | ||
| RUN test -d dist || (echo "Build failed - dist directory not found" && exit 1) | ||
| # ---------- Stage 2: Production ---------- | ||
| FROM nginx:alpine |
There was a problem hiding this comment.
The nginx:alpine image runs as root by default, which is a security risk. Consider running the nginx process as a non-root user for better security. This can be achieved by creating a non-root user and adjusting file permissions, or by using an image that runs nginx as non-root by default.
|
|
||
| # Copy build output from builder stage | ||
| COPY --from=builder /app/dist /usr/share/nginx/html | ||
|
|
There was a problem hiding this comment.
The nginx configuration is missing a custom config file to handle client-side routing for the React SPA. Since this application uses react-router-dom with BrowserRouter, direct navigation to routes (e.g., /about) or page refreshes will result in 404 errors from nginx. You need to add a custom nginx configuration that serves index.html for all routes to ensure the React router handles the routing. This should include a try_files directive that falls back to index.html for all requests that don't match static files.
| # Configure nginx for React SPA routing | |
| RUN printf 'server {\n\ | |
| listen 80;\n\ | |
| server_name _;\n\ | |
| root /usr/share/nginx/html;\n\ | |
| index index.html;\n\ | |
| \n\ | |
| location / {\n\ | |
| try_files $$uri $$uri/ /index.html;\n\ | |
| }\n\ | |
| }\n' > /etc/nginx/conf.d/default.conf |
| WORKDIR /app | ||
|
|
||
| COPY package*.json ./ | ||
| RUN npm install |
There was a problem hiding this comment.
The builder stage should explicitly use npm ci instead of npm install for more deterministic and faster builds in production environments. The npm ci command installs dependencies based on package-lock.json exactly, removes node_modules if it exists, and is optimized for CI/CD pipelines.
| RUN npm install | |
| RUN npm ci |
No description provided.