Typescript: this.fetch returns a Response

[singingwolfboy]

No description provided.

[benmccann]

Yeah, you're right. However, Response isn't something that's defined in this file, so I'm not sure your change will work? The fix might be to move line 15 to the outer scope and use that Res definition

[benmccann]

@bjon thanks for your testing on #1601. You might be interested in testing this one out as well

[benmccann]

Actually, it looks like this might need #1604 first

[singingwolfboy]

However, Response isn't something that's defined in this file, so I'm not sure your change will work?

Response is a global variable in Javascript, and as such, is also a global in Typescript. See lib.dom.d.ts.

When I test this change locally, it appears to work just fine. I'm not sure how to write an automated test for it...

[benmccann]

Ok. I think you need to specify --lib DOM in that case. But it looks like we do that so it's probably okay: https://github.com/sveltejs/sapper-template/blob/a3bd16ec8811cde28034b25c218de8857eea74b7/scripts/setupTypeScript.js#L237

[dummdidumm]

Not sure if this is correct. Fetch depends on the environment it is used in, and some server environments may have a different interface.

[bjon]

Maybe something like? fetch: <T = any>(url: string, options?: any) => Promise<T>;

Now this.fetch(url) and this.fetch<Response>(url), should work.

[dummdidumm]

Not sure if this would bring much benefit over the user just casting it himself. It also makes it "easier" to introduce breaking changes later on if we would want to add more generics like this to the function later on.

[benmccann]

Fetch depends on the environment it is used in, and some server environments may have a different interface.

I'm not sure I follow. I think Sapper uses node-fetch on the server and that defines the interface. Am I missing something?

[dummdidumm]

Is node-fetch available in all environments (Vercel, Lambda etc)?

[benmccann]

I would expect it to be deployed along with the user's app since it's one of Sapper's dependencies:

sapper/package.json

Line 53 in 28e6231

"node-fetch": "^2.6.0",

[ehrencrona]

This is isomorphic code, but the result will be slightly different on server and client. On the client it will always be Response because fetch is standardized in the browser, but what node-fetch returns isn't 100% the same thing. The typings for node-fetch define their own Response. I haven't analyzed what (if any) differences there are.

We should add typings that represent the guarantee of functionality that is always available, which is presumably a subset of Response. I guess the easiest thing would be to just add the most commonly used properties.

Btw, we should add types to options as well.

[thgh]

Looks like the only typing difference is the missing body.formData() method. I would be ok to simply use Response.

How about this?

import type fetchType from 'node-fetch'
type PreloadFetchResponse = Response | ReturnType<typeof fetchType>

[ehrencrona]

That seems like a good idea to me! It does express what type it indeed really is.