Conversation
iulusoy
commented
Feb 26, 2026
- also updates of server name in nginx.conf
- also updates to image names and metadata in dockerfile, docker-compose.yamldeploy: update image name and server directory, set api url in docker…
There was a problem hiding this comment.
Pull request overview
This pull request updates the project branding from "onehealth-map-frontend" to "heiplanet-frontend" across deployment configurations, including Docker image metadata, nginx server paths, and docker-compose service definitions. The PR also introduces API URL configuration via environment variables.
Changes:
- Rebranded Docker image name and metadata from onehealth to heiplanet
- Updated nginx base path from /onehealth-map-frontend/ to /heiplanet-frontend/
- Added VITE_NUTS_API_BASE environment variable configuration in Dockerfile
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| nginx/conf/nginx.conf | Updated location block paths from /onehealth-map-frontend/ to /heiplanet-frontend/ |
| docker-compose.yml | Changed image name from onehealth-map-frontend:latest to heiplanet-frontend:main |
| Dockerfile | Updated image labels, added VITE_NUTS_API_BASE environment variable, and modified build command to pass env var |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| ENV PNPM_HOME="/pnpm" | ||
| ENV PATH="$PNPM_HOME:$PATH" | ||
| ENV VITE_NUTS_API_BASE="http://api:8000" |
There was a problem hiding this comment.
The vite.config.ts sets the base path using the VITE_BASE_PATH environment variable (defaulting to "/"), but this environment variable is not set in the Dockerfile. The nginx configuration serves the app from /heiplanet-frontend/, which suggests VITE_BASE_PATH should be set to "/heiplanet-frontend/" to ensure proper asset loading and routing.
Add: ENV VITE_BASE_PATH="/heiplanet-frontend/"
Without this, the built application will look for assets at the wrong paths (e.g., /assets/... instead of /heiplanet-frontend/assets/...).
| ENV VITE_NUTS_API_BASE="http://api:8000" | |
| ENV VITE_NUTS_API_BASE="http://api:8000" | |
| ENV VITE_BASE_PATH="/heiplanet-frontend/" |
| COPY frontend/ . | ||
| ENV NODE_ENV=production | ||
| RUN pnpm build | ||
| RUN VITE_NUTS_API_BASE=$VITE_NUTS_API_BASE pnpm build |
There was a problem hiding this comment.
The RUN command explicitly passes VITE_NUTS_API_BASE to the build, but since it's already defined as an ENV earlier in the same stage, this is redundant. The environment variable will already be available to the build process.
Consider simplifying to: RUN pnpm build
However, this may be intentional for clarity or to allow build-time overrides via build args.
| RUN VITE_NUTS_API_BASE=$VITE_NUTS_API_BASE pnpm build | |
| RUN pnpm build |
| services: | ||
| frontend: | ||
| image: ghcr.io/ssciwr/onehealth-map-frontend:latest | ||
| image: ghcr.io/ssciwr/heiplanet-frontend:main |
There was a problem hiding this comment.
The docker-compose.yml image tag has changed from "latest" to "main". While "main" is a common convention for development branches, it's worth verifying this aligns with your CI/CD pipeline and tagging strategy. If this is meant for production deployment, consider using semantic versioning tags or "latest" for stability.
| image: ghcr.io/ssciwr/heiplanet-frontend:main | |
| image: ghcr.io/ssciwr/heiplanet-frontend:latest |
|
|
||
| ENV PNPM_HOME="/pnpm" | ||
| ENV PATH="$PNPM_HOME:$PATH" | ||
| ENV VITE_NUTS_API_BASE="http://api:8000" |
There was a problem hiding this comment.
The API URL "http://api:8000" is a Docker internal hostname that won't work at runtime in the browser. Vite environment variables are baked into the built JavaScript bundle at build time, so they need to be valid URLs that the client browser can reach, not Docker internal hostnames.
Since the nginx configuration proxies /api/ to the backend service, the frontend should use a relative path like "/api" or construct the full URL based on the browser's location. Consider either:
- Removing this environment variable and relying on the default in nutsApi.ts with "/api" as a relative path
- Setting it to a relative path: ENV VITE_NUTS_API_BASE="/api"
- Using a build argument that can be overridden for different deployment environments
| ENV VITE_NUTS_API_BASE="http://api:8000" | |
| ENV VITE_NUTS_API_BASE="/api" |
There was a problem hiding this comment.
I also think this could work to connect the frontend and backend in production (but as the one cartesian request was/is already working, then changing doesn't make so much sense. I can make the NUTS request use the same approach as the backend one to avoid this issue, if the cartesian one is working now?)