chore: new @types/node version compatibility
#164
Conversation
github-actions
bot
added
the
t-tooling
There was a problem hiding this comment.
Pull Request Overview
This PR addresses compatibility issues introduced by the new @types/node version by implementing missing methods and adding @ts-ignore comments to bypass stricter type definitions. Key changes include:
- Updated proxy hook implementations with additional ts-ignore directives.
- Extended the wrapped agent interface with new methods and type handling.
- Adjusted ESLint rules to warn on ts-comment usage.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/hooks/proxy.ts | Added ts-ignore annotations for HTTP/HTTPS proxy agent instantiation to resolve type narrowing issues. |
| src/agent/wrapped-agent.ts | Implemented new methods (createConnection, keepSocketAlive, reuseSocket, getName) with ts-ignore usage. |
| src/agent/h1-proxy-agent.ts | Modified constructors and connection creation functions with ts-ignore to comply with updated type definitions. |
| .eslintrc.json | Updated ESLint configuration by adding a rule to warn on ts-comment usage. |
Comments suppressed due to low confidence (1)
.eslintrc.json:12
- [nitpick] Confirm that setting ban-ts-comment to warn aligns with the project’s long-term strategy for handling type suppressions, as current usage of @ts-ignore is widespread.
"@typescript-eslint/ban-ts-comment": "warn"
src/agent/h1-proxy-agent.ts
Outdated
| constructor(options: AgentOptions) { | ||
| super(options); | ||
|
|
||
| // @ts-ignore New @types/node patch has narrower types than got-scraping exported interfaces |
There was a problem hiding this comment.
[nitpick] Review the need for @ts-ignore here and consider adding a TODO for revisiting this once updated types are available.
| // @ts-ignore New @types/node patch has narrower types than got-scraping exported interfaces | |
| // @ts-expect-error New @types/node patch has narrower types than got-scraping exported interfaces | |
| // TODO: Revisit this once updated types are available for @types/node or got-scraping. |
| return this; | ||
| } | ||
|
|
||
| createConnection(options: NetConnectOpts, callback?: (err: Error | null, stream: Duplex) => void): Duplex { |
There was a problem hiding this comment.
or is the actual fix those new methods?
There was a problem hiding this comment.
Partially, the linked @types/node PR adds those methods to the HttpAgent interface, so we need to implement those here to implement the interface correctly (so that TS doesn't throw compilation errors).
…new @types/node" This reverts commit 239496b.
|
seems to be working fine, lets ship a stable release right ahead i guess? |
Caused by a recent update of
@types/node(see here DefinitelyTyped/DefinitelyTyped#73095). By implementing the missing methods (and wrapping other calls in@ts-ignore), this should fix the downstream build errors (e.g. here)Closes #163