parseAs doesn't change the data type

[HugeLetters]

Description

Specifying parseAs as "string" or even "stream" still specifies data as JSON

Proposal

A new generic parameter could be added to coreFetch to infer correct return type.

• Either we modify FetchResponse,FetchOptions and the types it uses accordingly. This would technically be breaking for people who use these types in their codebase. We can instead of adding a third type parameter just modify the second one - something like O extends {method: HttpMethod, parseAs: ParseAs}.
• We could try to minimize the friction for people consuming these types by adding a third type parameter to coreFetch, then adding a second optional type parameter with a default value for FetchOptions and RequestOptions. And then also modify the return type of coreFetch directly instead of modifying FetchResponse type - something like

Promise<parseAs extends "json" ? FetchResponse<M>, { data: ReturnType<Response[parseAs]> }>

First approach seems more "correct" but is breaking in a sense. The second one feels like a band-aid but shouldn't affect users.

Checklist

• I’m willing to open a PR (see CONTRIBUTING.md)

[drwpow]

Thanks for suggesting! I think perhaps a more full code example of the desired end-user usage could help me (and others) understand? Including a snippet of the original OpenAPI schema, as well as highlighting before/after code would clarify the exact change being proposed.

[HugeLetters]

@drwpow
If you do

const reponseText = await api.GET('/path', { parseAs: 'text' });
reponseText.data; // the type is whatever JSON is returned from this route, but in reality the type is string

The same applies for any other parseAs - they all still report JSON type, not string, buffer, blob etc as the should.

[drwpow]

Ah I understand. What does the OpenAPI schema look like? Does it still have JSON typing as well as other media types?

[HugeLetters]

@drwpow I found an example in your tests - you check if data is equal for "{}" even though TS reports data type to be

{
    email: string;
    age?: number | undefined;
    avatar?: string | undefined;
} | undefined

But the test passes.

It's not an edge case with my schema - it's just that coreFetch return type does not depend on provided parseAs whatsoever so of course it always reports JSON type.

[HugeLetters]

@drwpow so would you like for me to try to work on this? If yes which of the two approaches(in the original post) would you prefer?

[drwpow]

There’s an internal change I’d like to make where the runtime code is separated from the type definitions (separate, and manually-managed .js and .d.ts files). We’ve run into several problems where we want certain types to be inferred on the user side, however, we DO NOT want to modify the runtime code. But in .ts files it’s a requirement to modify the code somehow to change type output.

Anyways, all that said, I need to make that change before we tackle this problem, as that will make this much easier. I’d welcome a PR once that change is in!

[drwpow]

Just made that change (#1424) so I’d welcome a PR on this! With the split, you’d only have to make a change in index.d.ts without affecting the runtime.

Overall I don’t have a strong opinion between either option, but 1 seems preferable. This library is still 0.x so breaking changes are more acceptable. But even if it wasn’t, I don’t consider changing an internal TS interface to be a true “breaking change” like a runtime change would be.

[HugeLetters]

Ok, working on it now.