feat: use native impit streaming
#2833
Conversation
github-actions
bot
added
the
t-tooling
|
The initial implementation shows slight discrepancies in the types exported from the impit package, requiring awkward casts on this side. Those problems should be fixed in Edit: regular |
| private getStreamWithProgress( | ||
| response: ImpitResponse, | ||
| ): [Readable, () => { percent: number; transferred: number; total: number }] { | ||
| const responseStream = Readable.fromWeb(response.body as ReadableStream<any>); |
There was a problem hiding this comment.
Is the cast necessary? ImpitResponse['body'] looks like it's typed pretty well.
There was a problem hiding this comment.
yup, unfortunately, Readable.fromWeb accepts ReadableStream from node:stream/web, but impit.fetch (and, more importantly, Node's native fetch) return ReadableStream that gets resolved with TS's lib.dom.d.ts type declarations. There is a slight discrepancy somewhere deep between those two types:
There may be a better solution somewhere, but given the (IMO small) size of this issue, a cast solves this just fine.
Bumps the
impitdependency and required Node version for the@crawlee/impit-clientpackage. Makes use of the new nativeReadableStreaminterface.Related to #2756