- From: Tab Atkins Jr. <jackalmage@gmail.com>
- Date: Sun, 19 May 2013 19:05:35 -0700
- To: Sean Hogan <shogun70@westnet.com.au>
- Cc: "www-dom@w3.org" <www-dom@w3.org>, "public-script-coord@w3.org" <public-script-coord@w3.org>, Anne van Kesteren <annevk@annevk.nl>
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