chore: remove usage of the prefer-global rules #11279
Conversation
didinele
requested review from
Jiralite,
SpaceEEC,
almeidx,
almostSouji,
ckohen,
kyranet,
monbrey and
sdanialraza
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
Unfortunately, there's no ESLint rules for those. Even so
Argument of type 'Blob' is not assignable to parameter of type 'BodyInit | undefined'.
Property 'slice' is missing in type 'Blob' but required in type 'import("buffer").Blob'.where
|
didinele
force-pushed
the
chore/no-prefer-global
branch
from
42703f9 to
a89e738
Compare
github-actions
bot
removed
packages:builders
packages:proxy-container
packages:util
packages:brokers
labels
didinele
force-pushed
the
chore/no-prefer-global
branch
from
a89e738 to
0e0cf46
Compare
didinele
force-pushed
the
chore/no-prefer-global
branch
from
0e0cf46 to
cb7c0e9
Compare
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThis PR removes explicit imports of Node.js global APIs across the monorepo in favor of using global availability, aligning with the goal to remove n/prefer-global ESLint rules for runtime compatibility. Changes include updating the root ESLint configuration with new rule definitions, removing imports of global APIs (URL, URLSearchParams, timers, TextDecoder, process in specific contexts), and adjusting imports to use explicit process where needed. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~40 minutes Areas requiring extra attention:
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
didinele
force-pushed
the
chore/no-prefer-global
branch
2 times, most recently
from
88a08f2 to
3451711
Compare
|
@coderabbitai review |
didinele
force-pushed
the
chore/no-prefer-global
branch
from
3451711 to
e1032a8
Compare
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/docgen/bin/index.ts (1)
1-27: Missing explicit import ofprocess.Line 24 uses
process.argvwithout an explicit import. Whileprocessis a global in Node.js, add the import statement:import process from 'node:process'near the top of the file with the other imports.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (42)
eslint.config.js(2 hunks)packages/actions/src/releasePackages/releasePackage.ts(0 hunks)packages/api-extractor-model/src/model/SourceLocation.ts(0 hunks)packages/api-extractor/src/cli/RunAction.ts(1 hunks)packages/api-extractor/src/start.ts(1 hunks)packages/brokers/src/brokers/redis/RPCRedis.ts(0 hunks)packages/core/src/api/oauth2.ts(0 hunks)packages/core/src/client.ts(0 hunks)packages/create-discord-bot/bin/index.ts(0 hunks)packages/create-discord-bot/scripts/rename-to-app.mjs(0 hunks)packages/create-discord-bot/src/create-discord-bot.ts(0 hunks)packages/create-discord-bot/template/JavaScript/eslint.config.js(1 hunks)packages/create-discord-bot/template/JavaScript/src/events/interactionCreate.js(0 hunks)packages/create-discord-bot/template/JavaScript/src/index.js(0 hunks)packages/create-discord-bot/template/JavaScript/src/util/deploy.js(0 hunks)packages/create-discord-bot/template/JavaScript/src/util/loaders.js(1 hunks)packages/create-discord-bot/template/TypeScript/eslint.config.js(1 hunks)packages/create-discord-bot/template/TypeScript/src/events/interactionCreate.ts(0 hunks)packages/create-discord-bot/template/TypeScript/src/index.ts(0 hunks)packages/create-discord-bot/template/TypeScript/src/util/deploy.ts(0 hunks)packages/discord.js/scripts/esmDts.mjs(0 hunks)packages/discord.js/scripts/generateRequires.mjs(0 hunks)packages/docgen/bin/index.ts(1 hunks)packages/formatters/__tests__/formatters.test.ts(0 hunks)packages/formatters/src/formatters.ts(0 hunks)packages/proxy/src/handlers/proxyRequests.ts(0 hunks)packages/rest/__tests__/BurstHandler.test.ts(0 hunks)packages/rest/__tests__/DiscordAPIError.test.ts(0 hunks)packages/rest/__tests__/REST.test.ts(1 hunks)packages/rest/__tests__/RequestHandler.test.ts(0 hunks)packages/rest/__tests__/UndiciRequest.test.ts(0 hunks)packages/rest/src/lib/handlers/Shared.ts(1 hunks)packages/rest/src/lib/utils/utils.ts(1 hunks)packages/rest/src/strategies/undiciRequest.ts(1 hunks)packages/scripts/bin/generateSplitDocumentation.ts(2 hunks)packages/scripts/src/generateIndex.ts(2 hunks)packages/scripts/src/generateSplitDocumentation.ts(2 hunks)packages/scripts/src/populateDevDatabaseBranch.ts(2 hunks)packages/voice/__tests__/Secretbox.test.ts(1 hunks)packages/voice/src/receive/AudioReceiveStream.ts(2 hunks)packages/ws/__tests__/strategy/WorkerShardingStrategy.test.ts(0 hunks)packages/ws/src/ws/WebSocketShard.ts(0 hunks)
💤 Files with no reviewable changes (25)
- packages/rest/tests/UndiciRequest.test.ts
- packages/create-discord-bot/template/TypeScript/src/util/deploy.ts
- packages/rest/tests/BurstHandler.test.ts
- packages/create-discord-bot/template/TypeScript/src/index.ts
- packages/core/src/api/oauth2.ts
- packages/brokers/src/brokers/redis/RPCRedis.ts
- packages/api-extractor-model/src/model/SourceLocation.ts
- packages/rest/tests/RequestHandler.test.ts
- packages/discord.js/scripts/esmDts.mjs
- packages/create-discord-bot/scripts/rename-to-app.mjs
- packages/create-discord-bot/bin/index.ts
- packages/create-discord-bot/template/JavaScript/src/util/deploy.js
- packages/create-discord-bot/src/create-discord-bot.ts
- packages/core/src/client.ts
- packages/ws/tests/strategy/WorkerShardingStrategy.test.ts
- packages/actions/src/releasePackages/releasePackage.ts
- packages/proxy/src/handlers/proxyRequests.ts
- packages/discord.js/scripts/generateRequires.mjs
- packages/rest/tests/DiscordAPIError.test.ts
- packages/create-discord-bot/template/TypeScript/src/events/interactionCreate.ts
- packages/create-discord-bot/template/JavaScript/src/events/interactionCreate.js
- packages/ws/src/ws/WebSocketShard.ts
- packages/formatters/tests/formatters.test.ts
- packages/formatters/src/formatters.ts
- packages/create-discord-bot/template/JavaScript/src/index.js
🧰 Additional context used
🧬 Code graph analysis (2)
packages/voice/src/receive/AudioReceiveStream.ts (4)
packages/discord.js/src/client/Client.js (1)
process(3-3)packages/discord.js/src/managers/RoleManager.js (1)
process(3-3)packages/discord.js/src/managers/GuildManager.js (1)
process(3-3)packages/discord.js/src/managers/GuildChannelManager.js (1)
process(3-3)
eslint.config.js (2)
packages/collection/src/collection.ts (1)
merge(994-1022)packages/api-extractor-utils/src/tsdoc/CommentNode.ts (1)
node(24-28)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Tests
🔇 Additional comments (24)
packages/voice/__tests__/Secretbox.test.ts (1)
1-1: ExplicitBufferimport aligns with non-global usage goalImporting
Bufferfrom'node:buffer'is correct and keeps the test from depending on a globalBuffer, matching the PR’s objective and the wayBuffer.fromis used below.packages/create-discord-bot/template/JavaScript/src/util/loaders.js (1)
3-3: Relying on globalURLinstead of importing it looks correct hereThe change to only import
fileURLToPathand use the globalURLindir instanceof URLkeeps behavior the same while removing an unnecessary import, which aligns with the prefer‑global cleanup. Just ensure your ESLint/TypeScript configuration for this template environment treatsURLas a global so you don’t get false positives.packages/scripts/bin/generateSplitDocumentation.ts (2)
1-2: Shebang and lint directive change look goodKeeping the shebang and dropping the
eslint-disablecomment is fine here and doesn’t affect runtime behavior.
23-25: Arrow function refactor is behavior‑preservingConverting these async callbacks to concise expression bodies keeps the same return values and types; no issues from this change.
packages/rest/src/lib/handlers/Shared.ts (2)
1-2: ESLint disable is appropriate for cross-environment compatibility.The file-level disable is necessary because this code uses
globalThis.processwith runtime checks (lines 188-189) to safely callprocess.emitWarning()only when running in Node.js environments. Then/prefer-global/processrule would typically prefer directprocessusage, but that would throw errors in browser/edge runtimes whereprocessis undefined.The defensive pattern
typeof globalThis.process !== 'undefined'is the correct approach for libraries that need to run across multiple environments.
188-189: LGTM! Proper defensive check for cross-environment compatibility.The runtime check for
processavailability before callingemitWarning()is the correct pattern for code that runs in multiple environments (Node.js, browser, edge). The fallback toconsole.warn()on line 191 ensures the warning is always emitted appropriately.packages/rest/src/lib/utils/utils.ts (1)
1-1: Type-onlyBufferimport correctly matchesisBufferLikeusage (LGTM).
Bufferis only used in the type predicate ofisBufferLike, and the runtime check already relies oninstanceof Uint8Array/Uint8ClampedArray/ArrayBuffer, which covers NodeBufferinstances viaUint8Array. ImportingBufferas atypefrom'node:buffer'keeps runtime behavior unchanged while avoiding reliance on a global type, which fits the PR’s goals.Please re-run the REST package type-check/tests to confirm there are no unexpected diagnostics around
Bufferusage after this change.Also applies to: 149-151
packages/rest/__tests__/REST.test.ts (1)
1-1: LGTM: Explicit Buffer import added.The explicit import of Buffer from node:buffer is appropriate here, as the file uses Buffer.from() at line 280.
packages/rest/src/strategies/undiciRequest.ts (2)
1-1: LGTM: Explicit Buffer import added.The explicit import of Buffer from node:buffer is correct for the Buffer.concat() usage at lines 73 and 81.
60-61: LGTM: Global URLSearchParams usage.The code correctly relies on the global URLSearchParams, which is available in Node.js 10+ and matches the PR's objective to use global implementations where appropriate.
packages/api-extractor/src/cli/RunAction.ts (1)
6-6: LGTM: Explicit process import added.The explicit import of process from node:process is appropriate for the usages at lines 136 (process.cwd()) and 154 (process.exitCode).
packages/api-extractor/src/start.ts (1)
5-5: LGTM: Import style updated to default import.The change from namespace import to default import (
import process from 'node:process') is consistent with the import style used in other files in this PR and correctly supports the usage at line 19.packages/scripts/src/generateSplitDocumentation.ts (2)
3-3: LGTM: Consistent with script file pattern.The change from
import { cwd }toimport processand usingprocess.cwd()is consistent with the pattern applied across other script files (generateIndex.ts, populateDevDatabaseBranch.ts) in this PR.
1004-1011: LGTM: All cwd() usages updated correctly.All three usages of path construction have been correctly updated to use
process.cwd()instead of the previously importedcwd()function.packages/scripts/src/generateIndex.ts (2)
3-3: LGTM: Consistent import pattern.The change to
import process from 'node:process'aligns with the pattern used across all script files in this PR.
154-160: LGTM: All path resolutions updated correctly.All usages of path construction have been correctly updated to use
process.cwd()instead of the previously importedcwd()function.packages/scripts/src/populateDevDatabaseBranch.ts (2)
2-2: LGTM: Consistent import pattern.The change to
import process from 'node:process'is consistent with other script files and supports multiple process usages in this file (process.env, process.cwd(), process.chdir()).
8-11: LGTM: Multiple process API usages covered.The explicit process import correctly supports process.env (line 8), process.cwd() (line 11), and process.chdir() (line 11).
packages/voice/src/receive/AudioReceiveStream.ts (2)
2-2: LGTM!The change from a named
{ nextTick }import to a defaultprocessimport aligns with the new'n/prefer-global/process': [2, 'never']rule and is consistent with patterns used elsewhere in the monorepo (e.g.,packages/discord.js/src/client/Client.js).
82-82: LGTM!Updated call site correctly uses
process.nextTick()to match the import change.packages/create-discord-bot/template/JavaScript/eslint.config.js (1)
14-21: LGTM!The added rules mirror those in the root
eslint.config.jsnodeRuleset, ensuring template-generated projects follow the same global/import conventions as the monorepo. The rule choices are consistent:bufferandprocessrequire explicit imports for runtime compatibility, while others likeURL,URLSearchParams,TextEncoder, andTextDecoderuse globals.packages/create-discord-bot/template/TypeScript/eslint.config.js (1)
21-28: LGTM!Rules are consistent with both the JavaScript template and the root
nodeRuleset, maintaining uniform linting across all project templates.eslint.config.js (2)
20-32: LGTM!Centralizing the global preference rules in
nodeRulesetis a clean approach. The rule configuration establishes clear conventions:bufferandprocessrequire explicit imports for runtime compatibility, whileconsole,TextDecoder,TextEncoder,URL, andURLSearchParamsuse globals which are available in modern Node.js environments.
115-115: LGTM!Including
nodeBinRulesetin the exported config correctly applies the shebang rule exemption to bin files.
didinele
force-pushed
the
chore/no-prefer-global
branch
from
e1032a8 to
f10860a
Compare
didinele
force-pushed
the
chore/no-prefer-global
branch
from
f10860a to
8d78c19
Compare
github-project-automation
bot
moved this from Review in Progress
to Review Approved
in discord.js
should I have ignored api-extractor?
Closes #11272