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

feat: typed API definition #371

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

anuragkumar19
Copy link

πŸ”— Linked issue

#364

❓ Type of change

  • πŸ“– Documentation (updates to the documentation, readme, or JSdoc annotations)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

This change will allow developers to define their api schema at one place and get type-safe fetch client.

Resolved #364

πŸ“ Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

@anuragkumar19 anuragkumar19 changed the title feat(types): typed api definition feat(types): typed API definition Mar 5, 2024
@pi0 pi0 changed the title feat(types): typed API definition feat: typed API definition Mar 5, 2024
@pi0 pi0 requested a review from danielroe March 6, 2024 15:50
Copy link
Member

@danielroe danielroe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks really great, and thank you so much for your work on this.

I think it makes sense for these types to live in ofetch, and I think the schema makes sense:

[url: string]: {
    [key: 'default' | HTTPMethod]: {
      request: {
        body?: BodyType;
        query?: QueryType
      };
      response: ResponseType;
    };
  };

It might also allow us to do things like define error types in future.

My one note is that I think we should spend some additional time thinking about the params implementation as this will require runtime code changes as well as just types.

Finally, having said that, the tricky thing will be to properly handle type inference when (for example) hitting and endpoint that has multiple methods with multiple valid request types, each mapping to a method.

Writing those types will be complex.

For that reason, I would like to see some type tests bundled with this PR, for example similar to ones we have in Nitro, covering the way these types are used and ensuring we don't cause regressions.

@anuragkumar19
Copy link
Author

I also thought that params can also be inferred from url.

This can be used to extract params from url.

For test I think I should first write test for types (will be another pr) of current version of ofetch, so then when new changes are merged it would be ensured that they are backward compatible. Additional tests will be written for new types.

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 this pull request may close these issues.

Typed API definition
2 participants