Skip to content

fix(docker): install node dependencies in init.sh#4854

Open
palash018 wants to merge 5 commits into
frappe:developfrom
palash018:fix/docker-init-yarn-install
Open

fix(docker): install node dependencies in init.sh#4854
palash018 wants to merge 5 commits into
frappe:developfrom
palash018:fix/docker-init-yarn-install

Conversation

@palash018

@palash018 palash018 commented Jul 5, 2026

Copy link
Copy Markdown

Closes #4845

Summary

bench init clones the Frappe app but does not install its Node dependencies. When bench start later launches the socketio service, it fails because socket.io is missing from apps/frappe/node_modules.

This PR ensures apps/frappe/node_modules is installed before bench start is reached, and that the script aborts if the installation fails.

Also includes small fixes in docker/init.sh:

  • Corrects the shebang from #!bin/bash to #!/bin/bash.
  • Adds exit 0 after the early "bench already exists" branch so the script does not fall through and attempt bench init again.
  • Moves the NVM Node PATH export to the top so yarn is available in both the fresh-bench and existing-bench paths.

Changes

  • docker/init.sh:
    • Install Node dependencies after bench init on fresh benches.
    • Install Node dependencies on existing benches before bench start.
    • Abort the script if yarn install fails so bench start never runs with missing dependencies.
    • Factor the install step into a helper so both call sites are consistent.
    • Fix shebang and prevent fall-through on existing bench.
    • Export NVM Node PATH before any yarn invocation.

Test plan

  • Run docker compose down and docker compose up in the docker/ directory.
  • Verify the frappe container completes initialization without Cannot find module 'socket.io' errors.
  • Verify bench start launches web, socketio, schedule, and worker processes successfully.
  • Verify the site is accessible at http://localhost:8000.

Generated with Devin

`bench init` clones the Frappe app but does not install its Node
dependencies. When `bench start` later launches the socketio service,
it fails because `socket.io` is missing from `apps/frappe/node_modules`.

Also fixes the shebang (`#!bin/bash` -> `#!/bin/bash`) and adds an
early `exit 0` so the script does not fall through to `bench init` when
the bench already exists.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@greptile-apps

greptile-apps Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Confidence Score: 5/5

This looks safe to merge.

  • No blocking issues found in the changed code.

Reviews (5): Last reviewed commit: "fix(docker): export NVM PATH before any ..." | Re-trigger Greptile

Comment thread docker/init.sh Outdated
Ensure the subshell running `yarn install` exits the script on failure
so `bench start` is never reached with missing Node dependencies.

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Comment thread docker/init.sh
If the bench already exists but its `node_modules` were removed or
never installed, the early-exit branch now installs them before starting
the bench.

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Comment thread docker/init.sh Outdated
Always run yarn install (both for freshly initialized and pre-existing
benches) instead of skipping based on directory/package existence
checks, which could miss stale or partially installed dependencies.
Factor the install step into a helper to keep both call sites in sync.

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Comment thread docker/init.sh
@palash018
palash018 marked this pull request as draft July 5, 2026 09:37
Move the Node/NVM PATH export to the top of the script so `yarn` is
available in the existing-bench branch before `install_node_deps` runs,
instead of relying on the Docker image's default PATH.

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@palash018
palash018 marked this pull request as ready for review July 5, 2026 09:40
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.

Cannot Start Docker Compose Up

1 participant