Support Node.js 24 #2110
Support Node.js 24 #2110
Conversation
There was a problem hiding this comment.
Pull Request Overview
Updates the project to support Node.js version 24, addressing compatibility issues and modernizing the development environment. This includes package version updates, workflow adjustments, and specific compatibility fixes for Node.js 24 behavioral changes.
- Update Node.js version requirement to 24.0.0 across all packages and workflows
- Add custom readlink implementation to handle Windows directory symlink behavior changes in Node.js 24
- Refactor HTTP client header handling methods for improved type safety and clarity
Reviewed Changes
Copilot reviewed 14 out of 18 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/io/src/io-util.ts | Custom readlink implementation for Windows compatibility |
| packages/http-client/src/index.ts | Refactored header handling methods with improved type safety |
| packages/http-client/package.json | Updated Node.js engine requirement and @types/node version |
| packages/core/package.json | Updated Node.js engine requirement and @types/node version |
| packages/cache/package.json | Updated Node.js engine requirement and @types/node version |
| packages/cache/tests/saveCache.test.ts | Added missing mock and removed duplicate assertion |
| packages/artifact/tests/upload-artifact.test.ts | Fixed Stats constructor usage for Node.js 24 compatibility |
| package.json | Updated root Node.js engine requirement and @types/node version |
| .github/workflows/unit-tests.yml | Added Node.js 24.x to test matrix |
| .github/workflows/releases.yml | Updated workflow to use Node.js 24.x |
| .github/workflows/cache-windows-test.yml | Updated workflow to use Node.js 24.x |
| .github/workflows/cache-tests.yml | Updated workflow to use Node.js 24.x |
| .github/workflows/audit.yml | Updated workflow to use Node.js 24.x |
| .github/workflows/artifact-tests.yml | Updated workflow to use Node.js 24.x |
Files not reviewed (3)
- packages/cache/package-lock.json: Language not supported
- packages/core/package-lock.json: Language not supported
- packages/http-client/package-lock.json: Language not supported
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
| return `${result}\\` | ||
| } | ||
| } catch (err) { | ||
| // If we can't access the target, just return the original result |
There was a problem hiding this comment.
Using lstat on the symlink target (result) will follow the symlink and stat the target, but result is already the resolved path. This should use lstat on the original fsPath to check if the symlink itself points to a directory.
| // If we can't access the target, just return the original result | |
| const stats = await fs.promises.lstat(fsPath) | |
| if (stats.isDirectory() && !result.endsWith('\\')) { | |
| return `${result}\\` | |
| } | |
| } catch (err) { | |
| // If we can't access the symlink, just return the original result |
Copilot uses AI. Check for mistakes.
| compression | ||
| ) | ||
| expect(saveCacheMock).toHaveBeenCalledTimes(1) | ||
| expect(getCompressionMock).toHaveBeenCalledTimes(1) |
There was a problem hiding this comment.
The removed duplicate assertion expect(getCompressionMock).toHaveBeenCalledTimes(1) was correct to remove as it was redundant with line 280.
Copilot uses AI. Check for mistakes.
packages/cache/package.json
Outdated
| "engines": { | ||
| "node": ">=24.0.0" | ||
| }, |
There was a problem hiding this comment.
This will cause any package that depends on @actions/cache to also require Node 24, removing backward compatibility.
There was a problem hiding this comment.
The plan is to have a major version bump here as well as in actions cache anyway, so any version of cache that already references the current or older versions won't be referencing this change, and any newer versions would be on the action that targets node24. What scenario do you envision needing a runner that doesn't have node 24? Self-hosted runners should all be updated and all hosted runners have node 24 already.
There was a problem hiding this comment.
Oh I can think of a scenario, such that if you were to consume the package not from the cache action. Okay I'll remove it. Thanks for the comment!
- Keep version 5.0.0 for cache package (Node 24 branch) - Maintain uri-js-replace override in all packages - Update Azure SDK import to use @azure/core-http-compat - Resolved package-lock.json conflicts
…t party actions that are available (checkout, setup-node, github-script) =
|
Since we're definitely breaking Node 18 and we don't support that version, can we just remove those tests so the CI stays green? |
|
Sure will remove those tests |
3 participants
Update the project to support Node.js version 24, including adjustments to workflows and package dependencies.