Skip to content

🔧(docker) add a service in compose to frontend development #1033

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lunika
Copy link
Member

@lunika lunika commented Jun 2, 2025

Purpose

We want a serice in compose starting the frontend application in development mode. We want to take the advantage of the hot reload module, so the sources are mounted inside the container.

Proposal

  • 🔧(docker) add a service in compose to frontend development

We want a serice in compose starting the frontend application in
development mode. We want to take the advantage of the hot reload
module, so the sources are mounted inside the container.
@lunika lunika requested a review from AntoLC June 2, 2025 19:31
@lunika lunika self-assigned this Jun 2, 2025
@@ -128,7 +128,7 @@ run-backend: ## Start only the backend application and all needed services
run: ## start the wsgi (production) and development server
run:
@$(MAKE) run-backend
@$(COMPOSE) up --force-recreate -d frontend
@$(COMPOSE) up --force-recreate -d frontend-development
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work, can you amend the build-frontend make target, too, so that it builds both services?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only the port 3000 is available, so starting both containers will cause problems.

image: impress:frontend-development
volumes:
- ./src/frontend:/home/frontend
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is lot of side effects by keeping the node_modules sync, I think it is common pattern to exclude it:

Suggested change
- ./src/frontend:/home/frontend
- ./src/frontend:/home/frontend
- /home/frontend/node_modules
- /home/frontend/apps/impress/node_modules

@AntoLC AntoLC requested a review from Copilot June 5, 2025 07:52
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Adds a new Docker Compose service for frontend development to enable hot reload by mounting local sources, and updates Makefile targets to use that service.

  • Introduces frontend-development service in docker-compose with build args, volume mount, and port mapping
  • Updates Makefile run alias to start the dev service instead of production frontend, and adjusts run-frontend-development to stop containers before launching

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
docker-compose.yml Expose production frontend image/ports and add frontend-development service with build, volume, ports
Makefile Change run to bring up frontend-development and update run-frontend-development stop logic
Comments suppressed due to low confidence (2)

docker-compose.yml:174

  • The image key for frontend-development is indented with 6 spaces, but service-level keys should use 4 spaces. This misalignment can cause a YAML parse error.
      image: impress:frontend-development

Makefile:319

  • The run-frontend-development target stops both services but never starts the frontend-development container. To leverage the new Compose service for hot reload, add @$(COMPOSE) up --force-recreate -d frontend-development before running yarn dev.
cd $(PATH_FRONT_IMPRESS) && yarn dev

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.

3 participants