Conversation
There was a problem hiding this comment.
Code Review
This pull request upgrades the Node.js version from 20 to 24 across the repository, including configuration files and Dockerfiles, and updates test mocks to prefix unused parameters with underscores. Feedback indicates that Node.js 24 is not yet released, which will cause build failures; it is recommended to use a stable version like Node 22 instead. Additionally, a missing ARG definition for ${NODE_VERSION} was identified in the Dockerfile runtime stage, which would lead to an invalid image name.
| @@ -1 +1 @@ | |||
| v20.20.2 | |||
| v24.15.0 | |||
There was a problem hiding this comment.
| # docker run -d -p 3000:3000 flowise | ||
|
|
||
| FROM node:20-alpine | ||
| FROM node:24-alpine |
| @@ -1,5 +1,5 @@ | |||
| # Stage 1: Build stage | |||
| FROM node:20-alpine AS build | |||
| FROM node:24-alpine AS build | |||
|
|
||
| # Stage 2: Runtime stage | ||
| FROM node:20-alpine | ||
| FROM node:${NODE_VERSION}-alpine |
There was a problem hiding this comment.
The variable ${NODE_VERSION} is used here but is not defined as an ARG in this Dockerfile. This will cause the build to fail as the image name will be incomplete (e.g., node:-alpine). Additionally, this is inconsistent with the hardcoded version used in the build stage on line 2. It is recommended to use a consistent, released version like 22-alpine.
FROM node:22-alpine
| }, | ||
| "engines": { | ||
| "node": "^20", | ||
| "node": "^24", |
There was a problem hiding this comment.
Setting the required Node.js engine to ^24 will prevent installation on current environments since Node 24 has not been released yet. Per repository rules, the Node.js engine version in package.json should be limited to versions verified by the CI pipeline to avoid implying support for untested versions. If the intention was to upgrade to the latest LTS, this should likely be ^22.
| "node": "^24", | |
| "node": "^22", |
References
- The Node.js engine version in package.json should be limited to versions verified by the CI pipeline to avoid implying support for untested versions.
There was a problem hiding this comment.
It was just released yesterday, perhaps too new
| }, | ||
| "engines": { | ||
| "node": "^20" | ||
| "node": "^24" |
There was a problem hiding this comment.
The Node.js engine version should be updated to a released version (e.g., ^22) to avoid installation failures and maintain consistency with the root package.json. Per repository rules, the Node.js engine version in package.json should be limited to versions verified by the CI pipeline to avoid implying support for untested versions.
| "node": "^24" | |
| "node": "^22" |
References
- The Node.js engine version in package.json should be limited to versions verified by the CI pipeline to avoid implying support for untested versions.
No description provided.