feat: export HttpAgent as standalone class to avoid namespace clutter and support ESM style imports
#119
Conversation
Remove need of `"esModuleInterop": true` when used with Typescript, clearing the module structure, types and imports
Fixes `Error: error:0A00018F:SSL routines::ee key too small`
WalkthroughThe pull request introduces a significant refactoring of the Changes
Sequence DiagramsequenceDiagram
participant User
participant Module as agentkeepalive
participant HttpAgent
User->>Module: Import HttpAgent
Module-->>User: Provide HttpAgent
User->>HttpAgent: Create new instance
HttpAgent-->>User: Return configured agent
User->>HttpAgent: Use for HTTP requests
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
smoke
changed the title
HttpAgent as standalone class to avoid namespace clutterHttpAgent as standalone class to avoid namespace clutter and support ESM style imports
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
example/http_agent.js (1)
37-37: Trim unnecessary console output
Logging the entirehttp_agentobject may produce very large output. Consider logging only essential fields to keep logs manageable.- console.log('keep alive sockets:', http_agent); + console.log('keep alive sockets:', { + statusChanged: http_agent.statusChanged, + sockets: Object.keys(http_agent.sockets).length, + freeSockets: Object.keys(http_agent.freeSockets).length, + });benchmark/echo.js (1)
57-57: Clarify log statement
Renaming the reference fromAgent(...)toHttpAgent(...)in the log provides more accurate details, matching the new class name. Consider additionally renaming the local variableagenttohttpAgentfor full consistency.- console.log('[%s] [worker:%d] HttpAgent(%s,%sms,%s,%s): requests: %d, created: %d, timeout: %d, ...', + console.log('[%s] [worker:%d] httpAgent(%s,%sms,%s,%s): requests: %d, created: %d, timeout: %d, ...',test/test-ECONNRESET.test.js (1)
5-5: Refactor import statement to a named import if desired.
While this import is valid, you could consider using named imports (ES modules) to align with future usage, e.g.,import { HttpAgent } from '...'.benchmark/proxy.js (3)
4-4: Consider replacingvarwithconst.
As recommended by linting tools, usingconst HttpAgentwould improve clarity and readability here.Apply this diff to address it:
-var HttpAgent = require('../').HttpAgent; +const HttpAgent = require('../').HttpAgent;🧰 Tools
🪛 eslint
[error] 4-4: Unexpected var, use let or const instead.
(no-var)
10-15: Avoidvarto align with modern JavaScript standards.
UseconstorletforagentKeepalivesince it is never reassigned.-var agentKeepalive = new HttpAgent({ +const agentKeepalive = new HttpAgent({Additionally, you can apply object shorthand for keys mirroring property names.
🧰 Tools
🪛 eslint
[error] 10-15: Unexpected var, use let or const instead.
(no-var)
[error] 12-12: Expected property shorthand.
(object-shorthand)
[error] 13-13: Expected property shorthand.
(object-shorthand)
16-16: Avoidvarto align with modern JavaScript standards.
UseconstorletforagentHttpsince it is never reassigned.-var agentHttp = new HttpAgent({ +const agentHttp = new HttpAgent({🧰 Tools
🪛 eslint
[error] 16-19: Unexpected var, use let or const instead.
(no-var)
test/http_agent.test.js (1)
108-108: Rename test description to avoid confusion.This test name still references
http.Agent(), but the code now usesHttpAgent. Renaming would avoid confusion and keep the test suite consistent.- it.skip('should request with connection: close with http.Agent()', done => { + it.skip('should request with connection: close with HttpAgent()', done => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
test/fixtures/agenttest-cert.pemis excluded by!**/*.pemtest/fixtures/agenttest-key.pemis excluded by!**/*.pemtest/fixtures/ca.pemis excluded by!**/*.pem
📒 Files selected for processing (13)
README.md(2 hunks)benchmark/echo.js(2 hunks)benchmark/proxy.js(1 hunks)example/http_agent.js(2 hunks)index.d.ts(3 hunks)index.js(1 hunks)test/fixtures/ca.key(1 hunks)test/fixtures/genkey.sh(1 hunks)test/fixtures/ts/index.ts(2 hunks)test/fixtures/ts/tsconfig.json(1 hunks)test/http_agent.test.js(43 hunks)test/server_timeout.test.js(2 hunks)test/test-ECONNRESET.test.js(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- test/fixtures/ca.key
🧰 Additional context used
🪛 eslint
benchmark/proxy.js
[error] 4-4: Unexpected var, use let or const instead.
(no-var)
[error] 6-6: Unexpected var, use let or const instead.
(no-var)
[error] 7-7: Unexpected var, use let or const instead.
(no-var)
[error] 8-8: Unexpected var, use let or const instead.
(no-var)
[error] 10-15: Unexpected var, use let or const instead.
(no-var)
[error] 12-12: Expected property shorthand.
(object-shorthand)
[error] 13-13: Expected property shorthand.
(object-shorthand)
🔇 Additional comments (65)
index.js (1)
3-5: Exporting both the default and a named HttpAgent is consistent with the PR goals
Exporting HttpAgent in both default and named styles allows flexible consumption by TypeScript and CommonJS users. This change aligns well with the PR objective of reducing namespace clutter and addressing TypeScript import issues.
example/http_agent.js (4)
4-4: Good use of named import
Importing HttpAgent through a named property clarifies intent and avoids potential collisions with other default exports.
6-6: Instantiating HttpAgent correctly
Creating a new HttpAgent instance adheres to the proposed interface changes, helping to validate the new export.
14-14: Supplying the agent via http_agent
Attaching the newly instantiated http_agent to request options is straightforward and consistent with the refactor.
43-45: Properly monitoring agent sockets
Accessing getName, sockets, and freeSockets is appropriate for debugging. Ensure no concurrency issues arise if multiple calls occur simultaneously.
test/fixtures/ts/index.ts (5)
1-2: Named import usage
Switching to a named import for HttpAgent, HttpsAgent, and related types is more explicit, improving clarity for TypeScript consumers.
9-9: Using HttpOptions
Declaring httpOpt as HttpOptions clarifies that it is specifically for HttpAgent configuration, eliminating confusion with all-purpose agent types.
15-15: Creating HttpAgent
Instantiating HttpAgent with httpOpt confirms the new named import structure functions correctly and improves type safety.
42-42: Tracking AgentStatus
Retrieving status via getCurrentStatus() and storing it in a typed AgentStatus variable is aligned with the updated agent pattern.
48-55: Consistent usage for HTTPS
Likewise, HttpsOptions and the resulting AgentStatus ensure the new architecture is fully mirrored for HTTPS as well.
benchmark/echo.js (2)
3-3: Loading the HttpAgent from the new export
This reflects the updated module export and ensures that the benchmark script aligns with the refactor.
5-5: Instantiating HttpAgent with keep-alive settings
Explicitly passing keep-alive parameters into the constructor is a correct approach for performance testing.
test/test-ECONNRESET.test.js (1)
30-30: Agent instantiation looks good.
The new HttpAgent provides a more explicit name and clarifies its purpose in the test scenario.
index.d.ts (3)
9-9: Naming convention changes are clear.
Renaming HttpAgent to _HttpAgent to differentiate the internal class from the exported class is acceptable. Ensure that the rest of the codebase references _HttpAgent only internally.
28-31: Deprecation notice is well-defined.
Marking AgentKeepAlive as deprecated, with guidance to use HttpAgent, is a good approach for a transitional release.
57-57: New HttpAgent export finalizes the refactor.
This allows external consumers to properly import the more descriptive entity. Keep thorough documentation around these changes to ensure a smooth transition for end-users.
test/server_timeout.test.js (2)
5-5: Use HttpAgent import aligned with the new export structure.
The import statement aligns with the updated export strategy, clarifying usage for maintainers and test readers.
33-33: Instantiation is accurate for the test environment.
Specifying { keepAlive: true } with HttpAgent ensures behavior remains consistent with prior usage.
test/http_agent.test.js (42)
8-8: Switch to HttpAgent import.
This import statement now references HttpAgent instead of the generic Agent, aligning with the PR’s goal of a standalone class export.
18-18: Same update as line 8.
132-132: Same update as line 8.
183-183: Same update as line 8.
277-277: Same update as line 8.
311-311: Same update as line 8.
349-349: Same update as line 8.
385-385: Same update as line 8.
399-399: Same update as line 8.
428-428: Same update as line 8.
451-451: Same update as line 8.
524-524: Same update as line 8.
575-575: Same update as line 8.
608-608: Same update as line 8.
656-656: Same update as line 8.
683-683: Same update as line 8.
711-711: Same update as line 8.
767-767: Same update as line 8.
819-819: Same update as line 8.
893-893: Same update as line 8.
929-929: Same update as line 8.
970-970: Same update as line 8.
1003-1003: Same update as line 8.
1023-1023: Same update as line 8.
1055-1055: Same update as line 8.
1099-1099: Same update as line 8.
1148-1148: Same update as line 8.
1180-1180: Same update as line 8.
1216-1216: Same update as line 8.
1259-1259: Same update as line 8.
1337-1337: Same update as line 8.
1387-1387: Same update as line 8.
1454-1454: Same update as line 8.
1499-1499: Same update as line 8.
1537-1537: Same update as line 8.
1569-1569: Same update as line 8.
1603-1603: Same update as line 8.
1650-1650: Same update as line 8.
1692-1692: Same update as line 8.
1733-1733: Same update as line 8.
1769-1769: Same update as line 8.
1807-1807: Same update as line 8.
test/fixtures/ts/tsconfig.json (1)
24-24: Confirm removal of esModuleInterop.
Removing this option might break certain default imports in projects that rely on ES module interop. Please verify that all TypeScript imports remain valid, especially in environments without "esModuleInterop".
test/fixtures/genkey.sh (2)
9-9: Increased RSA key size from 1024 to 2048.
This improvement enhances security by using a stronger key strength. Ensure that all references to the old 1024-bit key remain compatible with the 2048-bit version.
13-13: Reusing the updated key for certificate signing.
The CSR command now references the newly generated 2048-bit private key (agenttest-key.pem).
README.md (2)
64-66: Switched to HttpAgent in the code example.
Renaming Agent to HttpAgent clarifies usage and supports the new standalone export. The snippet remains functionally similar.
165-166: Updated usage example to HttpAgent.
Similarly, these lines now demonstrate HttpAgent instead of the older Agent name, in line with the PR’s objectives.
fengmk2
changed the title
HttpAgent as standalone class to avoid namespace clutter and support ESM style importsHttpAgent as standalone class to avoid namespace clutter and support ESM style imports
|
4.6.0 Released @smoke Thanks a lot! |
Checklist
npm testpassesAffected core subsystem(s)
Description of change
This PR changes that
HttpAgentclass is exported as standalone member,allowing use of
import { HttpAgent } from 'agentkeepalive'; orconst HttpAgent = require('agentkeepalive').HttpAgent;It also deprecates use ofimport * as HttpAgent from 'agentkeepalive'; orconst HttpAgent = require('agentkeepalive');This way fixing case when using Typescript and having
"esModuleInterop": false(that is the default), users can not import and use theHttpAgentclass and construct objects of it, because of compiler / type errors likeType 'typeof AgentKeepAlive' has no construct signatures.Summary by CodeRabbit
Documentation
HttpAgentinstead ofAgentHttpAgentacross documentationRefactor
AgenttoHttpAgentin multiple files and test fixturesindex.jsTests
HttpAgentSecurity