node-fetch: v3.0.0 `await res.json()` is `unknown`

this is unusable in typescript. v3.0.0 types the return value of await res.json() as unknown which means “this is dangerous info, you have no idea what it is. don’t touch it to avoid runtime errors.” but the point of a fetch request is to get the information back. you have to touch it. the type should stay any.

Reproduction

Steps to reproduce the behavior:

  1. use node-fetch in a typescript project
  2. try to build
  3. see error

Expected behavior

for the build to succeed

About this issue

  • Original URL
  • State: open
  • Created 3 years ago
  • Reactions: 39
  • Comments: 39 (11 by maintainers)

Most upvoted comments

I don’t mind defaulting to unknown instead of any, but can we have something like this json<T = unknown>(): Promise<T>; instead?

Aha, I see. Up to you of course. In my opinion it’s a bit more ergonomic to provide a type argument in the cases where you opt out of any runtime checks anyway — even if DefinitelyTyped thinks this is a common mistake. 😉

// Explicit type cast
const json1 = (await response.json()) as MyJson;
// Alternative type cast, not much better
const json2 = await (response.json() as Promise<MyJson>);
// Doesn't work anymore, 'unknown' is not assignable to type 'MyJson'
const json3: MyJson = await response.json();
// So pretty ;-)
const json4 = await response.json<MyJson>();

