Skip to content
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

Docker build does not rebuild frontend #10

Closed
pataquets opened this issue Oct 24, 2020 · 3 comments
Closed

Docker build does not rebuild frontend #10

pataquets opened this issue Oct 24, 2020 · 3 comments
Labels
enhancement New feature or request

Comments

@pataquets
Copy link
Contributor

pataquets commented Oct 24, 2020

As I've started to tinker with the code to do an easy frontend test, I could not even make the code run.
In my case, I rely exclusively on Docker to develop (hack/docker build/test/repeat). I don't have NodeJS toolchain installed on my OS, so I can work on any version (and even on any language) without breaking my PC.

I tried some easy frontend changes and it kept me puzzled for a while until I've finally realized that (as stated in the readme for normal development), the Docker build also does not rebuild frontend build dir.
I've done my best and tried to fix it by just following the frontend readme documentation and some googling, but it's way off my abilities (I'm neither a NodeJS dev nor frontend guy).

I think that Docker build should also rebuild frontend files always.
So, after banging my head for a few hours, I'm filing this issue.

Also, I guess that the build dir should not be commited in the repo.
I think it's preferable the run to fail loud out of the box instead of silently working with an outdated frontend. Also applies for any files that should be created by the build/CI process.

@KiraLT
Copy link
Owner

KiraLT commented Oct 26, 2020

Thank you for the feedback. I will research this topic a bit more. The biggest challenge is how to easily deploy to Heroko/Google App Engine. At the moment you can just push source and it will work (because the build is included in the source).

@KiraLT KiraLT added the enhancement New feature or request label Oct 26, 2020
pataquets referenced this issue in pataquets/torrent-stream-server Oct 28, 2020
pataquets referenced this issue in pataquets/torrent-stream-server Oct 28, 2020
@KiraLT KiraLT linked a pull request Oct 30, 2020 that will close this issue
@KiraLT
Copy link
Owner

KiraLT commented Nov 2, 2020

I added build support for Dockerfile. It builds both frontend & backend. I didn't use dockerignore, instead, I copied only necessary files. Also, GitHub automatically publishes the latest commit with the latest tag.

For now, I didn't remove build from the source - there is still some thinking to do regarding it.

FROM node:12

WORKDIR /usr/app
COPY package*.json ./
RUN npm ci
COPY tsconfig.json ./
COPY src/ src/
RUN npm run build

COPY frontend/package*.json frontend/
RUN npm ci --prefix frontend/
COPY frontend/tsconfig.json frontend/
COPY frontend/src/ frontend/src/
COPY frontend/public/ frontend/public/
RUN npm run --prefix ./frontend build && rm -rf ./frontend/node_modules

EXPOSE 3000
CMD node lib/index.js

@KiraLT KiraLT closed this as completed Nov 2, 2020
@pataquets
Copy link
Contributor Author

pataquets commented Nov 2, 2020

Great to see it, thanks. Some feedback:

  • Even when reasonably keeping the frontend build files in git, I think that they should not be sent to Docker at build time. Sounds kind of dangerous if the frontend build process fails, leaving the stale files present in the repo sent by Docker. If the build fails, let the frontend dir in the built container be obviously empty. So, I would .dockerignore the frontend build dir.
  • Also, if you .dockerignore the build dir, you can safely add their parent dirs in one fell swoop safely (COPY . .). Thus, static subdirs are not even sent to Docker daemon in first place. Safer, IMO.
  • As I've previously stated, I think frontend static dir should be removed, but I'm sure it has far more implications that I can't grasp. Anyway, if done, it should be .gitignore'd.
  • I'm not sure if there is not any possible build optimization for repeated builds left. Like as done when adding a few package* files at first and running some partial build step and running the COPY . . statement afterwards. I see this pattern twice, and I wonder if both can be run first and remaining code add and build be latest.
  • Linked to the previous item, I've tested it by modifiying some backend .js & .ts files, and that invalidated frontend build cache (I'm not sure if should be that way).

Other that the above feedback, the image works wonderfully. Great job!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants