-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Replace uuid with crypto.randomUUID() #1378
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Replaces the uuid
npm package dependency with Node.js's built-in crypto.randomUUID()
method to generate unique identifiers for temporary download folders.
- Removes the
uuid
package dependency from package.json - Updates the temporary folder naming logic to use the native crypto API instead of the external uuid library
Reviewed Changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
src/distributions/base-distribution.ts | Replaces uuid import and uuidv4() call with crypto.randomUUID() |
package.json | Removes uuid dependency from the dependencies list |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
||
// Create temporary folder to download to | ||
const tempDownloadFolder = `temp_${uuidv4()}`; | ||
const tempDownloadFolder = `temp_${crypto.randomUUID()}`; |
Copilot
AI
Sep 24, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The crypto.randomUUID()
method requires Node.js 14.17.0+ or 16.0.0+. Consider adding a runtime check or documenting the minimum Node.js version requirement to ensure compatibility.
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's specified in
Lines 7 to 9 in 89d709d
"engines": { | |
"node": ">=24.0.0" | |
}, |
Hi @trivikr, thank you for submitting the PR. Could you please resolve the check failures and run licensed status and npm run build commands to fix the failing checks related to licensed and dist. |
Done $ licensed status
Checking cached dependency records for setup-node
......................................................................................F.....
Errors:
* setup-node.npm.uuid
version: 8.3.2, filename: /Users/trivikram/workspace/setup-node/.licenses/npm/uuid.dep.yml, license: , allowed: false
- cached dependency record not found
92 dependencies checked, 1 errors found.
Licensed found errors during source enumeration. Please see https://github.com/github/licensed/tree/master/docs/commands/status.md#status-errors-and-resolutions for possible resolutions.
$ licensed cache
Caching dependency records for setup-node
npm
Using @actions/cache (4.0.3)
Using @actions/core (1.11.1)
...
Using xml2js (0.5.0)
Using xmlbuilder (11.0.1)
* 92 npm dependencies Commit |
The files in dist folders were updated in |
The issue in yarn 1 cache test seems to be with availability
https://github.com/actions/setup-node/actions/runs/18350090223/job/52267623041 I believe CI should be successful, once it's approved to run by one of the maintainers. |
Description:
Related issue:
Fixes: #1377
Check list: