[node] Update http2 definitions for 9.4 #23330
Conversation
typescript-bot
added
Popular package
|
@kjin Thank you for submitting this PR! 🔔 @parambirs @tellnes @WilcoBakker @octo-sniffle @smac89 @Flarna @mwiktorczyk @wwwy3y3 @DeividasBakanas @alvis @OliverJAsh @eps1lon @Hannes-Magnusson-CK @jkomyno @ajafff - please review this PR in the next few days. Be sure to explicitly select If no reviewer appears after a week, a DefinitelyTyped maintainer will review the PR instead. |
typescript-bot
added
the
Author is Owner
types/node/index.d.ts
Outdated
| readonly destroyed: boolean; | ||
| readonly encrypted?: boolean; | ||
| goaway(code?: number, lastStreamID?: number, opaqueData?: Buffer): void; // TODO |
There was a problem hiding this comment.
What is missing to remove the TODO?
Also: doc mentions <Buffer> | <TypedArray> | <DataView> for opaqueData.
There was a problem hiding this comment.
Yeah -- that was exactly it... it was supposed to be a reminder to myself to change the type. Will fix.
Good catch!
| readonly remoteSettings: Settings; | ||
| rstStream(stream: Http2Stream, code?: number): void; | ||
| setTimeout(msecs: number, callback?: () => void): void; | ||
| shutdown(callback?: () => void): void; | ||
| shutdown(options: SessionShutdownOptions, callback?: () => void): void; |
There was a problem hiding this comment.
As shutdown is gone also SessionShutdownOptions can be removed.
| @@ -6181,15 +6181,10 @@ declare module "http2" { | |||
|
|
|||
| export interface Http2Stream extends stream.Duplex { | |||
| readonly aborted: boolean; | |||
| close(code: number, callback?: () => void): void; | |||
There was a problem hiding this comment.
I think there should be also a closed: boolean and pending: boolean (see https://nodejs.org/dist/latest/docs/api/http2.html#http2_http2stream_closed and https://nodejs.org/dist/latest/docs/api/http2.html#http2_http2stream_pending)
| readonly destroyed: boolean; | ||
| readonly encrypted?: boolean; | ||
| goaway(code?: number, lastStreamID?: number, opaqueData?: Buffer | DataView /*| TypedArray*/): void; |
There was a problem hiding this comment.
Should we add a TypedArray alias in the meantime until typescript offers a built-in solution in one of their lib files?
type TypedArray = Int8Array | Uint8Array | Int16Array | Uint16Array | Int32Array | Uint32Array | Uint8ClampedArray | Float32Array | Float64Array;
There was a problem hiding this comment.
Is that going to cause re-definition issues when the built-in TypedArray becomes available? If so maybe it's better to either not include, or have a type with a different name. Since other places use TypedArray as well, it shouldn't be difficult to substitute all in a separate PR.
There was a problem hiding this comment.
As long as it's not defined in the global scope, you should be okay.
typescript-bot
added
the
Unmerged
|
A definition author has approved this PR ⭐️. A maintainer will merge this PR shortly. If it shouldn't be merged yet, please leave a comment saying so and we'll wait. Thank you for your contribution to DefinitelyTyped! |
typescript-bot
removed
the
Unmerged
[node] Update http2 definitions for 9.4
npm run lint package-name(ortscif notslint.jsonis present).