Skip to content

install node.js in docker container #87

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Nov 14, 2025
Merged

install node.js in docker container #87

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 6 additions & 1 deletion Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,13 @@ ENV RATE_LIMIT_WINDOW_MINUTES=1

Copy link

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

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

[nitpick] The unzip package is added but there's no clear indication in the PR description or code why it's needed. If it's a dependency for fnm or Node.js installation, please document this in a comment. If it's unrelated to the Node.js installation, consider adding it in a separate commit or PR for clarity.

Suggested change
# 'unzip' is required for fnm/Node.js installation scripts

Copilot uses AI. Check for mistakes.
RUN apt-get update && \
apt-get upgrade -y && \
apt-get install -y curl ffmpeg unzip && \
curl -o- https://fnm.vercel.app/install | bash
SHELL ["/bin/bash", "--login", "-c"]
RUN fnm install --lts && \
Comment on lines +24 to +26
Copy link

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

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

The SHELL change to use --login shell affects all subsequent RUN commands in the Dockerfile, including the poetry installation at line 34. This could cause unexpected behavior or performance issues as login shells source additional configuration files.

Consider one of these approaches:

  1. Revert SHELL after the Node.js installation is complete
  2. Set up fnm's PATH manually without requiring a login shell
  3. Use a single RUN command with explicit shell invocation for just the fnm commands

Example for approach 2:

RUN apt-get update && \
    apt-get upgrade -y && \
    apt-get install -y curl ffmpeg unzip && \
    curl -o- https://fnm.vercel.app/install | bash && \
    export PATH="/root/.local/share/fnm:$PATH" && \
    eval "$(fnm env)" && \
    fnm install --lts && \
    node -v && npm -v && \
    python -m pip install --upgrade pip && \
    apt-get clean && rm -rf /var/lib/apt/lists/* && \
    mkdir /app
Suggested change
curl -o- https://fnm.vercel.app/install | bash
SHELL ["/bin/bash", "--login", "-c"]
RUN fnm install --lts && \
curl -o- https://fnm.vercel.app/install | bash && \
export PATH="/root/.local/share/fnm:$PATH" && \
eval "$(fnm env)" && \
fnm install --lts && \

Copilot uses AI. Check for mistakes.
node -v && \
npm -v && \
Comment on lines +26 to +28
Copy link

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

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

The fnm installation adds Node.js to the build environment but doesn't ensure it's available in the runtime environment. The fnm tool modifies the shell's PATH through .bashrc, which works with the --login shell, but this doesn't guarantee Node.js will be available when the container runs the CMD.

To ensure Node.js is available at runtime, add:

ENV PATH="/root/.local/share/fnm:$PATH"

after the fnm installation, or configure fnm to install Node.js to a permanent location that's added to the PATH environment variable.

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

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

[nitpick] Extra trailing whitespace after npm -v. This should be removed for consistency.

Suggested change
npm -v && \
npm -v && \

Copilot uses AI. Check for mistakes.
python -m pip install --upgrade pip && \
apt-get install -y curl ffmpeg && \
apt-get clean && rm -rf /var/lib/apt/lists/* && \
mkdir /app
Comment on lines +26 to 31
Copy link

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

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

Splitting the RUN command into two separate RUN statements creates an additional Docker layer and prevents proper cleanup. The apt-get clean && rm -rf /var/lib/apt/lists/* on line 30 runs in a separate layer from the apt-get install on line 23, so it doesn't actually reduce the image size.

Docker layers are immutable, so removing files in a later layer doesn't remove them from earlier layers. To effectively reduce image size, the installation and cleanup should be in the same RUN command.

Consider combining these RUN commands back together as suggested in the previous comment.

Copilot uses AI. Check for mistakes.

Expand Down