Skip to content
This repository has been archived by the owner on Sep 26, 2023. It is now read-only.

Proposal documentation to bring back the access another service from inside a hook. #866

Closed
wants to merge 1 commit into from

Conversation

tuxrace
Copy link

@tuxrace tuxrace commented Oct 11, 2017

A common scenario is we need to access another service from inside the hook, I propose to revive a legacy documentation found on this issue feathersjs/feathers#382

A common scenario is we need to access another service from inside the hook, I propose to revive a legacy documentation found on this issue feathersjs/feathers#382
@eddyystop
Copy link
Contributor

eddyystop commented Oct 11, 2017

There's a lot more to it than that.

I also think this type of information is more appropriate in a guide than in the API docs. Please see https://docs.feathersjs.com/guides/step-by-step/basic-feathers/writing-hooks.html#calling-a-service .

And even then

ProTip Always consider params when doing service calls within a hook

frankly needs a section by itself.

@pkreipke
Copy link
Contributor

And even then ProTip Always consider params when doing service calls within a hook frankly needs a section by itself.

I second that! Was going without passing params for weeks, spaghetti code copies of basic auth and workarounds for restrictToOwner() and queryWithCurrentUser were everywhere. Couldn't figure out why it was so hard until I wondered if it was safe to pass params to nested call and found that Pro Tip :-) I'd offer to help but I'm still not confident I have it all down. Soon!

@daffl
Copy link
Member

daffl commented Oct 29, 2017

I'm going to update the documentation to caution against using those authentication hooks. They are causing more issues than they are worth. I'd much rather have people understand that instead of reading and understanding those hook docs and options and working around cases that don't work as you expect you can just write

function(hook) {
  hook.params.query.userId = hook.params.user._id;
}

Have it be fully customizable and no magic around it.

@pkreipke
Copy link
Contributor

@daffl - that's fine but only with respect to the use of restrictToOwner and qWCU. They're not the only confusing parts of the setup for calling nested services that this issue was opened for.

It'd be nice to understand what from params is needed or ignored on nested calls. I just happened to spend all day re-reading the entire docs site looking for confirmation that I could simply pass 'params' from outer to inner service call without modification, and if not, what fields I should assign / merge from the outer to my own inner.

I felt like there were just two places where it was hinted at:

  1. "Calling a service": https://docs.feathersjs.com/guides/step-by-step/basic-feathers/writing-hooks.html#calling-a-service

The paragraph that starts "Its important that context.params is used in the get." is a little understated. It could be its own expanded section, hoisted a little higher since building new functionality from composing services is so valuable.

  1. stashBefore hook source code: https://docs.feathersjs.com/guides/step-by-step/basic-feathers/writing-hooks.html#stashbefore-source

Here the paragraph after the second code block starting "stashBefore does not use context.params in the get call..." is super important and contains some key insights why a) passing the same params block wouldn't be right and b) what to set up in a params of your own (implying also what feathers is going to do for you which should be explicit). Has a nice "Pro Tip" before the end of it too: "ProTip Its not uncommon to indicate what state operations are in by setting flags in params.".

There were other hints throughout examples ( if you had feathers experience ), and the list of readable and writeable params fields was also useful. Perhaps that's where more info about how to use it properly could go (I just scrambled through again but can't locate).

Thanks again for some great work, really enjoying it!

@DaddyWarbucks
Copy link
Member

I am a bit late to the party on this, but I noticed this as I was perusing some other issues. I have found it useful to put all of "my" params in one prop. I have been calling it session but you could call it whatever you want. I actually remove the context.params.user and place it in context.params.session.user. In my apps I have more info in there as well, things that are generally on every call, not necessarily things that are specific to one service call. This makes it easier to "pass params" around because I know everything is in one place instead of having to remember to pass params.user, params.other, param.another, etc.

@daffl
Copy link
Member

daffl commented Sep 4, 2019

I'm going to close this PR since this is now covered in more detail in the new guide.

@daffl daffl closed this Sep 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants