Re: Deprecating Future's .then()

On Sun, May 19, 2013 at 6:04 PM, Sean Hogan <shogun70@westnet.com.au> wrote:
> On 20/05/13 8:32 AM, Tab Atkins Jr. wrote:
>> I *really* don't like this proposal.  Comments inline, as usual.
>>
>> On Sat, May 18, 2013 at 3:09 PM, Sean Hogan <shogun70@westnet.com.au>
>> wrote:
>>>
>>> BENEFITS:
>>>
>>> The main benefits of this addition are:
>>>
>>> a. consistency in the way futures are initialized and resolved - that is,
>>> resolving is always asynchronous via `resolver.accept|reject`.
>>
>> This comes at the expense of easier and simpler code to write in the
>> common case, when you're receiving an already-built future and are
>> just acting on it via .then().  You can no longer just return a value
>> like a normal function; you are forced to call `this.accept()`.
>
> You can still do this - I haven't proposed changing the behavior of .then().

I'm saying that .thenfu() is less convenient *in general*, and the one
case where it is more convenient (when you want to hand out the
ability to resolve your chained future) is too small for the code
savings your proposal grants it.  The fact that you can still use the
more convenient method doesn't change that.

>> Given that you have to reject the promise based on uncaught errors in
>> the callback anyway (or else you lose an important quality of
>> promises), `this.reject()` doesn't give you anything
>> (`this.reject(val)` is identical to `throw val`, but longer).
>>
>> You've also removed the ability to do `this.resolve()`, for some
>> reason.  This isn't explained at all in your proposal, which makes me
>> assume that you don't understand what it's used for.  This means you
>> can't force your promise to delegate to another promise, as you can do
>> today.
>
> The resolve algorithm is still part of the way .then() handles callbacks - I
> haven't suggested changing that.

By making it only part of how .then() handles callbacks, you've
removed the ability of the FutureResolverCallback to use it.  All the
essential primitives need to be exposed directly, not hidden in the
behavior of another function.

>> Finally, you're overriding `this` to be the future's resolver, thus
>> ruling out any possibility of passing in class methods that want to
>> refer to the class instance when processing the future's value.  It's
>> unclear what the effect of this would be on hard-bound methods, as
>> well.
>
> Yes, I couldn't work out why the callback would want access to the
> resolver's future.
> Could you give an example of that usage?

Apologies, I must have been unclear.  I was referring to something like this:

var x = new Foo();
var f = getFuture();
f.then(x.doSomething.bind(x));

Where Foo#doSomething relies on other properties or methods of the Foo
instance.  If you override `this` for the callback, then the method
will break.  If you *don't* override `this` (which is more likely
here, since `this` is hard-bound), then the callback can't resolve the
chained future.

> Actually I'm not proposing changing .then().

Sorry, throughout my email I'm intending to refer to .thenfu().
Apologies for this being unclear.

> The example I was thinking of (which I put in my proposal) used the Async
> callback model:
>
>         callAsyncFunction(value, function(err, result) { ... }

This doesn't materially alter my response.

> Yes, the difference is precisely the creation of a new Future. Every time.

Every time you're further deferring resolution to another async
operation, yes.  This doesn't seem to be onerous.

> I addressed this in my proposal but I'll say it a different way here:
>
> As you say:
>     .thenfu() can be implemented with .then() plus the creation of *another*
> new Future
>
> The reverse is also possible and even simpler:
>      .then() can be implemented with .thenfu() plus the "resolve" algorithm
>
> So .thenfu is simpler internally and is simpler for mixing with non-Promise
> async code.

This is not a significant simplicity difference for either side, imo.

> .then() is simpler for synchronous code and Promise objects, but...
>
>>> c. `then` doesn't need to be the feature-test for Promise objects,
>>> and doesn't need to be  a forbidden method of non-Promise objects.
>>
>> I don't understand how this is a result of your proposal.
>
>
>
> A contrived example:
>
>     var Data = {
>         x: 1,
>         y: 2,
>         then: function() { }
>     }
>
>     Future.accept()
>     .then(function() {
>         return Data;
>     })
>     .done(function(value) {
>         someExternalCodeExpectingData(value);
>     });
>
> This chain will stall (silently) because `Data` will be treated as a Promise
> object.
> Replace the .then() call with .thenfu and the following won't stall:
>
>     .thenfu(function(value) {
>         this.accept(Data);
>     })

Yes, this is why I don't understand.  You can do the same thing in
Futures, once Anne fixes the spec like he's saying he will, by doing:

.then(function(val) {
  return Future.accept(Data);
}

This unconditionally stores the Data object in the Future, without
regards to whether Data is a thenable, a native Future, or a plain
value.

(Alex Russell is pursuing an alternate approach to resolving this
which will solve the problem slightly better, in my opinion.)

> Personally:
> - I want my async code to look like async code;

I'm not sure what this means, but your examples aren't substantially
different in your proposal and the current spec.  Where they do have
differences, I find the current spec simpler and easier.

> - I don't like the idea of receiving some quantum "value-or-a-Promise";

I don't understand.  When do you ever receive a "value-or-a-Promise"?

> - and if my code receives one then I want **my** code to be in charge of
> detecting it.

You can detect whether something is a Future or not easily - just do
an instanceof check.

> I doubt I'm the only one who thinks this way.
>
> My proposal:
> - suits the async coding pattern,

I presume by this you mean Node's async pattern?  It seems reasonably
clear to me that Node settled on an async callback pattern due to
jumping the gun with an extremely bad initial promise implementation,
and they've now risen to a local optimum with it.  This local optimum
is much lower than what a good promise implementation with equal
uptake would bring them to, though.

> - adds two methods but **no** complexity to the spec, and

Every method is complexity.  It's more things to implement and test,
it's more things for authors to remember and possibly get wrong.  Even
literal function aliases have to justify themselves.

> - to my knowledge doesn't compromise the current compatibility with JS
> Promise implementations.

Saying "don't use the Promise properties, they're busted" isn't great
compatibility, and it leaves us stranded in a halfway-land where we're
neither compatible with promises nor Node's async pattern.  I don't
think it can justify itself on its merits sufficiently to overcome
these difficulties.

~TJ

Received on Monday, 20 May 2013 02:06:26 UTC