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

Path params #2

Closed
dodas opened this issue May 27, 2022 · 6 comments
Closed

Path params #2

dodas opened this issue May 27, 2022 · 6 comments
Labels
enhancement New feature or request idea

Comments

@dodas
Copy link
Collaborator

dodas commented May 27, 2022

Hey @jlalmes, this is a great little library!

I am exploring possibilities and as far as I can understand, there is currently no way to define a path parameter.
Consider the petstore swagger example, the uploadFile has petId parameter, which is part of the path.

Could we try to extract the path params by looking for curly braces in meta.openapi.path, and then find param with the same name in input and mark that param as from: "path" instead of from: "query"?

WIP attempt to implement the above approach: v0.1.0...dodas:feat/path-parameters

Have you thought about supporting this use case?
I am happy to discuss this and potentially take a stab at implementing it.

Thanks!

@jlalmes
Copy link
Owner

jlalmes commented May 29, 2022

I did initially consider adding path parameter support when creating this library, but I decided to leave it out as an unsupported feature due to tRPC having no similar concept and the DX getting somewhat complicated.

I'm open for adding support if there is a use-case though, could you explain your requirement for GET /user/some-user-id instead of GET /user?id=some-user-id?

@dodas
Copy link
Collaborator Author

dodas commented May 30, 2022

I think using path parameters for identifying an item within collection is a common (best) practice.
See from the docs:

They (path parameters) are typically used to point to a specific resource within a collection, such as a user identified by ID

We currently already have a couple of rest endpoints with path parameters and we are now trying to migrate them to trpc-openapi without making breaking changes to our public API.

What do you think about my proposed solution? It seems the model remains quite straightforward, where input object still remains the truth of source for all your parameters, but then those ones that are "mentioned" in path would be considered path parameters instead of query params.

@jlalmes
Copy link
Owner

jlalmes commented May 31, 2022

We currently already have a couple of rest endpoints with path parameters and we are not trying them to migrate to trpc-openapi without making breaking changes to our public API.

Ok, let's support this!

Breaking change

Input for queries are restricted to ZodObject({ [string]: ZodString }) schema, however input for mutations are currently unrestricted as any ZodType. The proposed solution introduces a breaking change requiring mutation input to follow a ZodObject({ [string]: ZodType }) schema.

I don't think isn't a huge issue (currently in alpha) but just wanted to raise.

Usage

trpc.router()
  .query('getUserById', {
    meta: { openapi: { enabled: true, method: 'GET', path: '/user/{userId}' } },
    input: z.object({ // validated as ZodObject({ [string]: ZodString })
      userId: z.string(), // `userId` is required in schema because it is used in path
      teamId: z.string(),
    }),
    output: ...,
    resolve: ...,
  })
  .mutation('updateUserById', {
    meta: { openapi: { enabled: true, method: 'PUT', path: '/user/{userId}' } },
    input: z.object({ // validated as ZodObject({ [string]: ZodType })
      userId: z.string(), // `userId` is required in schema & validated as ZodString
      details: z.object({
        name: z.string(),
      }),
    }),
    output: ...,
    resolve: ...,
  })

Obviously paths with multiple parameters should be supported too e.g./team/{teamId}/user/{userId}

GET /user?userId=user_123
> 404 NOT_FOUND

GET /user/user_123
> 400 BAD_REQUEST

GET /user/user_123?teamId=team_123
> { userId: 'user_123', teamId: 'team_123' }

GET /user/user_123?teamId=team_123&userId=user_456 // do not overwrite
> { userId: 'user_123', teamId: 'team_123' }
POST /user {"userId":"user_123"}
> 404 NOT_FOUND

POST /user/user_123
> 400 BAD_REQUEST

POST /user/user_123 {"teamId":"team_123"}
> { userId: 'user_123', teamId: 'team_123' }

POST /user/user_123 {"teamId":"team_123","userId":"user_345"} // do not overwrite
> { userId: 'user_123', teamId: 'team_123' }

A quick scan of your PR looks great, I will review properly and add to docs + some tests tomorrow.

Thanks @dodas!

@jlalmes jlalmes added enhancement New feature or request and removed discussion labels May 31, 2022
@dodas
Copy link
Collaborator Author

dodas commented Jun 1, 2022

Great! Happy to hear this.
Let me know if there is anything I can do to help with this.

@jlalmes
Copy link
Owner

jlalmes commented Jun 6, 2022

Hi @dodas, support for path parameters has just shipped, thanks for your help! 🚀

Please can you upgrade to trpc-openapi@0.1.0-alpha.20 & let me know if you find any issues. Keen to release v0.1.0 soon - so feedback is appreciated.

@jlalmes jlalmes closed this as completed Jun 6, 2022
@dodas
Copy link
Collaborator Author

dodas commented Jun 7, 2022

Perfect, thanks!!
From testing so far, everything seem to work as expected 💯

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request idea
Projects
None yet
Development

No branches or pull requests

2 participants