Skip to content
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

Typed API definition #364

Open
1 task done
anuragkumar19 opened this issue Feb 23, 2024 · 12 comments · May be fixed by #371
Open
1 task done

Typed API definition #364

anuragkumar19 opened this issue Feb 23, 2024 · 12 comments · May be fixed by #371

Comments

@anuragkumar19
Copy link

Describe the feature

Hi! I have been using nuxt and ofetch for a while. The ofetch package(independently) lacks the types safety I get when working with ofetch with nitro. I have to specify types for response on every request manually. So, I looked up in the repo and found out that types inference based on schema was a part of nitro not ofetch. I think those types should belong here in ofetch repo so, if we work independently with ofetch we can get type safety.

I open a dicussion in Nitro repo, here nitrojs/nitro#2157

I have been working on it for past couple of days. It is almost done. There are few thing to fix and discuss.

While I was implementing it. I thought it is also time to change the structure of InternalApi so that it will now support more types like body/query/params.

Here is now schema I come up with.

interface ApiDefinition {
  "/api/v1": {
    default: {
      response: { message: string };
    };
  };
  "/api/v1/auth/register": {
    post: {
      response: { message: string };
      request: {
        body: {
          name: string;
          email: string;
          username: string;
          password: string;
        };
      };
    };
  };
  "/api/v1/users/search": {
    get: {
      response: { users: { username: string }[] };
      request: {
        query: {
          username: string;
        };
      };
    };
  };
  "/api/users/:username": {
    get: {
      response: { user: { username: string; name: string; isFriend: boolean } };
      request: {
        params: {
          username: string;
        };
      };
    };
    post: {
      response: { message: string };
      request: {
        params: {
          username: string;
        };
      };
    };
  };
}

You can check out my version of implementation in
https://github.com/anuragkumar19/ofetch

There are some examples embedded in the types.ts for testing while development which will be removed in final version.

I have also examples in playground/index.ts for convention.

I think it will be totally backward compatible but we will discuss. Also compatible with nitro because it fully override $Fetch, also after this change we can fully drop types/fetch.ts in nitro.

This will solve many issues opened in nitro and nuxt repo for type-safety with request object.

I will open a PR soon. I need to clean few this up first.

Additional information

  • Would you be willing to help implement this feature?
@anuragkumar19
Copy link
Author

anuragkumar19 commented Feb 23, 2024

Related issues & PR:
nitrojs/nitro#1532
nuxt/nuxt#23009

@anuragkumar19
Copy link
Author

anuragkumar19 commented Feb 24, 2024

Things I want to discuss?

  • Should we force user to pass a method when Methods union doesn't include "get"?
  • Return response type for "get" method if no option is passed by user instead of union of all possible response

Note: The above two bugs are not specific to my version. It is also present in Nitro's current implementation

  • Weather we should force user to include a body/query/params if their type definition is present?

@anuragkumar19
Copy link
Author

anuragkumar19 commented Feb 26, 2024

Prove of concept of types working with nitro https://github.com/anuragkumar19/nitro/tree/types-request-poc. It is in the playground.

Note: It is just a prove of concept, I rushed it to show how future API may look like. There are many edge cases I ignored and tests are failing.

@enkot
Copy link

enkot commented Mar 4, 2024

ofetch is a general-purpose fetching library that has nothing to do with Nitro, and InternalApi is something ofetch doesn't need to know about. This is why we need to specify the types for the response, since ofetch can't and should not know about the backend implementation out of the box.

The scheme described in this issue is quite opinionated (e.g. OpenAPI, JsonAPI etc.) and the types should be added for each use case, same way Nitro does this for its internal API.

@pi0 pi0 changed the title Moving $Fetch (with type safety inference) from nitro to ofetch Typed API definition Mar 4, 2024
@pi0
Copy link
Member

pi0 commented Mar 4, 2024

