ci: Run PR pipeline on Node 26 - #34032
Conversation
PR review overviewBased on ownership of the 23 changed files in this PR:
|
Bundle ReportChanges will increase total bundle size by 20.11kB (0.04%) ⬆️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: editor-ui-esmAssets Changed:
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
# Conflicts: # .github/actions/setup-nodejs/action.yml # .github/workflows/test-linting-reusable.yml # .github/workflows/test-unit-reusable.yml # .github/workflows/test-workflow-scripts-reusable.yml # package.json # pnpm-lock.yaml # pnpm-workspace.yaml
Instance AI Discovery Eval ✅Branch: Eval output |
# Conflicts: # pnpm-workspace.yaml
|
|
||
| set +o pipefail | ||
| timeout --kill-after=30s 300s $INSTALL_COMMAND \ | ||
| timeout --kill-after=30s 600s $INSTALL_COMMAND \ |
There was a problem hiding this comment.
No sure if I got unlucky but on cold cache downloading and scanning the ~3,400 packages alone ate >300s
There was a problem hiding this comment.
Possibly. Can't remember this timing out in... ever?
| # While we're on alpine 3.22 awaiting NODE-4184 (graphicsmagick → sharp | ||
| # migration that unblocks 3.23), Node 26 base builds can't succeed. | ||
| # Restore '26' to this matrix once 3.23 is back. | ||
| node_version: ['22', '24.18.1'] |
There was a problem hiding this comment.
There used to be a bug here, matrix passed node_version as a build arg, but thedDockerfile never used it, its FROM was hardcoded to the Node 24 DHI base
| - node_version: '22' | ||
| dhi_ref: dhi.io/node:22.23.2-alpine3.24-dev@sha256:a6ac21cfbc4bb746c1cfc6cf1501ec13c961ae71558ff2ac7e021caaaf91a224 |
There was a problem hiding this comment.
As 22 is in maintenance mode, I'd possibly keep it. Maybe drop it with v3?
There was a problem hiding this comment.
I'd favor dropping it:
- We've had a long-time bug skipping Node 22 already
isolated-vmv7 does not officially support Node 22- n8n has no Node version policy
- I think @shortstacked wants to consolidate Node versions
There was a problem hiding this comment.
@Matsuuu I increased the min engine version to 24 at least and removed 22 from the CI matrix because with upgrading isolated-vm it would break once we enable isolated expressions by default.
I did leave this image untouched, because I have no idea who's using it. I'm not sure if we are using it at all.
There was a problem hiding this comment.
Okay a lot of fair points. Let's just rid of it then 👍🏼
|
Tip All tests passed and all changes approved!🟢 UI Tests: n8n: 491 tests unchanged |
isolated-vm v7 has no Node 22 build, so expressions, Merge SQL mode, and agents would break at runtime on Node 22. Raise engines to >=24 and test 24 + 26 on master. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Got released with |
Co-authored-by: Danny Martini <danny@n8n.io> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Summary
This PR
n8nio/base:26.5.1tag alongside 22 and 24.e2e still uses the Node 24 image.
Re: Kafka - Confluent ships no Node 26 prebuilds, so CI installs skip building it. The base now carries
librdkafkaso the follow-up can link the binding against it at image build time.Upcoming PRs
node-pcbecomesFROM n8nio/base:26.5.1(once published).librdkafka. This will switch e2e and released images to Node 26.-pcimage built on the pointer-compressed Node.1 and 2 are independent and both unblock once this PR #34032 merges and the bases publish. 3 needs 1 and 2.
Related Linear tickets, Github issues, and Community forum posts
https://app.notion.com/p/n8n/Halving-memory-usage-via-pointer-compression-3ac5b6e0c94f8005af87e93a9f837b53
Review / Merge checklist
Backport to Beta,Backport to Stable, orBackport to v1(if the PR is an urgent fix that needs to be backported)