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
URL is missing in typescript definitions for RequestInfo #1261
Comments
Just to give a bit more info here. node-fetch v2.6.1 allowed this: const mySpecialUrl = new URL('http://google.com');
return fetch(mySpecialUrl); Now it gives a TS error since fetch is defined as export default function fetch(url: RequestInfo, init?: RequestInit): Promise<Response>; where In definitely-typed the RequestInfo is I feel like I should be able to pass a |
Actually it's suppose to only accept a string and not any URL at all. but i stand by that TypeScript isn't a silverbullet. it's bad at handling a loosed typing language, there are many things i hate about TS, like not being able to do: |
@jimmywarting from a quick test in Chrome browser (using native browser fetch) I see the url can be passed as a I believe node-fetch should act the same as browser fetch API as it is porting the functionality to node env. |
We are already acting as the browser, the first thing we do is taking the argument and passing it into the URL constructor to parse it and validate it. we are not restricting the input to only a string (all doe the type definition don't currently allow it...) |
I disagree, the browser will let me pass a URL instance, node-fetch will not as it fails the TS definition. node-fetch has the signature: Line 179 in 2603c67
where RequestInfo is: Line 118 in 2603c67
Which means I cannot pass
Maybe this should be the case if the argument is
Yes you are, well, Even though the |
also consider a use case when you construct an URL before passing it to const url = new URL(
`path/with/${variable}/goes/${here}`,
this.baseUrl,
);
const response = await this.fetch(url);
... Using |
This is my (hopefully 🤞) short term solution, I would like to revert this commit once node-fetch updates the TS signature.
And the URL API is ok with passing a URL instance to the constructor |
right, that is |
Correct... Looking at But I agree that it is useful do do in fact looking at the spec of the fetch api it also says: nowhere dose it say it supports a URL as an input, the magic lies in converting the argument to a USVString if the input isn't a Request object It would be sweet if TypeScript somehow could different a pure |
Continuing discussion from #1695: I also explored an option to make it accept anything that implements diff --git a/node_modules/node-fetch/@types/index.d.ts b/node_modules/node-fetch/@types/index.d.ts
index f68dd28..0541422 100644
--- a/node_modules/node-fetch/@types/index.d.ts
+++ b/node_modules/node-fetch/@types/index.d.ts
@@ -145,9 +145,9 @@ export interface Body extends Pick<BodyMixin, keyof BodyMixin> {}
export type RequestRedirect = 'error' | 'follow' | 'manual';
export type ReferrerPolicy = '' | 'no-referrer' | 'no-referrer-when-downgrade' | 'same-origin' | 'origin' | 'strict-origin' | 'origin-when-cross-origin' | 'strict-origin-when-cross-origin' | 'unsafe-url';
-export type RequestInfo = string | Request;
-export class Request extends BodyMixin {
- constructor(input: RequestInfo, init?: RequestInit);
+export type RequestInfo<T extends {toString: () => string}> = string | T | Request<T>;
+export class Request<T extends {toString: () => string}> extends BodyMixin {
+ constructor(input: RequestInfo<T>, init?: RequestInit);
/**
* Returns a Headers object consisting of the headers associated with request. Note that headers added in the network layer by the user agent will not be accounted for in this object, e.g., the "Host" header.
@@ -177,7 +177,7 @@ export class Request extends BodyMixin {
* A referrer policy to set request’s referrerPolicy.
*/
readonly referrerPolicy: ReferrerPolicy;
- clone(): Request;
+ clone(): Request<T>;
}
type ResponseType = 'basic' | 'cors' | 'default' | 'error' | 'opaque' | 'opaqueredirect';
@@ -216,4 +216,4 @@ export class AbortError extends Error {
}
export function isRedirect(code: number): boolean;
-export default function fetch(url: RequestInfo, init?: RequestInit): Promise<Response>;
+export default function fetch<T extends {toString: () => string}>(url: RequestInfo<T>, init?: RequestInit): Promise<Response>; However, this will make So just adding diff --git a/node_modules/node-fetch/@types/index.d.ts b/node_modules/node-fetch/@types/index.d.ts
index f68dd28..286fee5 100644
--- a/node_modules/node-fetch/@types/index.d.ts
+++ b/node_modules/node-fetch/@types/index.d.ts
@@ -145,7 +145,7 @@ export interface Body extends Pick<BodyMixin, keyof BodyMixin> {}
export type RequestRedirect = 'error' | 'follow' | 'manual';
export type ReferrerPolicy = '' | 'no-referrer' | 'no-referrer-when-downgrade' | 'same-origin' | 'origin' | 'strict-origin' | 'origin-when-cross-origin' | 'strict-origin-when-cross-origin' | 'unsafe-url';
-export type RequestInfo = string | Request;
+export type RequestInfo = string | URL | Request;
export class Request extends BodyMixin {
constructor(input: RequestInfo, init?: RequestInit); I have created PR with a proposed change in case if you find the arguments above reasonable: #1696 |
Sounds good to me, it's what the official TypeScript typings does as well 👍 |
🎉 This issue has been resolved in version 3.3.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
node-fetch: 3.0.0
Note, jsdoc for fetch lists URL as supported input.
The text was updated successfully, but these errors were encountered: