Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Return type narrowing for service multi CRUD.#1717

Closed
deskoh wants to merge 1 commit intofeathersjs:masterfeathersjs/feathers:masterfrom
deskoh:prCopy head branch name to clipboard
Closed

Return type narrowing for service multi CRUD.#1717
deskoh wants to merge 1 commit intofeathersjs:masterfeathersjs/feathers:masterfrom
deskoh:prCopy head branch name to clipboard

Conversation

@deskoh
Copy link
Contributor

@deskoh deskoh commented Dec 1, 2019

This PR narrow the return type of service multi create, patch and remove methods.

Consider the following database service:

type Message = { text: string }

// Return type: Promise<Message>
service.create({ text: 'msg' });
// Return type: Promise<Message[]>
service.create([{ text: 'msg' }]);

// Return type: Promise<Message>
service.update(1, { text: 'msg' });
// Typing error remains (Database adapter does not support multi update)
service.update(null, { text: 'msg' });

// Return type: Promise<Message>
service.patch(1, { text: 'msg' });
// Return type: Promise<Message[]>
service.patch(null, { text: 'msg' });

@deskoh deskoh closed this Feb 29, 2020
@deskoh deskoh deleted the pr branch February 29, 2020 05:52
@pdfowler
Copy link

@deskoh I found this PR after fighting with this problem myself for a bit. What did you end up doing to resolve this issue for yourself?

@deskoh
Copy link
Contributor Author

deskoh commented Jan 27, 2021

Take a look at this: https://github.com/feathersjs-ecosystem/databases/pull/4
The packages have been moved back in upcoming version:
https://github.com/feathersjs/feathers/blob/dove/packages/adapter-commons/src/service.ts

Seems like the overloads were removed. Any comments @vonagam?

@daffl
Copy link
Member

daffl commented Jan 27, 2021

Good point, they should be added back in that file now. No idea yet if it will all play together nicely, there's a lot of changes in progress at https://github.com/feathersjs/feathers/tree/dove-typescript-refactor.

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.

3 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.