volojs / volo

Create front end projects from templates, add dependencies, and automate the resulting projects
https://volojs.github.io/
Other
1.41k stars 100 forks source link

v.command resolve handler never called #35

Closed mstade closed 12 years ago

mstade commented 12 years ago

I'm trying to run this from a custom volo command:

    v.command("add", template, name)
     .then(function() {
         console.log("this is never reached")
      })

However, the resolve handler is never called. I've tried logging throughout the code in v.js and main.js and see no reason for the then handler not to run (the add command executes fine.) A fail handler is called correctly however.

It did make me think about the semantics of the command though. If I understand the code correctly, the resolve handler should be called once the command has been executed but not necessarily finished? There's a good chance I'm misunderstanding this, and I think it needs clarifying. If I am getting it right, should this be changed so that the resolve handler is only called once the command has finished executing?

mstade commented 12 years ago

@sammyt was kind enough to find the solution for me: I wasn't calling d.resolve() in my volofile.

Thus, no problems at all here, sorry about the confusion!

mstade commented 12 years ago

To be fair though, it's very easy to forget the call d.resolve in a volofile. An alternative pattern here is to -- instead of passing a deferred -- require the handler to return a promise in case it's supposed to be asynchronous. If no promise is returned, assume it's synchronous.

jrburke commented 12 years ago

I can see allowing a promise return, although I still think it is possible for the user to forget to call resolve. I was trying to avoid enforcing volo command creators to adopt promises for their tasks, so I am not comfortable with saying "if no return value it means task complete" path too. I think this is just a hazard of async programming -- if you do not call your callbacks, then things fall down. But it would be good to have some sort of tool that could lint commands, suggest possible problems for cases like these. That seems a bit off in the future though.