Add Node 18 - 26 support (possible to add 27 in later) - #134
Open
GroophyLifefor wants to merge 1 commit into
Open
Add Node 18 - 26 support (possible to add 27 in later) #134GroophyLifefor wants to merge 1 commit into
GroophyLifefor wants to merge 1 commit into
Conversation
- Added Node.js engine requirement (>=18) to package.json and package-lock.json. - Updated GitHub Actions workflows to accept Node.js version as an input for baseline, test, and compare jobs. - Modified artifact naming in workflows to include Node.js version for better traceability. - Refactored path handling in CLI scripts to use `fileURLToPath` for improved compatibility.
O4FDev
approved these changes
Aug 27, 2026
| "author": "", | ||
| "license": "MIT", | ||
| "engines": { | ||
| "node": ">=18" |
Member
There was a problem hiding this comment.
Given that Node 18 is EOL do we want to worry about it here and do performance reporting for 18?
Might not be good to release stats using it if it's not being patched anymore
Member
There was a problem hiding this comment.
My two cents: what matters here is supporting Express 5’s minimum supported version (Node 18) and the latest LTS (Node 26). We don’t really care about the versions in between.
| name: Run Baseline Load Tests (node ${{ matrix.node }}) | ||
| strategy: | ||
| matrix: | ||
| node: [18, 20, 22, 24, 26] |
| name: Run Test Load Tests (node ${{ matrix.node }}) | ||
| strategy: | ||
| matrix: | ||
| node: [18, 20, 22, 24, 26] |
| needs: [baseline, test] | ||
| strategy: | ||
| matrix: | ||
| node: [18, 20, 22, 24, 26] |
| artifactName: | ||
| description: "Artifact name for results" | ||
| value: perf-load-test-${{github.run_id}} | ||
| value: perf-load-test-${{github.run_id}}-${{inputs.node}} |
Member
There was a problem hiding this comment.
These two will overlap so 2nd is rejected
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: perf-load-test-${{github.run_id}} | ||
| name: perf-load-test-${{github.run_id}}-${{inputs.node}} |
| matrix: | ||
| node: [18, 20, 22, 24, 26] | ||
| uses: ./.github/workflows/load.yml | ||
| with: |
Member
There was a problem hiding this comment.
I would propose that we upload with separate artifact names here and same below
Then in load.yml:
inputs:
artifactName:
required: true
type: string
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
import.meta.dirname(needs Node 20.11+) in CLI/runner-local-docker → replaced with stdlibdirname(fileURLToPath(import.meta.url)).import.meta.main/import.meta.filename(needs Node 22.12+/20.11+) in allperf/load/*/index.mjsentrypoints — on Node 18 both areundefined, so the server never started and the load runner hung silently. Replaced with the portableimport.meta.url === pathToFileURL(process.argv[1]).hrefcheck.engines: ">=18"topackage.json.pr.yml; fixed an artifact-name collision inload.yml/compare.ymlthat would've broken parallel matrix runs, and a missingnodeinput incompare.yml.Verified
loadandcompareend-to-end on Node 18.20.8 and 20.20.2.ref: #47