Re: Deprecating Future's .then()

On 20/05/13 12:05 PM, Tab Atkins Jr. wrote:
> On Sun, May 19, 2013 at 6:04 PM, Sean Hogan <[email protected]> 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 <[email protected]>
>>> 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.


The accept/reject callbacks in .thenfu() have the same form as the 
`init` callback in `new Future()`.
Do you think that form of `init` is not useful?


>>> 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.


So people who want future.then() will want auto-`then`-detection in 
their init callback and, therefore, resolver.resolve().
Fair enough.


>>> 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));


Let's see. If I'm reading the spec right, what 
`f.then(x.doSomething.bind(x))` does is create a new Future (call it 
`newf`) and append to it a "future-callback-wrapper for the associated 
resolver (call it `newr`) and the callback (call it `onaccept`)". The 
wrapper should do more-or-less:

     function(arg) {
         try {
             var value = onaccept.call(newf, arg);
             newr.resolve(value);
         }
         catch(err) {
             newr.reject(err);
         }
     }

Now `onaccept` is `x.doSomething.bind(x)` which is more-or-less:

     function() {
         var func = x.doSomething, thisArg = x;
         return func.apply(thisArg, arguments);
     }

In this scenario `onaccept.call(newf, arg)` does the same as 
`onaccept(arg)` - `doSomething()` is never bound to anything other than 
`x`. So I can't see how it matters what `this` is set for `onaccept`.

FWIW, Alex Russell's polyfill - 
https://github.com/slightlyoff/DOMFuture/blob/master/polyfill/src/Future.js 
- sets the `onaccept` callback's `this` to `null`.


> 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.
>


See above


>> 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);
> }


But that is just a restatement of:

     This can be addressed by creating a new Future.

>> 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?


No. I can't see how you could reasonably interpret "async" that way -
.thenfu() callbacks match `new Future()` init callbacks.


>> - 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.


But your solutions add more complexity than .thenfu():
a. "just create another new Future" - complexity for the JS dev
b. "we could create a convenience wrapper" - complexity for the spec and 
browsers


>> - 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.


That a big misrepresentation.
I'm not suggesting removing .then() nor changing its behavior.
And I haven't advocated Node's async pattern.

Sean