Skip to content

Conversation

@fratzinger
Copy link
Contributor

This PR addresses the params property, which was pretty useless. Basically there were 3 problems:

  1. The Object.assign for params shallow merges the query so the query becomes overwritten -> only params without the query-property make sense.
  2. Who needs static properties in params outside of query?
  3. You cannot pass authentication to the services for custom restrictions

The 1. point is also mentioned in #15. Marshall came up with the idea of an function to manipulate the params after feathers-shallow-populate. I achieved the solution with two approaches:

  1. use lodash/merge for the passed params object to merge deeply.
  2. pass a function with params: (params) => {}. The function can manipulate the passed params or return a new params object which gets merged deeply.

For the 3. point it made sense for me to also pass the context down to the params object: params: (params, context) => {}. This way the context.params.authentication can be passed to params.authentication or context.params.user to params.user

This feature is totally optional. I wrote tests for the various approaches. The deep merge instead of the shallow Object.assign changes the behaviour, I guess, but makes it better.

I also documented the possible values in the README.md. This can also be copied to feathers-graph-populate which addresses marshallswain/feathers-graph-populate#12

@marshallswain marshallswain merged commit 4c89570 into Mattchewone:master Sep 30, 2020
@marshallswain
Copy link
Collaborator

This is a beautiful thing! Thanks!

@marshallswain
Copy link
Collaborator

Released as [email protected]

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