Unless TypeScript provides a native json type (https://github.com/microsoft/TypeScript/issues/1897), I think that unknown is absolutely preferred over any.

If you have it return any, you don’t get any type safety at all and the benefits of using TypeScript is being negated.

It’s very easy to cast the value to whatever type you want, and then it’s very clear in the code that you are saying to the compiler “trust me, this data will always have the type I’m saying”:

const data = await res.json() as any as Foobar

edit to add:

“this is dangerous info, you have no idea what it is. don’t touch it to avoid runtime errors.”

Actually, I think it more means: “you have no idea what it is, introspect the data to avoid runtime errors”

I was already casting my response like so:

const response = await fetch(url, { headers });
const json: MySpecialResponseType = await response.json();

Since any is pretty much useless for type safety so I think switching to unknown is the right one. The simple solution is to simply cast to your wanted type like this:

const response = await fetch(url, { headers });
const json = (await response.json()) as MySpecialResponseType;

The more “robust” and “correct” solution would be to use a type guard check to 100% make sure your response actually is the type you expect, this is invaluable when interacting with 3rd party endpoints rather than your own I guess.

one fast workaround would be (if this never get’s reversed to any)

const res = (await response.json()) as any;

I prefer to be as strict as possible and avoid using any. Instead I use unknown or never. User defined type guards are how I:

  1. Validate the response JSON is indeed what my business logic needs (throwing an exception if it isn’t)
  2. Narrow the type returned from response.json()

As @martijndeh has indicated, if you don’t want a runtime check, the response.json<MyType> is the way to go.

[…] but can we have something like this json<T = unknown>(): Promise<T>; instead?

I don’t think that we should do this. It was discussed in both of the links that I posted above, but the argument basically boils down to that it’s a hidden forced typecast. I think that it’s easy to accidentally miss that you are unsafely changing the type of the data, when no compile time or runtime check is actually being done on it.

edit to add:

DefinitelyTyped also lists this as something you shouldn’t do under their common misstakes

TS types the result of .json() as Promise<any> for browsers so if anything it should follow the types made by TS itself, and not make up stuff because you think it makes sense but you actually just misread documentation. We shouldn’t need that workaround although yes it would work if you want an explicit any

Some relevant discussions:

Personally, I feel that unknown provides the least amount of danger of shooting oneself in the foot, which I think it most important. But I can see us going with any to match dom.d.ts. Do note that these types was added to dom.d.ts before the unknown type was added to TypeScript, so at the time they didn’t had a choice…

Got notified of @Bessonov’s comment today and came back to this issue in 2022 to say that I learned about assertion functions (here’s another link about it) 2 months ago and it’s useful if you don’t plan on using runtime validation. I have never seen official TS documentation on the asserts keyword 🤷 but it’s nifty and useful when you trust the shape of a thing.

When I don’t trust the shape of a thing, I still use runtime validation. When I do, I have been using runtypes to type and validate an API response object more conveniently! (ty @guyellis for showing me that package 😄)

I’d also vote for json<T = unknown>(): Promise<T> so that I’m forced to deal with the fallout of my own type-laziness 😅.

Instead of unknown, I’d even be cool with Array<unknown> | Record<string | number, unknown> as that is closer to what JSON is allowed to be. Ultimately, this comes down to the JSON.parse() method which is the source of my temptation to be type-lazy 😈

If the type doesn’t matter to you then let it stay as unknown. But the fact remains, there is no way to be absolutely sure what the server returned at compile time, that is why there are only 2 ways out: cast to tell TS you know better and you know you trust the response (which you had to do anyway with any type) or, you can have runtime checks with type guards which TS can understand and help you out at compile time. Unknown type is a better choice for the response than any. I’m just offering people a quick solution to the error

From my point of view, it’s wrong and you make it more complicated than it should be. But let me explain this. Actually, I was wrong.

First of all, I’m type-safety advocate. But fetch is very special, because it doesn’t have any type-safety mechanics. You just get an arbitrary blob of something. You can’t be even sure that you can call .json() at all. In this situation you can’t type the body. There is no way. Using unknown instead of any do only one thing: it forces to give some imaginary type. All json() as MyData or json<MyData>() has nothing to do with reality.

To make the body type safe (and after checking that it is truly json body) you must check it at runtime. For example with using a combination of:

And because of that there is no benefit trying make json() type safe with typescript only. Therefore, I vote for json<T = unknown>(): Promise<T> just because it is more ergonomic and more expected. But I was even fine with any for this special case - there was no surprises with fetch.

I see. In which case, we should just be following whatever is set in the official window.fetch typings (if they exist).

@tatemz

and it’s useful if you don’t plan on using runtime validation

I’ve corrected my comment. I meant the combination of OpenAPI definition, ajv and assertion function. I use this combination in all places:

  • validation of http-requests bodies coming outside aka inside request handler
  • validation of events coming from kafka
  • validation of fetch responses.

For example:

const payload = await parseJSON(ctx, req) // <-- get json from a request, but works for fetch responses in the same way
ensureUserProfileSearchRoot(ctx, payload) // <-- assertion function with runtime validation against OpenAPI definition - created any way
const userProfileSearchRoot = UserProfileSearchRootFromJSON(payload) // <-- generated by [OpenAPI generator](https://github.com/OpenAPITools/openapi-generator) from the same OpenAPI definition
return searchUserProfile(ctx, userProfileSearchRoot) // <-- type safe usage

@LinusU

Do you think that typing await response.json<Foo>() is so much more convenient than typing await response.json() as Foo?

I see two points:

  1. For me it’s more convenient. But, well, I wouldn’t die by typing as (4 chars) instead of <> (2 chars) 😃
  2. It’s more expected. Like some others in this topic, I tried to type json<MyData>() before I was surprised by the definition.

The benefit of having the latter is that organisations that really care about type safety can use lints to disallow unsafe type casting, whereas if we allow the former there is no way to statically infer that unsafe type casting is happening in the code.

Damn, I agree with it. It’s a very strong argument for me, even I’m not convinced that it is the majority of ts users.

[…] but can we have something like this json<T = unknown>(): Promise<T>; instead?

I don’t think that we should do this. It was discussed in both of the links that I posted above, but the argument basically boils down to that it’s a hidden forced typecast. I think that it’s easy to accidentally miss that you are unsafely changing the type of the data, when no compile time or runtime check is actually being done on it.

edit to add:

DefinitelyTyped also lists this as something you shouldn’t do under their common misstakes

There’s little to no difference between casting from unknown to a type vs using a generic with unknown as the default.

If the type doesn’t matter to you then let it stay as unknown. But the fact remains, there is no way to be absolutely sure what the server returned at compile time, that is why there are only 2 ways out: cast to tell TS you know better and you know you trust the response (which you had to do anyway with any type) or, you can have runtime checks with type guards which TS can understand and help you out at compile time. Unknown type is a better choice for the response than any. I’m just offering people a quick solution to the error

You can’t simply “let it stay as unknown” because that would mean you cannot do anything with the JSON, nor access any of its properties. In the case I described above I only need to access 1 property of the JSON which is then properly typed, but not the JSON itself. I only used node-fetch to send requests to public APIs in my code so yes I can be absolutely sure of what the API is going to return if it’s all properly documented. Either way, setting the type to unknown is just taking away the flexibility we had before and forcing us to do something that we may not want, and even TS itself does not want.

I even tried doing this but I just ended up with tons of useless interfaces that mean absolutely nothing and present no advantage at all

I see your issue as I hate useless interfaces and repetition, but I think you can use a generic type quite efficiently to get around writing “tons of useless interfaces” where you cast your json to this new generic type with the interface you care about as the generic which is the interface without the "data" wrapper.

Happy to provide an example if you share a sample of your code 🙂

I believe someone already showed an example of this generic interface above and I also showed an example of a JSON object I get in a specific part of my code so that’s fine, but wouldn’t this generic type put us back with the same “problem” you described with the any type? I mean if you use this you’re essentially saying any property on your JSON can be anything, which isn’t type safe and is pretty much useless. Any workaround to this change is essentially a waste of time and code, even if it doesn’t make it into the compiled code, it’s still making our lives much much harder for no reason at all

Good point. How about object | string | boolean | number then? While its not as narrow as it can possibly be, it’s more specific than unknown.

Edit: or rather type Json = null | string | number | boolean | Json[] | { [name: string]: Json }

use object

JSON can also return only strings, booleans, numbers also. JSON isn’t always a object

I think it’s pretty safe to use object instead of unknown by default.

You can assign arrays and nulls to an object as well.

This means that projects targeting both the browser and node could have problems…

They’d only have problems if they were actually using the generic (e.g. await response.json<Foo>()), rather than relying on the default value of unknown (via json<T = unknown>(): Promise<T>;). Is that right, or is there another issue I’m overlooking?

That seems like a pretty reasonable and straightforward tradeoff. Especially considering almost all of that real-world code is already broken because of the change from any to unknown. e.g.

const response = await fetch('/foo')
const foo = await response.json()

console.log(foo.bar) // works in dom fetch and node-fetch@2, fails in node-fetch@3

I understand the arguments in favor of unknown and actually mostly agree with them (as the default in a generic type). However that is in direct conflict with the idea of dom fetch compatibility.

The downside mostly comes down to code style and expectations, rather than any technical limitation. <Foo> is what I would expect as a TypeScript user, while as Foo feels like a hack to work around a limitation of the library.

I’ve used axios and got in the past and both support generics for this exact use case.

Like others I came here because I was very surprised at the current behavior.

I don’t think it’s fair to categorize the desire for a generic in this situation as not really caring about type safety.

By its own nature fetch is designed for consuming other APIs. If an API says they will return some JSON in the form of a Foo when you make a GET /foo request, then it’s safe to expect response.json() to return a Promise<Foo>.

I trust that the GET /foo endpoint will return a Foo in the same way I trust calling fetch() will return a Response. There’s no runtime or compile time guarantee that the types shipped with node-fetch are correct, in the same way that there’s no runtime or compile time guarantee that the docs provided by the API I’m hitting are correct. However I trust both the same.

Do you think that typing await response.json<Foo>() is so much more convenient than typing await response.json() as Foo?

@LinusU I don’t have a preference, but that is not the crux of the matter for me. I would prefer await response.json() to have the return type of unknown instead of any. At the end of the day, all this is moot IMO because I’ve learned to be more and more type-safe as I grow as a developer. If anything, this issue and thread will continue to educate onlooking devs that are recently making the switch out of JS or making the switch to stronger tsconfig.json configurations.

node-fetch has an opportunity to build a more type-safe library and naturally educate devs (like myself) through its type system. In the meantime, we can add functions like this to build stronger type systems in our applications.

const getJSONFromResponse = <T = unknown>(response: Response): Promise<T> => response.json();

@ChromeQ that is all fine when the type that you get from the fetch method actually matters to you. Please refer to my message above, we shouldn’t be forced to type cast something we don’t need to use

Some relevant discussions:

Personally, I feel that unknown provides the least amount of danger of shooting oneself in the foot, which I think it most important. But I can see us going with any to match dom.d.ts. Do note that these types was added to dom.d.ts before the unknown type was added to TypeScript, so at the time they didn’t had a choice…

I have multiple instances in my code where I don’t need to make an interface for the JSON I get from the fetch call and only do so later on in a property of that JSON. For example, why would I need to make an interface for an object that looks like this

{
    "data": {
        "foo": "bar"
    }
}

When only the type inside data matters to me? I even tried doing this but I just ended up with tons of useless interfaces that mean absolutely nothing and present no advantage at all. By giving us the any type you’re trusting that we know what we’re doing, instead of saying you have to type this, otherwise we won’t let you go any further

You also mentioned TS didn’t have a choice when the window.fetch types were added but the unknown type has been a thing for quite a while and they’ve had plenty of time to change it. If they didn’t then it’s because they didn’t see a reason to do that, and I don’t think you should have any saying on that when your package just ports something from the browser to node. This is not node-fetch anymore, this is something you made up yourselves.