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

fix: allow _patch to modify the entire base schema#3300

Merged
daffl merged 1 commit intofeathersjs:dovefeathersjs/feathers:dovefrom
AshotN:an/_patchTypeFixAshotN/feathers:an/_patchTypeFixCopy head branch name to clipboard
Nov 3, 2023
Merged

fix: allow _patch to modify the entire base schema#3300
daffl merged 1 commit intofeathersjs:dovefeathersjs/feathers:dovefrom
AshotN:an/_patchTypeFixAshotN/feathers:an/_patchTypeFixCopy head branch name to clipboard

Conversation

@AshotN
Copy link
Contributor

@AshotN AshotN commented Oct 5, 2023

Summary

With this given schema

export const userSchema = Type.Object(
  {
    id: Type.Number(),
    username: Type.String(),
    isAdmin: Type.Boolean()
  },
  { $id: 'User', additionalProperties: false }
)

You may want a user to be able modify their own username, but obviously not set isAdmin to true whenever they want.

So you have a patch schema that only permits the username to be modified.

export const userPatchSchema = Type.Pick(userSchema, ['username'], {
  $id: 'UserPatch'
})

But the backend still retains the ability to modify any property by using underscore methods. _patch ignores all hooks and can set isAdmin to true

app.service('user')._patch(user.id, { isAdmin: true })

The issue is that currently Feathers throws a type error if you try to modify anything that isn't part of the userPatchSchema, even if you are using _patch. You get an error similar to this, Argument of type  { isAdmin: boolean; }  is not assignable to parameter of type  { username: string; } 

This is very frustrating to deal with, since the code is perfectly valid.

This PR attempts to address that type issue.

Other Information

I have tried my best in testing this, I ran the test script and all passed. I also tried this against my local feathers app and it seems to work correctly. I spent a decent bit of time on figuring out these whole 12 lines of changes. This was seemingly the simplest way of fixing the types, without breaking a lot of stuff

@daffl daffl merged commit 0f41622 into feathersjs:dove Nov 3, 2023
@daffl
Copy link
Member

daffl commented Nov 3, 2023

Thank you and sorry for the delay! This will go out with the next release 😄

@AshotN AshotN deleted the an/_patchTypeFix branch December 2, 2023 03:35
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.