-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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.
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
fix(deps): update undici to 4.x #8842
Changes from all commits
fea217c
e79239e
4cc8333
b92f97d
206c0ab
3644418
c6b0286
6ddf052
487d6a8
22b805c
3a2df2c
6079f14
50b79d6
9659281
c0cebfd
32ac3e2
65e110c
37f6c02
db39f11
79460db
d885a7b
63c83bc
92a3334
42aa830
4dd5fb0
a811b01
ac83530
fb7d30d
c2a8ed8
347dd06
fcc6f06
6b860b3
6c82dc0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,14 +1,17 @@ | ||
import getStream from 'get-stream' | ||
import type { Client } from 'undici' | ||
import { Pool } from 'undici' | ||
import type { Dispatcher, Pool } from 'undici' | ||
import type { URL } from 'url' | ||
|
||
export type Result<R> = { | ||
statusCode: Client.ResponseData['statusCode'] | ||
headers: Client.ResponseData['headers'] | ||
statusCode: Dispatcher.ResponseData['statusCode'] | ||
headers: Dispatcher.ResponseData['headers'] | ||
data: R | ||
} | ||
|
||
// because undici lazily loads llhttp wasm which bloats the memory | ||
// TODO: hopefully replace with `import` but that causes segfaults | ||
const undici = () => require('undici') | ||
Comment on lines
+11
to
+13
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So that's our current workaround for the segfaults for the Binary? any idea why is this happening? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I initially wanted to do There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Worth opening a PR again to see if it still happens, and if so maybe point it out to Undici/Node ppl? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tried this today. I made |
||
|
||
/** | ||
* Assertion function to make sure that we have a pool | ||
* @param pool | ||
|
@@ -53,10 +56,11 @@ export class Connection { | |
open(url: string | URL, options?: Pool.Options) { | ||
if (this._pool) return | ||
|
||
this._pool = new Pool(url, { | ||
this._pool = new (undici().Pool)(url, { | ||
connections: 1000, | ||
keepAliveMaxTimeout: 600e3, | ||
headersTimeout: 0, | ||
bodyTimeout: 0, | ||
...options, | ||
}) | ||
} | ||
|
@@ -72,8 +76,8 @@ export class Connection { | |
async raw<R>( | ||
method: 'POST' | 'GET', | ||
endpoint: string, | ||
headers?: Client.DispatchOptions['headers'], | ||
body?: Client.DispatchOptions['body'], | ||
headers?: Dispatcher.DispatchOptions['headers'], | ||
body?: Dispatcher.DispatchOptions['body'], | ||
) { | ||
assertHasPool(this._pool) | ||
|
||
|
@@ -84,8 +88,7 @@ export class Connection { | |
'Content-Type': 'application/json', | ||
...headers, | ||
}, | ||
body, | ||
bodyTimeout: 0, | ||
body: body, | ||
}) | ||
|
||
const result: Result<R> = { | ||
|
@@ -104,7 +107,11 @@ export class Connection { | |
* @param headers | ||
* @returns | ||
*/ | ||
post<R>(endpoint: string, body?: Client.DispatchOptions['body'], headers?: Client.DispatchOptions['headers']) { | ||
post<R>( | ||
endpoint: string, | ||
body?: Dispatcher.DispatchOptions['body'], | ||
headers?: Dispatcher.DispatchOptions['headers'], | ||
) { | ||
return this.raw<R>('POST', endpoint, headers, body) | ||
} | ||
|
||
|
@@ -115,7 +122,7 @@ export class Connection { | |
* @param headers | ||
* @returns | ||
*/ | ||
get<R>(path: string, headers?: Client.DispatchOptions['headers']) { | ||
get<R>(path: string, headers?: Dispatcher.DispatchOptions['headers']) { | ||
return this.raw<R>('GET', path, headers) | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this example for @millsp ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It used to show how to inline wasm when the wasm file is not directly imported. However, in the mean time undici changed how they load the wasm and now puts the wasm contents in a .js file instead of doing a
readFileSync
so this is not needed anymore and works well when bundled.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we remove this then in a follow up PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can probably trim it a bit, but I'd like to keep an example for how to use
replaceWithPlugin
.