8000 RFC: add support for Service Request and Response type by bwgjoseph · Pull Request #2125 · feathersjs/feathers · GitHub
[go: up one dir, main page]
Skip to content

RFC: add support for Service Request and Response type#2125

Closed
bwgjoseph wants to merge 1 commit intofeathersjs:crowfrom
bwgjoseph:feat/request-response
Closed

RFC: add support for Service Request and Response type#2125
bwgjoseph wants to merge 1 commit intofeathersjs:crowfrom
bwgjoseph:feat/request-response

Conversation

@bwgjoseph
Copy link
Contributor
@bwgjoseph bwgjoseph commented Nov 13, 2020

Hi,

This PR would likely be a breaking change, so I am actually hoping to make it into dove instead of crow. I have most of the description written in this-repo that I created to showcase, but I will put some of key info here.

For the longest time, I have been wanting to have some ways to allow me to define a different set of request/response interface since it can be different (and it is for most cases that I have encounter). There have been some chats about this in slack as well, and I thought I can start making some changes and see if this can be accepted, and I'm sure can be further improved as well.

https://feathersjs.slack.com/archives/C52QSFK0A/p1594138177347400
https://feathersjs.slack.com/archives/C3M4SBWT0/p1600014049010600

Objective

Allow to declare a different set of interface for Request and Response which can also greatly improve dev-experience.

Example

// post.interface.ts
interface Base {
    readonly _id: string;
    createdAt: Date;
    updatedAt: Date;
}

/ Client facing interface
interface PostRequest {
    title: string;
    body: string;
    comments?: string[];
}

// Base fields generated at server side
interface ServerPostRequest extends PostRequest, Omit<Base, '_id'> {}

interface PostResponse extends Omit<PostRequest, 'comments'>, Base {}

// post.service.ts
declare module '../../declarations' {
  interface ServiceTypes {
    'post': Service<PostRequest, PostResponse> & Post;
  }
}

// if you choose not to have different request/response interface, you can still do this
// the response type will still be PostInterface
declare module '../../declarations' {
  interface ServiceTypes {
    'post': Service<PostInterface> & Post;
  }
}

Result

infer-create-request-1
infer-create-request-2
infer-create-response-1
infer-create-response-2

For more details, and example, please refer to feathers-request-response-demo

If there is a better way to do this, I would like to have your feedback. This should not affect any actual functionality of the application, it all just providing better typing's for typescript developers.

Thanks!

@bwgjoseph
Copy link
Contributor Author
bwgjoseph commented Dec 18, 2020

@daffl would this be considered for v5 seeing that the first pre-release is out. Thanks

@daffl daffl mentioned this pull request Mar 18, 2021
Merged
@daffl
Copy link
Member
daffl commented Mar 19, 2021

This is a good suggestion. Since there already was a major refactoring in the works (PR via #2255) I included it there so we'll close this PR. Thank you for putting this up!

@daffl daffl closed this in #2255 Mar 19, 2021
@bwgjoseph
Copy link
Contributor Author
bwgjoseph commented Mar 20, 2021

Thanks @daffl!

Will try it out after you have release a alpha/beta for it. The only thing that I noticed (now, with a quick look) is that you had the request at the 2nd arg, and response at the first. ServiceInterface<Response, Request = Response>. I guess it make sense if majority of the usage is without a different request, then there is no need to define the 2nd arg. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@bwgjoseph @daffl
0