Comfy-Org/comfyui-ci-container

Fork 0

mirror of https://github.com/Comfy-Org/comfyui-ci-container.git synced 2026-11-04 23:47:34 +00:00

[PR #26] [MERGED] feat: drive Node version from caller's .nvmrc via fnm + Corepack #26

New issue

Closed

opened 2026-10-26 22:23:02 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-26 22:23:02 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/Comfy-Org/comfyui-ci-container/pull/26
Author: @DrJKL
Created: 6/24/2026
Status: ✅ Merged
Merged: 6/25/2026
Merged by: @DrJKL

Base: main ← Head: node-version-via-fnm

📝 Commits (1)

2ef22df feat: drive Node version from caller's .nvmrc via fnm + Corepack

📊 Changes

2 files changed (+13 additions, -5 deletions)

View changed files

📝 Dockerfile (+11 -4)
📝 README.md (+2 -1)

📄 Description

Problem

The image inherits a fixed Node (24) from the Playwright base image (mcr.microsoft.com/playwright:v1.58.1-noble). The frontend repo declares engines.node: ">=25 <26" and .nvmrc: 25, so CI logs:

[WARN] Unsupported engine: wanted: {"node":">=25 <26"} (current: {"node":"v24.13.0","pnpm":"11.3.0"})

Approach

Rather than swap one fixed Node pin for another, make the container version-agnostic so the calling repo owns both versions:

Node → .nvmrc via fnm. Node 25 (NODE_VERSION build arg) is pre-installed as the offline fast-path default; any other .nvmrc is honored at runtime via fnm use --install-if-missing (downloaded on demand). /opt/fnm is writable by pwuser so a future bump self-heals without a rebuild.
pnpm → packageManager via Corepack, installed explicitly because Node 25 no longer bundles it. Replaces the drift-prone npm install -g pnpm.

Caller impact

Because Node 25 is the baked default and .nvmrc is 25, existing frontend workflows work unchanged and the warning disappears. The fnm use line is only needed once .nvmrc diverges from the baked default (see README).

Verification

⚠️ Not yet built locally (no Docker daemon available in the authoring environment). The build-and-push workflow on this PR is the verification — confirm node -v → 25 and pnpm -v resolves via Corepack in the built image before merging.

🤖 Generated with Claude Code

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/Comfy-Org/comfyui-ci-container/pull/26 **Author:** [@DrJKL](https://github.com/DrJKL) **Created:** 6/24/2026 **Status:** ✅ Merged **Merged:** 6/25/2026 **Merged by:** [@DrJKL](https://github.com/DrJKL) **Base:** `main` ← **Head:** `node-version-via-fnm` --- ### 📝 Commits (1) - [`2ef22df`](https://github.com/Comfy-Org/comfyui-ci-container/commit/2ef22df2831cf7d7eb4040541768363611188cf3) feat: drive Node version from caller's .nvmrc via fnm + Corepack ### 📊 Changes **2 files changed** (+13 additions, -5 deletions) <details> <summary>View changed files</summary> 📝 `Dockerfile` (+11 -4) 📝 `README.md` (+2 -1) </details> ### 📄 Description ## Problem The image inherits a fixed Node (24) from the Playwright base image (`mcr.microsoft.com/playwright:v1.58.1-noble`). The frontend repo declares `engines.node: ">=25 <26"` and `.nvmrc: 25`, so CI logs: ``` [WARN] Unsupported engine: wanted: {"node":">=25 <26"} (current: {"node":"v24.13.0","pnpm":"11.3.0"}) ``` ## Approach Rather than swap one fixed Node pin for another, make the container **version-agnostic** so the calling repo owns both versions: - **Node → `.nvmrc`** via [fnm](https://github.com/Schniz/fnm). Node 25 (`NODE_VERSION` build arg) is pre-installed as the offline fast-path default; any other `.nvmrc` is honored at runtime via `fnm use --install-if-missing` (downloaded on demand). `/opt/fnm` is writable by `pwuser` so a future bump self-heals without a rebuild. - **pnpm → `packageManager`** via [Corepack](https://github.com/nodejs/corepack), installed explicitly because Node 25 no longer bundles it. Replaces the drift-prone `npm install -g pnpm`. ## Caller impact Because Node 25 is the baked default and `.nvmrc` is `25`, existing frontend workflows work unchanged and the warning disappears. The `fnm use` line is only needed once `.nvmrc` diverges from the baked default (see README). ## Verification ⚠️ **Not yet built locally** (no Docker daemon available in the authoring environment). The `build-and-push` workflow on this PR is the verification — confirm `node -v` → 25 and `pnpm -v` resolves via Corepack in the built image before merging. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-26 22:23:02 +00:00

closed this issue
added the
pull-request
label

No labels

pull-request

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

Comfy-Org/comfyui-ci-container#26

No description provided.

Rows
Columns