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

REST api bad pratices #10

Open
roiLeo opened this issue Mar 6, 2023 · 8 comments
Open

REST api bad pratices #10

roiLeo opened this issue Mar 6, 2023 · 8 comments

Comments

@roiLeo
Copy link
Contributor

roiLeo commented Mar 6, 2023

Please:

  • use nouns instead of action
  • use kebab-case for urls
  • plural name for ressources
  • /resources/:resourceId/subresources => /collections/:id/items
@vikiival
Copy link
Member

vikiival commented Mar 6, 2023

Hey, thanks for you suggestion!
I have a couple of Qs

use kebab-case for urls

Will do!

plural name for ressources

so rename itemList to items ? My brain process was avoiding s in url because people can make typos easily.

use nouns instead of action

ELI5 pls? 🥺

@roiLeo
Copy link
Contributor Author

roiLeo commented Mar 6, 2023

so rename itemList to items ? My brain process was avoiding s in url because people can make typos easily.

  • Models are represented in singular
  • Ressource are always in plurals

use nouns instead of action

ELI5 pls? 🥺

Wow, I wouldn't hand over the task of writing an Api to a 5 year old 😅

I was referring to the REST part that should be Stateless:

  • collection/:id => /collections/:id
  • collectionByIssuer/:issuer => /collections?issuer=:issuer
  • collectionByOwner/:owner => /collections?owner=:owner
  • eventByAddress/:address => /events?address=:address
  • eventByInteraction/:interaction => /events?interaction=:interaction or => /items/:id/events?interaction=:interaction
  • eventByNftId/:id => /items/:id/events
  • item/:id => /items/:id
  • itemByCollection/:id => /collections/:id/items
  • itemByCollectionList/:ids => ?
  • itemByIssuer/:issuer => /items?issuer=:issuer
  • itemByCid/:id => /items?cid=:cid
  • itemByOwner/:owner => /items?owner=:owner
  • itemCollectedBy/:address => /items?owner=:address
  • itemSoldBy/:address => ?

it should be human readable & easy to extend.

Bonus:
It would be top if you could create a Swagger doc for this api specs

@vikiival
Copy link
Member

vikiival commented Mar 6, 2023

Wow!
Now I understand that!

I have just a couple of problems:

  1. I currently cant parse queries like /collections/:id/items
  2. when Is it as query param I can't validate if fields are correct
  3. I need to rethink how the REST magic is built (now it's build on the top of client query.

Other option I have in mind it to fork

https://github.com/unjs/ungh

and write proper api 😅

@roiLeo
Copy link
Contributor Author

roiLeo commented Mar 8, 2023

I have just a couple of problems:

  1. I currently cant parse queries like /collections/:id/items
  2. when Is it as query param I can't validate if fields are correct
  3. I need to rethink how the REST magic is built (now it's build on the top of client query.

I would like to help you but I don't have enough knowledge on TS backend (only basic express experience)
Like how do you handle different endpoints query? are you wrapping gql with a rest api?

Other option I have in mind it to fork
https://github.com/unjs/ungh
and write proper api 😅

Code looks simpler & maintenable. I agree with that.

@vikiival
Copy link
Member

vikiival commented Mar 8, 2023

Like how do you handle different endpoints query? are you wrapping gql with a rest api?

basically what rest this version does it that expect path like /{chain}/{call}/{id}

Then I check the call against the map I have defined (let's assign the result to fn)
Last thing I do is make const query = fn(id)
Find endpoint based on the chain and wrap it together

Code looks simpler & maintenable. I agree with that.

anime-approve

@vikiival
Copy link
Member

Decided to deprecate this one for the sake of #29

@roiLeo
Copy link
Contributor Author

roiLeo commented Oct 26, 2023

Decided to deprecate this one for the sake of #29

wdym? new api version? new routes? nitro stack?

@vikiival
Copy link
Member

wdym? new api version? new routes? nitro stack?

Like plan to remove the ask function for uniquery and would rather develop hosted version (nitro/hono) based on the client implementation.

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

No branches or pull requests

2 participants