Added new populate, dePopulate, serialze hooks. #62
Conversation
- Docs are in eddyystop/feathers-hooks-populate for the moment. - Old populate hook renamed legacyPopulate. - New populate calls legacyPopulate based on func signature. - @marshall reviewed the README and pushed a PR. - @ekryski "took a quick gander as well and it looks pretty good." - I'll copy the README to GitBook once the code is reviewed. - Perhaps these 3 hooks ought to have their own subpage hooks/common/populate as the docs are not short.
| @@ -391,7 +391,7 @@ export function disable (realm, ...args) { | |||
| * | |||
| * If 'senderId' is an array of keys, then 'sender' will be an array of populated items. | |||
| */ | |||
| export function populate (target, options) { | |||
| export function legacyPopulate (target, options) { | |||
| options = Object.assign({}, options); | |||
There was a problem hiding this comment.
We should probably add a deprecation warning telling them to use the new function signature.
|
|
||
| return populate({ schema, checkPermissions })(hook) | ||
| .then(() => { throw new Error('was not supposed to succeed'); }) | ||
| .catch(() => {}); |
There was a problem hiding this comment.
No need to stop this PR but I usually check that an error exists as well
.catch(error) => expect(error).to.not.equal(undefined))
There was a problem hiding this comment.
Interesting. Info, thxs.
I did. Didn't see anything that stood out.
Agreed.
💥 nice. I'd add a note about adding a deprecation warning guiding people towards the new format. Even just something like "Calling Other than that I think it looks pretty good. I'll have to pull it down and give it a try. |
- legacyPopulate has a deprecate message - throws tests check err is defined
|
I think this is probably good to |
as the docs are not short.