I like the idea. Without adding overhead to bundle size we can make a type safe fetch API also without depending on backend 👍 Nitro might use it or not in the future..

I guess mainly we should finalize the type interface. I like the current one as well generally wdyt @danielroe ?

@pi0
Copy link
Member

pi0 commented Mar 4, 2024

@anuragkumar19 Feel free to draft a PR btw!

@anuragkumar19
Copy link
Author

@anuragkumar19 Feel free to draft a PR btw!

I will do it.

@wadeV12
Copy link

wadeV12 commented Mar 4, 2024

@anuragkumar19

"/api/users/:username": {
    get: {
      response: { user: { username: string; name: string; isFriend: boolean } };
      request: {
        params: {
          username: string;
        };
      };
    };
    post: {
      response: { message: string };
      request: {
        params: {
          username: string;
        };
      };
    };
  };

could we use it like this?

"/api/users/:username": {
   default: {
      request: {
       params: {
         username: string;
       };
     }
   },
   get: {
     response: { user: { username: string; name: string; isFriend: boolean } };
   };
   post: {
     response: { message: string };
   };
 };

As for me, there is no reason to repeat it for REST API.

request: {
      params: {
        username: string;
      };
    }

Do you think it's useful to have it as an object type and provide a type for every single property? We could safely pass to URL just string or number so for me there is no big reason to type string again and again for every URL param.

@anuragkumar19
Copy link
Author

@wadeV12 I think it will work in current implementation. I will cross check it anyway.

@enkot
Copy link

enkot commented Mar 4, 2024

@anuragkumar19 What do you think about adding the error field to the schema, so we can also benefit from 100% typed response if this feature is possibly implemented?
Example:

"/api/users/:username": {
  get: {
    response: {
      data: {
        user: { username: string; name: string; isFriend: boolean }
      },
      error: { status: string, code: string }
    };
    request: {
      params: {
        username: string;
      };
    };
  };
}

Maybe more correct:

"/api/users/:username": {
  get: {
    response: {
      200: {
        user: { username: string; name: string; isFriend: boolean }
      },
      403: { status: number, code: string },
      404: { status: number, message: string },
    };
    request: {
      params: {
        username: string;
      };
    };
  };
}

In this case the type of the error will be:

{
  status: number;
  code: string;
} | {
  status: number;
  message: string;
} | undefined

@anuragkumar19 anuragkumar19 linked a pull request Mar 5, 2024 that will close this issue
8 tasks
@johannschopplich
Copy link
Contributor

@anuragkumar19 This is such a great idea! Actually, the project unjs/api-party tries to solve this: Unifying multiple ways of writing API interactions, which are all based on ofetch:

Example: ofetch adapter:

import { createClient, ofetch } from "api-party";

const baseURL = "<your-api-base-url>";
const adapter = ofetch();
const api = createClient({ baseURL }).with(adapter);

// GET request to <baseURL>/users/1
await api("users/1", { method: "GET" });

Example: OpenAPI adapter:

import { OpenAPI, createClient } from "api-party";

const baseURL = "https://petstore3.swagger.io/api/v3";
// Pass pre-generated schema type ID to adapter
const adapter = OpenAPI<"petStore">();
const api = createClient({ baseURL }).with(adapter);

// Typed parameters and response
const response = await api("user/{username}", {
  method: "GET",
  pathParams: { username: "user1" },
});

As of right now, it even supports OpenAI type generation for usage with ofetch. Maybe we can release the current version, @pi0? I think keeping the logic separate from ofetch could be benficial.

@astahmer
Copy link

astahmer commented Apr 5, 2024

I've searched a bit for an existing solution and couldnt find one, then stumbled upon this issue
it could indeed be very convient to have something working out of the box with ofetch (agree that it's best to keep it out of this repo anyway)

fwiw I tried using my own tool typed-openapi and since ofetch has the same API as fetch, it just worked, so I figured I'd share it
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants