Skip to content

feat: add @nkzw/oxlint-config for stricter linting #696

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

Open
fengmk2 wants to merge 1 commit into
base: master
Choose a base branch
from
Open

feat: add @nkzw/oxlint-config for stricter linting #696

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 5 additions & 5 deletions examples/h2-other-side-closed-exit-0-fetch.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,13 @@ async function main() {
try {
const r = await fetch('https://edgeupdates.microsoft.com/api/products');
console.log(r.status, r.headers, (await r.text()).length);
} catch (err) {
} catch (error) {
// console.error(err);
// throw err;
if (err.code === 'UND_ERR_SOCKET') {
if (error.code === 'UND_ERR_SOCKET') {
continue;
} else {
throw err;
throw error;
}
}
}
Expand All @@ -27,8 +27,8 @@ main()
.then(() => {
console.log('main end');
})
.catch((err) => {
console.error('main error throw: %s', err);
.catch((error) => {
console.error('main error throw: %s', error);
// console.error(err);
process.exit(1);
});
Expand Down
10 changes: 5 additions & 5 deletions examples/h2-other-side-closed-exit-0.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,13 @@ async function main() {
try {
const r = await request('https://edgeupdates.microsoft.com/api/products');
console.log(r.statusCode, r.headers, (await r.body.blob()).size);
} catch (err) {
} catch (error) {
// console.error(err);
// throw err;
if (err.code === 'UND_ERR_SOCKET') {
if (error.code === 'UND_ERR_SOCKET') {
continue;
} else {
throw err;
throw error;
}
}
}
Expand All @@ -27,8 +27,8 @@ main()
.then(() => {
console.log('main end');
})
.catch((err) => {
console.error('main error throw: %s', err);
.catch((error) => {
console.error('main error throw: %s', error);
// console.error(err);
process.exit(1);
});
Expand Down
6 changes: 3 additions & 3 deletions examples/httpclient.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ tryHttpclient(HttpClient, 'urllib');
function tryHttpclient(HttpClient, name) {
const options = {
method: 'GET',
timeout: 10000,
timeout: 10_000,
timing: true,
};
const urllib = new HttpClient();
Expand All @@ -22,7 +22,7 @@ function tryHttpclient(HttpClient, name) {
.then(function () {
return urllib.request('https://nodejs.org/en/', options);
})
.catch(function (err) {
console.error('catch', err);
.catch(function (error) {
console.error('catch', error);
});
}
12 changes: 6 additions & 6 deletions examples/longruning.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ const httpClient = new HttpClient({
});

async function main() {
for (let i = 0; i < 1000000; i++) {
for (let i = 0; i < 1_000_000; i++) {
// await httpClient.request('https://registry.npmmirror.com/');
// console.log(r.status, r.headers, r.res.timing);
try {
Expand All @@ -15,8 +15,8 @@ async function main() {
// console.log(r.status, r.headers, r.data.length, r.res.timing);
console.log(i, r.status, process.memoryUsage());
}
} catch (err) {
console.error('%s error: %s', i, err.message);
} catch (error) {
console.error('%s error: %s', i, error.message);
}
}
}
Expand All @@ -25,9 +25,9 @@ main()
.then(() => {
console.log('main end');
})
.catch((err) => {
console.error('main error throw: %s', err);
console.error(err);
.catch((error) => {
console.error('main error throw: %s', error);
console.error(error);
process.exit(1);
});

Expand Down
4 changes: 2 additions & 2 deletions examples/search_github.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
// curl https://api.github.com/legacy/user/search/location:china

var urllib = require('../');
const urllib = require('../');

urllib
.request('https://api.github.com/legacy/user/search/location:china', {
dataType: 'json',
timing: true,
timeout: 10000,
timeout: 10_000,
})
.then((response) => {
console.log(response);
Expand Down
72 changes: 72 additions & 0 deletions oxlint.config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
import nkzw from '@nkzw/oxlint-config';
import { defineConfig } from 'oxlint';

export default defineConfig({
extends: [nkzw],
// urllib is a Node.js HTTP client library — no React
plugins: ['typescript', 'import', 'unicorn'],
ignorePatterns: ['dist/**', 'test/fixtures/**', 'test/esm/**', 'test/cjs/**', 'test/mts/**'],
rules: {
// Allow console usage in a Node.js library
'no-console': 'off',
// Allow any in internal utility code
'@typescript-eslint/no-explicit-any': 'off',
// Allow require() for CJS compat
'@typescript-eslint/no-require-imports': 'off',
Comment on lines +13 to +15

// --- Disable React rules (not a React project) ---
'react/display-name': 'off',
'react/jsx-key': 'off',
'react/jsx-no-comment-textnodes': 'off',
'react/jsx-no-duplicate-props': 'off',
'react/jsx-no-target-blank': 'off',
'react/jsx-no-undef': 'off',
'react/no-children-prop': 'off',
'react/no-danger-with-children': 'off',
'react/no-direct-mutation-state': 'off',
'react/no-find-dom-node': 'off',
'react/no-is-mounted': 'off',
'react/no-render-return-value': 'off',
'react/no-string-refs': 'off',
'react/no-unescaped-entities': 'off',
'react/no-unknown-property': 'off',
'react/require-render-return': 'off',
'react-hooks/rules-of-hooks': 'off',
'react-hooks/exhaustive-deps': 'off',
'react-hooks-js/component-hook-factories': 'off',
'react-hooks-js/config': 'off',
'react-hooks-js/error-boundaries': 'off',
'react-hooks-js/gating': 'off',
'react-hooks-js/globals': 'off',
'react-hooks-js/immutability': 'off',
'react-hooks-js/incompatible-library': 'off',
'react-hooks-js/preserve-manual-memoization': 'off',
'react-hooks-js/purity': 'off',
'react-hooks-js/refs': 'off',
'react-hooks-js/set-state-in-effect': 'off',
'react-hooks-js/set-state-in-render': 'off',
'react-hooks-js/static-components': 'off',
'react-hooks-js/unsupported-syntax': 'off',
'react-hooks-js/use-memo': 'off',
'@nkzw/ensure-relay-types': 'off',
'@nkzw/require-use-effect-arguments': 'off',

// --- Too invasive for an existing codebase ---
// Object/interface sorting would touch every file
'perfectionist/sort-objects': 'off',
'perfectionist/sort-object-types': 'off',
'perfectionist/sort-interfaces': 'off',
'perfectionist/sort-enums': 'off',
'perfectionist/sort-heritage-clauses': 'off',
'perfectionist/sort-jsx-props': 'off',

// Allow instanceof — common pattern in Node.js error handling
'@nkzw/no-instanceof': 'off',

// Library code legitimately uses no-undef globals (Buffer, etc.)
'no-undef': 'off',
Copy link

@gemini-code-assist gemini-code-assist bot Mar 16, 2026

Choose a reason for hiding this comment

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

medium

Disabling the no-undef rule for the entire project can be risky as it might hide legitimate bugs caused by typos or forgotten imports. A safer approach would be to declare the expected globals where they are used.

While oxlint.config.ts might not support a globals key, oxlint (and ESLint) respects inline global declarations. For files that use Node.js globals like Buffer or process, you could add a comment at the top of the file:

/* globals Buffer, process */

This would allow you to re-enable the no-undef rule (or set it to warn), providing better protection against undefined variables. Please consider this alternative approach for enhanced safety.


// Allow top-level await flexibility
'unicorn/prefer-top-level-await': 'off',
},
});
11 changes: 9 additions & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,9 @@
"clean": "rm -rf dist && tsc -b --clean",
"prepublishOnly": "vp run build",
"prepare": "node -e \"const v = parseInt(process.versions.node, 10); if (v >= 20) require('child_process').execSync('vp config', {stdio: 'inherit'});\"",
"check": "vp check",
"check:fix": "vp check --fix"
"lint:oxlint": "oxlint",
"check": "vp check && npm run lint:oxlint",
"check:fix": "vp check --fix && npm run lint:oxlint --fix"
},
"dependencies": {
"form-data": "^4.0.5",
Expand All @@ -82,6 +83,8 @@
"devDependencies": {
"@arethetypeswrong/cli": "^0.18.2",
"@eggjs/tsconfig": "^2.0.0",
"@nkzw/eslint-plugin": "^2.0.0",
"@nkzw/oxlint-config": "^1.0.1",
"@types/busboy": "^1.5.4",
"@types/mime-types": "^2.1.4",
"@types/node": "^22.19.15",
Expand All @@ -94,7 +97,11 @@
"@vitest/coverage-v8": "^4.1.0",
"busboy": "^1.6.0",
"cross-env": "^10.1.0",
"eslint-plugin-no-only-tests": "^3.3.0",
"eslint-plugin-perfectionist": "^5.6.0",
"eslint-plugin-unused-imports": "^4.4.1",
"iconv-lite": "^0.6.3",
"oxlint": "^1.55.0",
Comment on lines 70 to +104
"proxy": "^1.0.2",
"selfsigned": "^3.0.1",
"string.prototype.towellformed": "^1.0.2",
Expand Down
2 changes: 1 addition & 1 deletion scripts/replace_urllib_version.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ async function main() {
const pkg = JSON.parse(await fs.readFile(path.join(root, 'package.json')));
const files = [path.join(root, 'dist/commonjs/HttpClient.js'), path.join(root, 'dist/esm/HttpClient.js')];
for (const file of files) {
const content = await fs.readFile(file, 'utf-8');
const content = await fs.readFile(file, 'utf8');
// replace "const VERSION = 'VERSION';" to "const VERSION = '4.0.0';"
// "exports.VERSION = 'VERSION';" => "exports.VERSION = '4.0.0';"
const newContent = content.replace(/ = 'VERSION';/, (match) => {
Expand Down
8 changes: 4 additions & 4 deletions src/FormData.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ import path from 'node:path';

import _FormData from 'form-data';

// eslint-disable-next-line
const NON_ASCII_RE = /[^\x00-\x7F]/i;
// eslint-disable-next-line no-control-regex
const NON_ASCII_RE = /[^\u0000-\u007F]/i;
Comment on lines +5 to +6
Copy link

@coderabbitai coderabbitai bot Mar 16, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Check lint pipeline tools =="
rg -n '"check"|biome|oxlint|eslint' package.json

echo "== Check current regex and suppression in FormData =="
rg -n 'NON_ASCII_RE|no-control-regex|\\u0000|\\p\{ASCII\}' src/FormData.ts

Repository: node-modules/urllib

Length of output: 689

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Search for oxlint/biome configuration files =="
fd -t f '\.(json|js|yaml|yml)$' | grep -i -E '(oxlint|biome)' | head -20

echo "== Check for .oxlintrc or biome.json =="
find . -maxdepth 2 -type f \( -name '.oxlintrc*' -o -name 'biome.json' -o -name 'oxlint.config.*' \) 2>/dev/null

echo "== Check eslintrc configuration =="
find . -maxdepth 2 -type f -name '.eslintrc*' 2>/dev/null | head -5

echo "== View eslint-plugin config to understand rule coverage =="
rg -A 5 '"no-control-regex"' . 2>/dev/null | head -20

Repository: node-modules/urllib

Length of output: 290

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Check for biome.json or oxlint config in repo root =="
ls -la | grep -E '(oxlint|biome|eslint)' || echo "No direct matches in root"

echo "== Search package.json for '@nkzw' config hints =="
rg -A 10 '"@nkzw/eslint-plugin"' package.json

echo "== Verify if vp is a linter/checker (likely biome wrapper) =="
rg '"vp"' package.json -A 2 -B 2

echo "== Check src/FormData.ts full context around Line 6 =="
sed -n '1,15p' src/FormData.ts

Repository: node-modules/urllib

Length of output: 763

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Read biome.json =="
cat biome.json

echo ""
echo "== Read oxlint.config.ts =="
cat oxlint.config.ts

echo ""
echo "== Search for 'vp' in package.json =="
rg '"vp"' package.json

Repository: node-modules/urllib

Length of output: 4693

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Check full src/FormData.ts context (lines 1-40) =="
sed -n '1,40p' src/FormData.ts

echo ""
echo "== Verify proposed regex syntax support =="
echo "Checking if /u flag and \\p{ASCII} are supported in Node.js TypeScript..."
</script>

Repository: node-modules/urllib

Length of output: 1741

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Check tsconfig.json for target ES version =="
cat tsconfig.json | grep -A 5 '"target"' || echo "No explicit target found"

echo ""
echo "== Verify Unicode property escape is standard in modern Node.js =="
echo "Node.js 12+ supports \\p{ASCII} with /u flag"

echo ""
echo "== Verify the proposed regex is functionally equivalent =="
echo "Original: /[^\\u0000-\\u007F]/i (case-insensitive, inverted ASCII range)"
echo "Proposed: /[^\\p{ASCII}]/u (Unicode property, negated ASCII)"
echo "Both should match the same thing: any non-ASCII character"

Repository: node-modules/urllib

Length of output: 768

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Check package.json for Node.js version requirement =="
rg '"engines"' package.json -A 3

echo ""
echo "== Check if Biome noControlCharactersInRegex rule is mentioned anywhere =="
rg 'noControlCharactersInRegex' . 2>/dev/null | head -10 || echo "No explicit references found"

Repository: node-modules/urllib

Length of output: 305

ESLint suppress comment won't prevent Biome from flagging the control-character regex

Line 6's ESLint disable applies only to ESLint, not Biome. Since noControlCharactersInRegex is enabled in biome.json's recommended rules, the \u0000 escape will still trigger a linting error. Use a Unicode property escape instead:

Proposed change 
-// eslint-disable-next-line no-control-regex
-const NON_ASCII_RE = /[^\u0000-\u007F]/i;
+const NON_ASCII_RE = /[^\p{ASCII}]/u;

This regex is functionally equivalent, compatible with the project's ES2022 target and Node.js 18.19.0+ requirement, and will not trigger linting errors.

📝 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
// eslint-disable-next-line no-control-regex
const NON_ASCII_RE = /[^\u0000-\u007F]/i;
const NON_ASCII_RE = /[^\p{ASCII}]/u;
🧰 Tools
🪛 Biome (2.4.6)

[error] 6-6: Unexpected control character in a regular expression.

(lint/suspicious/noControlCharactersInRegex)

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/FormData.ts` around lines 5 - 6, Replace the control-character escape
regex used by NON_ASCII_RE with a Unicode property escape to satisfy Biome's
noControlCharactersInRegex rule: change the declaration of NON_ASCII_RE
(currently using /[^\u0000-\u007F]/i) to use a Unicode property escape (for
example /[^\p{ASCII}]/u or /\P{ASCII}/u) and remove the "//
eslint-disable-next-line no-control-regex" comment; ensure the 'u' flag is
present and drop the 'i' flag since it's not needed for ASCII checks.


export class FormData extends _FormData {
_getContentDisposition(value: any, options: any): string | undefined {
Expand All @@ -14,13 +14,13 @@ export class FormData extends _FormData {

if (typeof options.filepath === 'string') {
// custom filepath for relative paths
filename = path.normalize(options.filepath).replace(/\\/g, '/');
filename = path.normalize(options.filepath).replaceAll('\\', '/');
} else if (options.filename || value.name || value.path) {
// custom filename take precedence
// formidable and the browser add a name property
// fs- and request- streams have path property
filename = path.basename(options.filename || value.name || value.path);
} else if (value.readable && value.hasOwnProperty('httpVersion')) {
} else if (value.readable && Object.prototype.hasOwnProperty.call(value, 'httpVersion')) {
// or try http response
filename = path.basename(value.client._httpMessage.path || '');
}
Expand Down
10 changes: 6 additions & 4 deletions src/HttpAgent.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,19 +40,21 @@ export class HttpAgent extends BaseAgent {
const { lookup = dns.lookup, ...baseOpts } = options;

const lookupFunction: LookupFunction = (hostname, dnsOptions, callback) => {
lookup(hostname, dnsOptions, (err, ...args: any[]) => {
lookup(hostname, dnsOptions, (err, ...args: Array<any>) => {
// address will be array on Node.js >= 20
const address = args[0];
const family = args[1];
if (err) return (callback as any)(err, address, family);
if (err) {
return (callback as any)(err, address, family);
}
if (options.checkAddress) {
// dnsOptions.all set to default on Node.js >= 20, dns.lookup will return address array object
if (typeof address === 'string') {
if (!options.checkAddress(address, family, hostname)) {
err = new IllegalAddressError(hostname, address, family);
}
} else if (Array.isArray(address)) {
const addresses = address as { address: string; family: number }[];
const addresses = address as Array<{ address: string; family: number }>;
for (const addr of addresses) {
if (!options.checkAddress(addr.address, addr.family, hostname)) {
err = new IllegalAddressError(hostname, addr.address, addr.family);
Expand All @@ -77,7 +79,7 @@ export class HttpAgent extends BaseAgent {
let hostname = originUrl.hostname;
// [2001:db8:2de::e13] => 2001:db8:2de::e13
if (hostname.startsWith('[') && hostname.endsWith(']')) {
hostname = hostname.substring(1, hostname.length - 1);
hostname = hostname.slice(1, hostname.length - 1);
}
const family = isIP(hostname);
if (family === 4 || family === 6) {
Expand Down
23 changes: 13 additions & 10 deletions src/HttpClient.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ export interface UndiciTimingInfo {
}

// keep typo compatibility
// eslint-disable-next-line @typescript-eslint/no-empty-object-type -- intentional alias for backward compat
export interface UnidiciTimingInfo extends UndiciTimingInfo {}

function noop(): void {
Expand Down Expand Up @@ -112,7 +113,7 @@ export type ClientOptions = {

export const VERSION: string = 'VERSION';
// 'node-urllib/4.0.0 Node.js/18.19.0 (darwin; x64)'
export const HEADER_USER_AGENT: string = `node-urllib/${VERSION} Node.js/${process.version.substring(1)} (${process.platform}; ${process.arch})`;
export const HEADER_USER_AGENT: string = `node-urllib/${VERSION} Node.js/${process.version.slice(1)} (${process.platform}; ${process.arch})`;

function getFileName(stream: Readable): string {
const filePath: string = (stream as any).path;
Expand All @@ -131,7 +132,7 @@ export type RequestContext = {
socketErrorRetries: number;
requestStartTime?: number;
redirects: number;
history: string[];
history: Array<string>;
};

export const channels: {
Expand Down Expand Up @@ -232,7 +233,9 @@ export class HttpClient extends EventEmitter {
const pool = (typeof ref.deref === 'function' ? ref.deref() : ref) as unknown as Pool & { dispatcher: Pool };
// NOTE: pool become to { dispatcher: Pool } in undici@v7
const stats = pool?.stats ?? pool?.dispatcher?.stats;
if (!stats) continue;
if (!stats) {
continue;
}

poolStatsMap[key] = {
connected: stats.connected,
Expand Down Expand Up @@ -376,7 +379,7 @@ export class HttpClient extends EventEmitter {
bodyTimeout = args.timeout[1] ?? bodyTimeout;
} else {
// compatible with urllib@2 timeout string format
headersTimeout = bodyTimeout = typeof args.timeout === 'string' ? parseInt(args.timeout) : args.timeout;
headersTimeout = bodyTimeout = typeof args.timeout === 'string' ? Number.parseInt(args.timeout) : args.timeout;
}
}
if (originalHeaders) {
Expand Down Expand Up @@ -471,7 +474,7 @@ export class HttpClient extends EventEmitter {
requestOptions.method = 'POST';
}
const formData = new FormData();
const uploadFiles: [string, string | Readable | Buffer, string?][] = [];
const uploadFiles: Array<[string, string | Readable | Buffer, string?]> = [];
if (Array.isArray(args.files)) {
for (const [index, file] of args.files.entries()) {
const field = index === 0 ? 'file' : `file${index}`;
Expand Down Expand Up @@ -637,7 +640,7 @@ export class HttpClient extends EventEmitter {
res.status = res.statusCode = response.statusCode;
res.statusMessage = res.statusText = STATUS_CODES[res.status] || '';
if (res.headers['content-length']) {
res.size = parseInt(res.headers['content-length']);
res.size = Number.parseInt(res.headers['content-length']);
}

// https://developer.mozilla.org/en-US/docs/Web/HTTP/Redirections
Expand Down Expand Up @@ -682,11 +685,11 @@ export class HttpClient extends EventEmitter {
if (isCompressedContent && data.length > 0) {
try {
data = contentEncoding === 'gzip' ? gunzipSync(data) : brotliDecompressSync(data);
} catch (err: any) {
if (err.name === 'Error') {
err.name = 'UnzipError';
} catch (error: any) {
if (error.name === 'Error') {
error.name = 'UnzipError';
}
throw err;
throw error;
}
}
if (args.dataType === 'text' || args.dataType === 'html') {
Expand Down
2 changes: 1 addition & 1 deletion src/IncomingHttpHeaders.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@ import type { Except } from 'type-fest';

// fix set-cookie type define https://github.com/nodejs/undici/pull/1893
export interface IncomingHttpHeaders extends Except<HTTPIncomingHttpHeaders, 'set-cookie'> {
'set-cookie'?: string | string[];
'set-cookie'?: string | Array<string>;
}
Loading
Loading