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

fix(client): Add underscored methods to clients #3176

Merged
merged 9 commits into from
Jul 28, 2023

Conversation

DaddyWarbucks
Copy link
Member

@DaddyWarbucks DaddyWarbucks commented Apr 23, 2023

This PR adds the common _get, _find, _create, _update, _patch, and _remove methods to both the Rest and Socket client services. These methods are common for all Feathers database adapters (and many other feathers libraries) and offer a convenient way to call service methods without hooks. But, this convention has never been available to the default client services. These methods will be helpful on the client in retries, caching, batching and any number of other reasons a user may want to call the underlying http/socket request without running client hooks.

This PR also follows this convention for the user's custom methods. Note that it does not attempt to call a server's version of this underscored method (if it even existed). It is only a convention on the client side that would allow the client to call the underscored method without running client hooks, even if they did not implement this convention on the server.

  • I am terrible at TS. This was a very easy update to make to both the code and the types. But I am not sure if I would need to handle types anywhere else? All of the tests are passing.

  • I also updated the body var name to data in the REST service. It felt weird and inconsistent that it was called body in those methods. And I updated the REST update method to use Id instead of NullableId. And I updated the Socket service to use D for data instead of any. Like I said, I am not great at TS and hope this doesn't break anything, but it felt like a nice consolidation and cleanup.

@daffl
Copy link
Member

daffl commented Jul 17, 2023

This makes sense. The tests should pass when rebasing with latest.

@DaddyWarbucks
Copy link
Member Author

@daffl Should I update this line to throw an error for null as well? And I would like to update the socket client to do the same.

_update(id: Id, data: D, params?: P) {

I am thinking something like

if (typeof id === 'undefined' || id === null) {
  return Promise.reject(new Error("id for 'update' can not be undefined or null"))
}

@daffl
Copy link
Member

daffl commented Jul 20, 2023

The id for update can be null. It's just the database adapters that don't support it.

I also forgot to mention that we should probably do this for the Socket.io client as well.

@DaddyWarbucks
Copy link
Member Author

All updated @daffl. Let me know if you need anything else.

@daffl daffl changed the title Add underscored methods to clients fix(client): Add underscored methods to clients Jul 28, 2023
@daffl daffl merged commit f3c01f7 into feathersjs:dove Jul 28, 2023
2 checks passed
@daffl
Copy link
Member

daffl commented Jul 28, 2023

Great, thank you! Will go out with the next release.

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.

2 participants