Skip to content

fix: compatible with pool stats reading #578

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.

Sign up for GitHub

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

Merged
fengmk2 merged 6 commits into from
Jun 28, 2025
Merged

fix: compatible with pool stats reading #578

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1cc61d0
test: fix fail tests
fengmk2 Jun 28, 2025
e615bb7
f
fengmk2 Jun 28, 2025
0149b9f
f
fengmk2 Jun 28, 2025
a24c671
f
fengmk2 Jun 28, 2025
cfa7cdd
f
fengmk2 Jun 28, 2025
333b129
f
fengmk2 Jun 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/nodejs.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,6 @@ jobs:
uses: node-modules/github-actions/.github/workflows/node-test.yml@master
with:
os: 'ubuntu-latest, macos-latest, windows-latest'
version: '16, 18, 20, 22, 23'
version: '16, 18, 20, 22, 24'
secrets:
CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ content-type: application/json
}
```

This exmaple can use `options.data` with `application/json` content type:
This example can use `options.data` with `application/json` content type:

```js
await request('https://example.com', {
Expand Down
4 changes: 2 additions & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
"test": "npm run lint -- --fix && vitest run",
"test-keepalive": "cross-env TEST_KEEPALIVE_COUNT=50 vitest run --test-timeout 180000 keep-alive-header.test.ts",
"test-node16": "node examples/httpclient.cjs && node examples/search_github.cjs && node examples/timing.cjs",
"cov": "cross-env NODE_OPTIONS='--require ./test/patch-structuredClone.cjs' vitest run --coverage",
"cov": "cross-env NODE_OPTIONS='--require ./test/patch-for-node16.cjs' vitest run --coverage",
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Jun 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

❓ Verification inconclusive

Quote usage in NODE_OPTIONS breaks on Windows cmd / PowerShell

cross-env does not strip the single quotes on Windows, so NODE_OPTIONS will literally be set to
'--require ./test/patch-for-node16.cjs', causing Node to treat the leading ' as part of the flag and fail to load the patch.
Replace the quoted string with an un-quoted assignment (POSIX shells still parse it correctly).

-"cov": "cross-env NODE_OPTIONS='--require ./test/patch-for-node16.cjs' vitest run --coverage",
+"cov": "cross-env NODE_OPTIONS=--require ./test/patch-for-node16.cjs vitest run --coverage",

Update cov script to remove single quotes for Windows compatibility

Remove the single quotes around the --require flag so that cross-env sets NODE_OPTIONS correctly on Windows shells.

  • File: package.json, line 42
-  "cov": "cross-env NODE_OPTIONS='--require ./test/patch-for-node16.cjs' vitest run --coverage",
+  "cov": "cross-env NODE_OPTIONS=--require ./test/patch-for-node16.cjs vitest run --coverage",
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"cov": "cross-env NODE_OPTIONS='--require ./test/patch-for-node16.cjs' vitest run --coverage",
"cov": "cross-env NODE_OPTIONS=--require ./test/patch-for-node16.cjs vitest run --coverage",
🤖 Prompt for AI Agents 
In package.json at line 42, the cov script uses single quotes around the
--require flag which causes issues on Windows shells. Remove the single quotes
around --require ./test/patch-for-node16.cjs so that cross-env sets NODE_OPTIONS
correctly on Windows. This will ensure the script runs properly across different
environments.

"ci": "npm run lint && npm run cov && npm run prepublishOnly && npm pack && attw --pack",
"clean": "rm -rf dist",
"prepublishOnly": "npm run build"
Expand Down Expand Up @@ -78,7 +78,7 @@
"tshy": "^3.0.0",
"tshy-after": "^1.0.0",
"typescript": "^5.0.4",
"vitest": "^3.0.2"
"vitest": "^3.2.4"
},
"engines": {
"node": ">= 18.19.0"
Expand Down
13 changes: 13 additions & 0 deletions test/patch-for-node16.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// vitest require structuredClone
if (!('structuredClone' in globalThis)) {
const structuredClone = require('@ungap/structured-clone').default;

globalThis.structuredClone = structuredClone;
// console.debug('patched structuredClone for Node.js %s', process.version);
}

// vitest require crypto.getRandomValues
const crypto = require('node:crypto');
if (typeof crypto.getRandomValues !== 'function') {
crypto.getRandomValues = crypto.webcrypto.getRandomValues.bind(crypto.webcrypto);
}
7 changes: 0 additions & 7 deletions test/patch-structuredClone.cjs
View file
Open in desktop

This file was deleted.

Loading