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

feat(adapter): Add patch data type to adapters and refactor AdapterBase usage#2906

Merged
daffl merged 6 commits intodovefeathersjs/feathers:dovefrom
adapter-patchfeathersjs/feathers:adapter-patchCopy head branch name to clipboard
Dec 6, 2022
Merged

feat(adapter): Add patch data type to adapters and refactor AdapterBase usage#2906
daffl merged 6 commits intodovefeathersjs/feathers:dovefrom
adapter-patchfeathersjs/feathers:adapter-patchCopy head branch name to clipboard

Conversation

@daffl
Copy link
Member

@daffl daffl commented Dec 4, 2022

This pull request adds the ability to use a PatchData type in the adapters which may be different than the usual data type.

It also refactors the AdapterBase to remove the confusing and convoluted $ prefixed service methods. Instead an adapter just implements the underscored service methods as before. While for the top level usage backwards compatible this may be a breaking change in some hopefully still uncommon instances but it reduces a lot of the complexity when implementing a database adapter.

You can still extend directly from the adapter class to implement the actual service methods with your own signatures and return types. This will circumvent the legacy query sanitisation which is now done with query schemas anyway.

@netlify
Copy link

netlify bot commented Dec 4, 2022

Deploy Preview for feathers-dove ready!

Name Link
🔨 Latest commit cbcad81
🔍 Latest deploy log https://app.netlify.com/sites/feathers-dove/deploys/638e37913d540f000cc0f2cc
😎 Deploy Preview https://deploy-preview-2906--feathers-dove.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

packages/adapter-commons/test/service.test.ts Outdated Show resolved Hide resolved
async create(data: D[], params?: P): Promise<T[]>
async create(data: D | D[], params?: P): Promise<T | T[]> {
if (Array.isArray(data) && !this.allowsMulti('create', params)) {
throw new MethodNotAllowed('Can not create multiple entries')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Can Not" --> "Cannot"

Copy link
Member

@marshallswain marshallswain left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Will we make sanitizeQuery conditionally run in another PR, or did I miss it in this one?

@daffl
Copy link
Member Author

daffl commented Dec 4, 2022

@daffl daffl marked this pull request as ready for review December 4, 2022 21:47
Copy link
Member

@marshallswain marshallswain left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New updates look good.

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

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