Skip to content

Added Docker environment setup #132

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: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions Dockerfile
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
FROM node:17
Copy link
Member

Choose a reason for hiding this comment

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

Please use

Suggested change
FROM node:17
FROM node:16

instead. See https://nodejs.org/en/about/releases/ - odd-numbered (9, 11, etc.) releases become unsupported after 6 months and their use in production is discouraged.


WORKDIR /webide
ADD . .
RUN npm install
Comment on lines +4 to +5
Copy link
Member

Choose a reason for hiding this comment

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

To use layer caching more efficiently, the best practice is to first COPY "package.json" and "package-lock.json", then RUN npm install, and only then copy the rest of the files with COPY . .. See https://nodejs.org/en/docs/guides/nodejs-docker-webapp/ to see the pattern (don't forget to add .dockerignore) and https://docs.docker.com/get-started/09_image_best/#layer-caching for an explanation why this approach is better.

Also please use COPY instead of ADD, the reason is here: https://docs.docker.com/develop/develop-images/dockerfile_best-practices/#add-or-copy


CMD node serve.js --compile
6 changes: 6 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,12 @@ Online editor / visualizer for Kaitai Struct .ksy files
- `node serve.js --compile`
- Go to [http://127.0.0.1:8000/](http://127.0.0.1:8000/)

## run in docker container
- Install docker and docker-compose
- `git clone --recursive https://github.com/kaitai-io/kaitai_struct_webide`
- `docker-compose build && docker-compose up -d`
- Go to [http://localhost:8000/](http://localhost:8000/)
Copy link
Member

Choose a reason for hiding this comment

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

Please use 127.0.0.1:8000 instead of localhost:8000.

The "automatic reload when a local change is detected" feature works only for the 127.0.0.1 hostname:

if(location.hostname === "127.0.0.1")
checkModifications();

Also using localhost instead of 127.0.0.1 caused some 1 sec delay (but I have no idea what the cause was, and whether it still persists), see

print "please use 127.0.0.1:%d on Windows (using localhost makes 1sec delay)" % PORT

(this is a different file for different purpose, unused by the current Web IDE version, but whatever)


Actually for the "automatic reload" to work and also for easier development, it would be nice if the Docker image didn't have to be manually rebuilt every time a change is made to the source files. This is certainly achievable with Docker using bind mounts, see https://docs.docker.com/get-started/06_bind_mounts/.

node serve.js --compile is already capable of watching file modifications and recompiling the app, so just creating the bind mount should be enough I guess.


## screenshots

![Example screenshot of a .zip file](docs/zip_example.png)
Expand Down
6 changes: 6 additions & 0 deletions docker-compose.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
version: "3.9"
services:
webide:
build: ./
ports:
- "8000:8000"