Repository navigation
Consider adding queue.deferWith. #7
Description
Activity
Hmm, I’m not sure I see the value in using the queue as the context. Queues are generally transient objects; you create them to run a number of tasks in parallel, and then they disappear. I would think that using the queue as the context would encourage manipulation of the queue from the tasks themselves, which seems error prone. Although if you can come up with a convincing example of where this would be useful, I might change my mind.
Ideally, an API like
queue.defer(method, object)would work, similar to array.map. But of course this doesn’t work because the object is interpreted as an argument to the method, rather than the context. You could perhaps interpret based on the method’s length but this would not be reliable for functions that inspect theargumentsarray rather than declare a fixed number of arguments. You could use the argument orderqueue.defer(object, method), and inspect whether the first argument is a function; but this too would fail if the object was also a function (which is perfectly valid, as functions are first-class objects in JavaScript; see D3’s scales for examples).That leaves a few explicit options, such as queue.deferUsing (perhaps queue.deferWith?).
queue() .deferWith(object, object.method1) .deferWith(object, object.method2) .await(done);
You might instead set a context on the queue that is used for callbacks:
queue() .context(object) .defer(object.method1) .defer(object.method2) .await(done);
But then you’d want to capture the context’s value as tasks are deferred, so that the results are deterministic when you defer multiple asynchronous tasks with different contexts:
queue() .context(object1) .defer(object1.method1) .context(null) .defer(global2) .await(done);
But again we come back to the principle of parsimony, and I think it’s cleaner here to use function.bind:
queue() .defer(object.method1.bind(object)) .defer(object.method2.bind(object)) .await(done); queue() .defer(object.method1.bind(object)) .defer(global2) .await(done);
Or a closure:
queue() .defer(function(callback) { object.method1(callback); }) .defer(function(callback) { object.method2(callback); }) .await(done); queue() .defer(function(callback) { object.method1(callback); }) .defer(global2) .await(done);
Yeah, as stated in the OP I really don't think
deferWithwould be worth adding —Function.prototype.bindis the way forward and the API pattern that did allow conveniently passingthisthrough seems to be vestigial (e.g. node.jsemitter.ondid not "fix" theelement.addEventListener's missing context parameter…ES5 simultaneously set precedent for it with.mapand friends, while also removing the need via.bind).However, please consider providing a slightly-more-likely-to-be-useful
thisto the existing callbacks: the queue. The use case is indeed to encourage "manipulation of the queue" from tasks — specifically, so tasks can enqueue additional tasks. So I write a "download file" task and it adds eight "download range of file" tasks, which may each add "themselves" back to retry once or twice before giving up.I'm all for encouraging
.bindfor any other use, just don't see any reason to not provide a sensible default in its absence.To clarify: any
Function.prototype.bindwould automatically take precedence over the context provided.function logCtx() { console.log(this.valueOf()); } logCtx.bind("thing2").apply("thing1");…logs the bound "thing2", not the caller's "thing1".
The problem with encouraging the tasks to manipulate the queue is that there’s a pretty strict way in which a queue should you used: you defer a set of tasks, and then you specify an await callback. Generally the definition of a queue is synchronous; only the task evaluation is asynchronous.
If you’re deferring tasks within a task, then you’re probably deferring tasks after an await callback has been set, which is illegal. (I unfortunately lack the authority to arrest you, but at any rate the behavior of the queue will be undefined, as described in the API reference. Perhaps in the future this will throw an error.)
Could you accomplish the same behavior by using multiple (nested) queues instead?
Maybe I should just plead the fifth on this one, but…why block support for adding tasks later?! Is there some upcoming feature that would interfere with?
Clearly (at least the last time I read through the code, which admittedly was before the conversion from a linked list to an array) adding additional items to the queue was sure to behave as I would expect, except for issue #9.
If you add tasks after setting the await callback the behavior becomes nondeterministic; it depends on whether the previously-deferred tasks have or not have yet completed. If all previously-deferred tasks are completed prior to adding additional tasks, then your await callback will have already been invoked, and setting additional tasks will cause one or more additional await callbacks. This was true before and is not affected by the rewrite.
You could carefully protect against this by ensuring that at least one task in the queue remains unfinished, but that seems brittle. And if Queue.js were to enforce this brittle behavior, failure would be nondeterministic, too; I’d rather reserve the option for deterministic failure by disallowing the ability to defer tasks after the await callback is set. (Queue doesn’t currently enforce this, but it might be nice to avoid nondeterminism.)
Right, but an easy and reliable way to ensure this is if a task adds another task(s) to the queue before it finishes itself. Hence my original "Apply queue as context to deferred function call" request.
Right, that would be safe, provided the callback is invoked after any deferred tasks are added to the queue. :) OK. That would require more nuanced language describe what usage patterns are valid… Something like, tasks can only be deferred after the await callback is set provided you can guarantee that at least one previously-deferred tasks has not yet completed; in practice, one of the few safe places where this usage pattern is valid is within another previously-deferred task, prior to invoking the earlier task’s callback.
Still, that seems, well, complicated. I’m not sure allowing (or encouraging) this behavior is cleaner than creating a meta-task that uses a nested queue. Maybe you could expound a bit on your real-world example to help convince me.
I think the easiest way to document it is in terms of the state transition: the await/awaitAll callback is called every time the number of remaining tasks goes to 0, i.e. whenever the last task finishes.
The user can choose to deal with this three ways:
- Don't add tasks except as part of initial queue setup
- Don't let the queue drain before adding tasks
- (weird, but…) Set
await(null)when no longer wanting to handle an empty queue
I'm realizing part of why these requests might seem weird: in such use cases I'm often ignoring the results (only watching for error) in my "done" callback. So when I talk about tasks adding subtasks and whatnot, I'm either enqueueing calls for their side effects, or side-channel information passing like in my batch processing example. Note how in that example, the passed-in
qvariable continuously gets new tasks added.
[Original subject: Apply queue as context to deferred function call]
Right now the library does
f.apply(null,a)— it would be nice if the context could be overridden e.g.queue.deferUsing(ctx, fn, …)or if the context were set to the queue itself.Actually, nix
queue.deferUsing— that's what Function.prototype.bind is for. But passing the queue instead of nothing seems useful, no?