-
Notifications
You must be signed in to change notification settings - Fork 365
Promises for operation hooks #449
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
Conversation
Allow the observer functions passed to `ModelBaseClass.observe` to return a promise instead of calling the callback.
LGTM |
Support both promise and callback styles in ModelBaseClass.notifyObserversOf. When there is no callback supplied, the method returns a promise that is resolved (or rejected) with the result.
9538dd5
to
76ebdcb
Compare
…n-hooks Promises for operation hooks
@bajtos are you updating the docs / examples (atleast creating examples) for this? |
@ritch Good catch, I added a task to strongloop/loopback#418 to update docs & examples once we have all pieces in place. |
@bajtos, I see that Using promises says you can use promises with Operation hooks. But I'm missing where the example is. It took me a few days to figure out that you don't need to return next() in the promise chain. Is there any way we could have an example out there so others don't go through the same struggle? Returning next caused the Operation hooks to be called twice. |
@ctcampb3 I am sorry to hear that you lost so much time trying to figure this out :( Perhaps the following example would be a good one to update? BTW I think we have a similar problem in remoting hooks too, see strongloop/strong-remoting#406 (comment) and strongloop/strong-remoting#406 (comment). It may be possible to improve our code to detect the situation where |
@bajtos , I think those issues look pretty similar. For the example I thought it might be good to have it on the Using promises page under the LoopBack support for promises section. Having an example with a warning here would make it more clear. When returning a Promise instead of a callback the callback function should not be called at all. For example:
|
@ctcampb3 makes sense. Would you mind contributing this doc improvement yourself? See this page to get you started: http://loopback.io/doc/en/contrib/doc-contrib.html Please mention my handle in the pull request to make sure I notice it. |
ModelBaseClass.observe
to return a promise instead of calling the callback.ModelBaseClass.notifyObserversOf
.Related: strongloop/strong-remoting#179 and #447
Connect strongloop/loopback#418
/to @raymondfeng Please review.
/cc @partap This patch provides the infrastructure needed for promise support, it's based on my comments in your PR. My plan is to land this PR first, so that you can use this infrastructure in your PR.