-
Notifications
You must be signed in to change notification settings - Fork 1k
feat: use native Request
/ Response
interface
#3163
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
base: v4
Are you sure you want to change the base?
Changes from all commits
b22aaed
aac26de
ee5f2b2
450a393
47711c6
e68e8fc
709814c
20db2c8
f55b0d3
414b3bb
f539905
34a0833
de8855e
9675723
e2b338b
6635213
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,5 +1,5 @@ | ||
import type { BatchAddRequestsResult, Dictionary } from '@crawlee/types'; | ||
import type { OptionsInit, Response as GotResponse } from 'got-scraping'; | ||
import type { OptionsInit } from 'got-scraping'; | ||
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 would love to see this go as well 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'd prefer to do that as a part of a separate PR. Removing |
||
import type { ReadonlyDeep } from 'type-fest'; | ||
|
||
import type { Configuration } from '../configuration.js'; | ||
|
@@ -163,7 +163,7 @@ export interface CrawlingContext<Crawler = unknown, UserData extends Dictionary | |
* }, | ||
* ``` | ||
*/ | ||
sendRequest<Response = string>(overrideOptions?: Partial<OptionsInit>): Promise<GotResponse<Response>>; | ||
sendRequest(overrideOptions?: Partial<OptionsInit>): Promise<Response>; | ||
} | ||
|
||
/** | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
import type { Readable } from 'node:stream'; | ||
|
||
import type { AllowedHttpMethods } from '@crawlee/types'; | ||
import { applySearchParams, type SearchParams } from '@crawlee/utils'; | ||
|
||
import type { FormDataLike } from './form-data-like.js'; | ||
|
@@ -15,24 +16,6 @@ type Timeout = | |
} | ||
| { request: number }; | ||
|
||
type Method = | ||
| 'GET' | ||
| 'POST' | ||
| 'PUT' | ||
| 'PATCH' | ||
| 'HEAD' | ||
| 'DELETE' | ||
| 'OPTIONS' | ||
| 'TRACE' | ||
| 'get' | ||
| 'post' | ||
| 'put' | ||
| 'patch' | ||
| 'head' | ||
| 'delete' | ||
| 'options' | ||
| 'trace'; | ||
|
||
/** | ||
* Maps permitted values of the `responseType` option on {@apilink HttpRequest} to the types that they produce. | ||
*/ | ||
|
@@ -79,7 +62,7 @@ export interface HttpRequest<TResponseType extends keyof ResponseTypes = 'text'> | |
[k: string]: unknown; // TODO BC with got - remove in 4.0 | ||
|
||
url: string | URL; | ||
method?: Method; | ||
method?: AllowedHttpMethods; | ||
headers?: SimpleHeaders; | ||
body?: string | Buffer | Readable | Generator | AsyncGenerator | FormDataLike; | ||
|
||
|
@@ -146,6 +129,14 @@ interface HttpResponseWithoutBody<TResponseType extends keyof ResponseTypes = ke | |
request: HttpRequest<TResponseType>; | ||
} | ||
|
||
export class ResponseWithUrl extends Response { | ||
override url: string; | ||
constructor(body: BodyInit | null, init: ResponseInit & { url?: string }) { | ||
super(body, init); | ||
this.url = init.url ?? ''; | ||
} | ||
} | ||
|
||
/** | ||
* HTTP response data as returned by the {@apilink BaseHttpClient.sendRequest} method. | ||
*/ | ||
|
@@ -169,7 +160,7 @@ export interface StreamingHttpResponse extends HttpResponseWithoutBody { | |
* Type of a function called when an HTTP redirect takes place. It is allowed to mutate the `updatedRequest` argument. | ||
*/ | ||
export type RedirectHandler = ( | ||
redirectResponse: BaseHttpResponseData, | ||
redirectResponse: Response, | ||
updatedRequest: { url?: string | URL; headers: SimpleHeaders }, | ||
) => void; | ||
|
||
|
@@ -182,12 +173,12 @@ export interface BaseHttpClient { | |
*/ | ||
sendRequest<TResponseType extends keyof ResponseTypes = 'text'>( | ||
request: HttpRequest<TResponseType>, | ||
): Promise<HttpResponse<TResponseType>>; | ||
): Promise<Response>; | ||
|
||
/** | ||
* Perform an HTTP Request and return after the response headers are received. The body may be read from a stream contained in the response. | ||
*/ | ||
stream(request: HttpRequest, onRedirect?: RedirectHandler): Promise<StreamingHttpResponse>; | ||
stream(request: HttpRequest, onRedirect?: RedirectHandler): Promise<Response>; | ||
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. Isn't the stream method obsolete? The web Response class can be streamed using 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. Good point, it actually is 👍 I'd prefer to do this in a separate PR, for the same reasons as the total |
||
} | ||
|
||
/** | ||
|
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.
This will cause a merge conflict with the ContextPipeline PR. I would like to go first if that's possible (the vessel that's harder to steer has the right of way).