Skip to content

Conversation

@lunika
Copy link
Member

@lunika lunika commented Jun 2, 2025

Purpose

When a new user come on the project, clone it and run make bootstrap, he is able to test the project but not starting directly to work on it, he also have to install all the frontend dependencies on its machine and then run the frontend application outside of the compose stack. Installing frontend dependencies is quite expensive and it has already made during the bootstrap but not reusable.

We change the strategy in this PR, the make bootstrap build all the images in frontend mode, there is no need anymore to re-install all frontend dependencies and run the frontend application outside the compose stack.

The legacy behaviour is maintained, you can still run only the backend application with compose and the frontend using your local node installation.

@lunika lunika requested a review from AntoLC June 2, 2025 19:31
@lunika lunika self-assigned this Jun 2, 2025
@AntoLC AntoLC requested a review from Copilot June 5, 2025 07:52
Copy link

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 
@lunika lunika force-pushed the feature/frontend-developement-container branch from 2a4510e to 118a5ae Compare June 11, 2025 07:39
@lunika lunika requested review from AntoLC and Kharonus June 11, 2025 07:40
@lunika
Copy link
Member Author

lunika commented Jun 11, 2025

@AntoLC @Kharonus I made other modification, the y-provider is run in dev mode and I removed all the services running application in production mode, we don't use them.

@lunika lunika force-pushed the feature/frontend-developement-container branch from 0adb6c0 to ca7055d Compare June 13, 2025 08:35
lunika added 5 commits June 13, 2025 10:38
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.
`app` and `celery` services are not used when we run the compose configuration. The compose file is only used for development purpose.
To have a better developer experience, the y-provider service run in dev mode, allowing hot reload when a file is modified. To avoid issue with shared node_modules, they are mounted in a separated volume to not have then in the local directory.
The usage of docker-compose.yaml file is deprecated, we can rename it in compose.yml
We want to run the e2e tests using the frontend and y-provider production images. We created a dedicated compose file adding just missing services. These services are built in the CI.
@lunika lunika force-pushed the feature/frontend-developement-container branch from ca7055d to 35c7b9e Compare June 13, 2025 08:39
lunika and others added 2 commits June 13, 2025 10:53
In order to speed the rebuild of images, the --no-cache option is removed by default. If we want to build the images without cache, the cache paramter must be used.
Without this, YdocConverter throws an error when developping.
@lunika lunika force-pushed the feature/frontend-developement-container branch from 35c7b9e to f97ab51 Compare June 13, 2025 08:53
Copy link
Collaborator

@AntoLC AntoLC left a comment

Choose a reason for hiding this comment

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

Looks good !

@lunika lunika merged commit f97ab51 into main Jun 13, 2025
20 of 21 checks passed
@lunika lunika deleted the feature/frontend-developement-container branch June 13, 2025 12:00
@AntoLC AntoLC mentioned this pull request Jul 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

5 participants