task.complete loses context

[myklemykle]

If I define a Jake task that calls fs.write(), and I try to pass the task's complete() method (this.complete) as the callback, I'll get various exceptions about undefined functions. It seems that Jake's task.complete() method requires its task as context, but doesn't do anything special to ensure that's what it gets.

Of course I can wrap the method in a function, or bind it, to give it the right context. But it'd be great if a Jake task already had its complete() method correctly bound when it was created. It's a very common pattern for JS libraries to take a bare callback as an argument.

Here's the fix I'm using at the moment: monkey-patching jake.createTask so I don't have to bind every single task individually:

// Insert at top of Jakefile:
var _ = require('underscore');  
jake.createTask = _.wrap(jake.createTask, function(createTask){
  var t = createTask.apply(jake, _.rest(arguments));
  t.complete = t.complete.bind(t);
  return t;
});

Would there be any harm in baking something this into jake.createTask?

[mde]

Definitely common for JS libs (especially in the Node world) to take a single callback. OTOH, complete is just an ordinary JavaScript instance method, and pre-binding all instance methods is not the usual process with ordinary JS OOP. In fact, passing a reference to an instance method in a callback (e.g., someFunc(myObj.methodName)) is one of the canonical examples of needing to pre-bind, or pass an optional context object (like the thisArg defined for JS Array.prototype.forEach). I'm really not sure that pre-binding is the correct thing to do here.

[myklemykle]

I havent looket at how task.complete uses its context ... Maybe it just needs to take some closure in its init method, or something? I agree that binding the method to its object feels inelegant. -m-

-- IM IN MY IPHON MISPELIN UR W0RDZ

On Jun 18, 2017, at 2:06 PM, Matthew Eernisse notifications@github.com wrote:

Definitely common for JS libs (especially in the Node world) to take a single callback. OTOH, complete is just an ordinary JavaScript instance method, and pre-binding all instance methods is not the usual process with ordinary JS OOP. In fact, passing a reference to an instance method in a callback (e.g., someFunc(myObj.methodName)) is one of the canonical examples of needing to pre-bind, or pass an optional context object (like the thisArg defined for JS Array.prototype.forEach). I'm really not sure that pre-binding is the correct thing to do here.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

[mde]

Given that this is expected behavior for JS OOP, I'm closing this issue.