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:
- use node-fetch in a typescript project
- try to build
- 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)
I don’t mind defaulting to
unknowninstead ofany, but can we have something like thisjson<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. 😉
Unless TypeScript provides a native json type (https://github.com/microsoft/TypeScript/issues/1897), I think that
unknownis absolutely preferred overany.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”:
edit to add:
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:
Since
anyis pretty much useless for type safety so I think switching tounknownis the right one. The simple solution is to simply cast to your wanted type like this: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)I prefer to be as strict as possible and avoid using
any. Instead I useunknownornever. User defined type guards are how I:response.json()As @martijndeh has indicated, if you don’t want a runtime check, the
response.json<MyType>is the way to go.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
unknownprovides the least amount of danger of shooting oneself in the foot, which I think it most important. But I can see us going withanyto matchdom.d.ts. Do note that these types was added todom.d.tsbefore theunknowntype 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
assertskeyword 🤷 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
runtypesto 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 withArray<unknown> | Record<string | number, unknown>as that is closer to what JSON is allowed to be. Ultimately, this comes down to theJSON.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. Usingunknowninstead ofanydo only one thing: it forces to give some imaginary type. Alljson() as MyDataorjson<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 forjson<T = unknown>(): Promise<T>just because it is more ergonomic and more expected. But I was even fine withanyfor 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
I’ve corrected my comment. I meant the combination of OpenAPI definition, ajv and assertion function. I use this combination in all places:
For example:
@LinusU
I see two points:
as(4 chars) instead of<>(2 chars) 😃json<MyData>()before I was surprised by the definition.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.
There’s little to no difference between casting from unknown to a type vs using a generic with unknown as the default.
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 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
We can probably just type out a normal JSON object (see https://github.com/sindresorhus/type-fest/blob/2783a0890925bc180a406d0e34290578812b4864/source/basic.d.ts#L15-L45)
Good point. How about
object | string | boolean | numberthen? While its not as narrow as it can possibly be, it’s more specific thanunknown.Edit: or rather
type Json = null | string | number | boolean | Json[] | { [name: string]: Json }JSON can also return only strings, booleans, numbers also. JSON isn’t always a object
I think it’s pretty safe to use
objectinstead ofunknownby default.You can assign arrays
and nullsto anobjectas well.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 ofunknown(viajson<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
anytounknown. e.g.I understand the arguments in favor of
unknownand 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, whileas Foofeels 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
fetchis designed for consuming other APIs. If an API says they will return some JSON in the form of aFoowhen you make aGET /foorequest, then it’s safe to expectresponse.json()to return aPromise<Foo>.I trust that the
GET /fooendpoint will return aFooin the same way I trust callingfetch()will return aResponse. 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.@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 ofunknowninstead ofany. 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 strongertsconfig.jsonconfigurations.node-fetchhas 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.@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
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
When only the type inside
datamatters 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 furtherYou 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.