RocketChat / RC4Conferences

A set of scalable components for communities to build, manage, and run virtual conferences of any size.
https://conf.rceng.shop/conferences/c/1
24 stars 38 forks source link

[IMPROVE] Dockerfile to support hot-reload #74

Open SarveshLimaye opened 1 year ago

SarveshLimaye commented 1 year ago

Fixes : #48

Added volumes in docker-compose.yml to support hot reload in next-js

Dnouv commented 1 year ago

Hey @SarveshLimaye

Thanks for the PR. Did you also fix the BUILD_KIT issue? If not, can you please research more about it and try to come up with a solution?

SarveshLimaye commented 1 year ago

Hey @Dnouv , I have not fixed the BUILD_KIT issue. in this pr. The issue is still open on the docker/buildx github repository , so there is no official solution available to this. Currently, BuildKit only supports four network modes: "bridge", "host", "none" and "container:<name|id>". So I guess it's not possible to solve this error unless there is some support added in the official docker repository, but we can obviously find some way around.

After reading the discussions I found this way around to be super useful -

We can create a new sh and try to run it in DOCKERFILE. The sh can be similar to the following - https://github.com/moby/buildkit/issues/978#issuecomment-1060652480

Since there is no official solution available to this, we need to try out a lot of different ways on a separate branch. I think it would be better if we try to solve the BUILD_KIT issue on a different PR. Do let me know if you find any better solution. Thank you !

Dnouv commented 1 year ago

@SarveshLimaye ok, sounds good. I tried out this PR (with DOCKER_BUILDKIT=0). However, there were a few errors, and I was not able to build Docker Image. The error:

Error occurred prerendering page "/conferences/create/basic-detail". Read more: https://nextjs.org/docs/messages/prerender-error
FetchError: request to http://localhost:1337/api/top-nav-item failed, reason: connect ECONNREFUSED 127.0.0.1:1337

Please pull the latest changes from the main before trying to build. The localhost:1337 is already up and running.

Also, you will have to add a new dockerfile for the development environment, since as you can find here we are doing a production build, and AFAIK NextJS won't watch for file changes in the production build.

Please look into this issue. Thank you!

SarveshLimaye commented 1 year ago

@Dnouv I have added Docker file for development environment. Could you pls take a look at changes ?

SarveshLimaye commented 1 year ago

Hey @Dnouv , I have fixed all the errors and everything is working fine. Could you please take a look ?

screen-capture (2).webm

Dnouv commented 1 year ago

Nicely done, @SarveshLimaye !

However, could you please take a look, this does not seem to work on Gitpod, I always get this error:

image

Once this is fixed the PR is good to merge. I have tried building the new Dockerfile, and it works quite well.

Thank you!

SarveshLimaye commented 1 year ago

Hey @Dnouv, Thanks for the review. I once encountered this error when I tried building the docker file without starting the backend server.

I just tested building the dockerfile on gitpod and its working fine .

screen-capture (6).webm

Dnouv commented 1 year ago

Any idea why that occurs?

SarveshLimaye commented 1 year ago

@Dnouv AFAIK it can happen because of the following reasons -

  1. not setting the DOCKER_BUILDKIT = 0
  2. When we don't provide the GITPOD with permissions to make this port public.
  3. Pretty obvious one ig - When the backend server is not up and running.
Dnouv commented 1 year ago

Oh ok, I am trying once again with all the backend up and running.

Dnouv commented 1 year ago

@SarveshLimaye, you see here, I have set the NEXT_PUBLIC_STRAPI_URL to localhost, however, in the Client-side app it is still trying to connect to - http://172.28.0.1:1337/api/carousels image

Dnouv commented 1 year ago

Hey @SarveshLimaye Please fix this error, it occurs because you forgot to import the assets directory, to see the error visits /conferences page:

error - ./pages/conferences/index.js:5:0
app-app-1  | Module not found: Can't resolve '../../assets/event_logo.svg'
app-app-1  |   3 | import EventHome from "../../components/conferences/EventHome";
app-app-1  |   4 | import Image from "next/image";
app-app-1  | > 5 | import eventLogo from "../../assets/event_logo.svg";

Also, update the docs to incorporate this method of starting the development environment. If possible, try to make changes in the startNext.sh such that:

sh startNext.sh localhost --docker

Should start a docker development image of NextJS.

Please rename docker-compose.development.yml to docker-compose.dev.yml

And lastly, please don't make the NextJS fix on port 3000 so that if the user's local port 3000 is occupied, the NextJS can run on any other port

ports:
      - "3000:3000"
 should be
 ports:
      - "$NEXTJS_PORT:3000"

This is the variable that is set to the available port you only need to pass this on to the Docker Compose.

Once these changes are done, please ping me. Thank you!

SarveshLimaye commented 1 year ago

@Dnouv I am really not sure how to make changes in startNext.sh to start the docker development image of NextJs using command sh startNext.sh localhost --docker

Also, I just had a question related to updating docs. Should I update docs to the same steps as we are running the dockerfile now . Something like - cd app docker-compose -f docker-compose.dev.yml up --build

or wait until we find a way to build dockerfile using this command - sh startNext.sh localhost --docker and then update the docs Other than this I have done all the changes . Thank You !

SarveshLimaye commented 1 year ago

@Dnouv all the suggested changes done !