Repository navigation
Hooks with a callback #167
Description
I was wondering if the option to support asynchronous functions from within a hook (supposedly by using a done callback passed in as a parameter to indicate continuing on with the actions/hooks) was on this project's timeline? It certainly would be a nice feature to have, since I have one slightly CPU intensive sync method inside a hook that I would prefer to use the async version of.
Activity
dxg commentedon May 19, 2013
Whilst the actual question will have to be answered by dresende, I have a question of my own (and forgive me if I misunderstand):
What's the benefit of having a CPU intensive task in an asynchronous callback?
Async solves the problem of I/O blocking operations, not cpu intensive ones.
jogly commentedon May 19, 2013
Absolutely correct, and as I described my problem it wouldn't solve anything. But I can break my one CPU intensive task into many less intensive smaller tasks as functions to give as much time back to the thread as possible.
dresende commentedon May 24, 2013
But you're saying you need to do it before the Hook finishes? I'm not sure if we can detect if the user will call done or not. Maybe by the number of arguments of the anonymous (or real) function passed to the Hook. Or maybe by forcing people to return
falseor something. What would you suggest to avoid breaking current implementation?jogly commentedon May 24, 2013
I certainly understand the question. Maintaining the current functionality is definitely important. I'm currently battling two notions in my head. First, your idea of working with the return value. This would suit my purposes splendidly, but I would personally be shy suggesting that, solely because that would reduce the flexibility of the hook to future ideas to fit one use case (unless there are other folk who also would benefit?).
The other is a duplicate hook with an 'Async' post-fix (or other identifier) for every synchronous hook. I dislike this idea primarily because I dislike duplication; it can get messy. I've seen many reputable libraries include synchronous versions of asynchronous functions, though. The converse couldn't be too out of the ordinary, and it would maintain the return value's flexibility.
Apologies, this doesn't really answer your question, but they're thoughts and neither implementation should break current functionality.
jogly commentedon May 26, 2013
I don't know how I missed this in your original comment, but after re-reading I noticed your suggestion for number of arguments. By checking the arity (
length) of the function passed as a hook you could tell if the user included, and is expecting to call, thedoneargument. This would be my personal choice as it does not break functionality and the hook remains flexible. Unless, of course, I've misunderstood something fundamental...this is known to happen.dresende commentedon May 27, 2013
Yes, that was my suggestion. The only thing that could break it would be to use any kind of async library that might add arguments to the call. I'm going to try this, I think Function.length is standard and not deprecated.
Adds Hooks.wait to later on use it for async hooks (#167)
dresende commentedon May 28, 2013
Ok, I added this only for
beforeRemovehook. Perhaps only thebefore*should have this async behavior, the rest it doesn't make sense. Take a look attest-hook-before-remove-wait.jstest file.If the hook function does not have the arity of the hook (in this case, none) + the
next()function, it will assume the hook as no objection and so it will continue without waiting. If you want to just do something or check something and there's no problem to continue with execution just callnext(). If you want to abort just call it with anErroras first argument. That error will be passed to theModel.removecallback.dresende commentedon May 28, 2013
I forgot to mention I will add this to more hooks if this implementation looks OK.
jogly commentedon May 28, 2013
I can't think of a use case for the
after*hooks, but I can't presume to deny they exist. My request was specifically for thebefore*hooks anyway :). The test scenario looks like exactly what I needed. Ill pull the commit when the other hooks are in place. Thanks! Faster than I could have hoped for.Changes beforeCreate, beforeSave and beforeValidation to use Hooks.wa…
Updates Readme to explain async before* hooks (#167)
dresende commentedon May 28, 2013
OK, there's information about this in the Readme. If you find anything missing just point it out :)