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

infer multi, id & paginate of service.options #3012

Open
fratzinger opened this issue Jan 27, 2023 · 4 comments
Open

infer multi, id & paginate of service.options #3012

fratzinger opened this issue Jan 27, 2023 · 4 comments

Comments

@fratzinger
Copy link
Member

Problem:

  • For all services the type of data for create is T or T[], even true for services with multi: false / multi: { create: false }.
  • For all services the type of id for patch & remove is NullableId, even for services with multi: false / multi: { patch: false, remove: false }
  • For all services the awaited return type of find is Result[] | Paginated<Result>, even for services with paginate: false
  • { $select: ['email'] } does not return Pick<Result, 'email'> but the 'full' type Result instead. Even though Pick<Result, 'email'> wouldn't be the correct type either. It has to be Pick<Result, 'email' | Service['options']['id']>

The root of these problems above is, that the type of service.options is not considered in the service methods. The AdapterBase already has a generic Options. We only need to infer the type and fall back to the general types, if it cannot infer.

Other things to consider:

es6 classes are tedious to work with for heavy TS work like inference. We already saw this for feathers-pinia and moved away from model classes. I think we should move away from es6 classes for adapters & services and make a custom useService(...) function. But I guess I need to open another issue/discussion for this.

@mrobst
Copy link
Contributor

mrobst commented Jan 27, 2023

This is interesting - I made a question on discord about the return type of the find method and how to handle it. ( https://discord.com/channels/509848480760725514/1062772689913266216/1062772689913266216 ). If the service return type "understood" the pagination property it would make (my!) code much much neater! There might be a better way to handle it currently but it would be great to have this type inference!

@mrobst
Copy link
Contributor

mrobst commented Feb 6, 2023

I noticed using the v5/Dove client that the return type is correct and changes according to the use of paginate:false. This is great!

@fratzinger
Copy link
Member Author

Yes, that's true, if you do find({ paginate: false }) explicitely. That catches 80% but is not the point of this issue.
When you don't use it explicitely as in find({ query: { ... } }) the returned type is T[] | Paginated<T>.

@AshotN
Copy link
Contributor

AshotN commented Oct 6, 2023

data for create is T or T[]

This is actually the correct type for a before hook, the validation step for multi isn't done until the database adapter. So technically a create before hook may receive array type data.

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

3 participants