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: allow _patch to modify the entire base schema #3300

Merged
merged 1 commit into from
Nov 3, 2023

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