jashkenas / coffeescript

Unfancy JavaScript
https://coffeescript.org/
MIT License
16.47k stars 1.98k forks source link

backcalls: let's add them #2762

Closed michaelficarra closed 7 years ago

michaelficarra commented 11 years ago

Copied proposal from https://github.com/jashkenas/coffee-script/issues/2662#issuecomment-13026081 below


Taken pretty much from the coffee-of-my-dreams list, the best way to explain my preferred backcall syntax is through example:

this must be preserved in the continuation by rewriting references as we currently do in bound functions. I don't think we should worry about arguments rewriting, but that's your call.

Here's a real-world example @paulmillr gave in #1942:

compile = (files, callback) ->
  async.sortBy files, @sort, (error, sorted) =>
    return @logError error if error?
    async.map sorted, @map, (error, mapped) =>
      return @logError error if error?
      async.reduce mapped, null, @reduce, (error, reduced) =>
        return @logError error if error?
        @write reduced, (error) =>
          return @logError error if error?
          do @log
          callback? @getClassName()

and here it is using backcalls:

compile = (files, callback) ->
  (error, sorted) <- async.sortBy files, @sort
  return @logError error if error?
  (error, mapped) <- async.map sorted, @map
  return @logError error if error?
  (error, reduced) <- async.reduce mapped, null, @reduce
  return @logError error if error?
  error <- @write reduced
  return @logError error if error?
  do @log
  callback? @getClassName()

edit: dropped the callback position indicator, opting for "hugs" style

vendethiel commented 11 years ago

You can outdent back.

createBinder = ->
  <- foobar
  @add 5

foo = createBinder()
michaelficarra commented 11 years ago

@hden: Feel free to use them at program level. That's actually the only place where it's truly capturing the entire program's continuation.

hden commented 11 years ago

@Nami-Doc @michaelficarra You're right.

<- $
do makeSense
00dani commented 11 years ago

It's fine to use program-level backcalls. Without being allowed to do that, these don't work particularly well:

# load dependencies for this file
($, _) <- require ["jquery", "underscore"]
anyCodeCanGoHere()
evenCodeUsing $ and _
# or define a module with dependencies
($, _) <- define 'myModule', ["jquery", "underscore"]
00dani commented 11 years ago

Here's a question: Do we want a backcall syntax to work for assigning the function to some variable, instead of passing it into a function? The only place I see this as obviously useful is when assigning to module.exports, but that does seem a rather useful place for it. For example, writing a Gruntfile.coffee:

grunt <-= module.exports # syntax meaning module.exports = (grunt) ->
grunt.initConfig blah blah

However, it's possible just to do this:

export = (m) -> module.exports = m
grunt <- export
grunt.initConfig blah blah

So special syntactic support mightn't be necessary.

Edit: If we allow placeholders, this sort of structure might arise as possible:

grunt <- module.exports = ^
00dani commented 11 years ago

To throw another spanner into the very simple works of regular backcalls, what if they had special treatment when they can be interpreted as function arguments? Specifically, what if they worked like this?

(err, res) <- async.parallel
  users <- getTable "users"
  posts <- getTable "posts"
doSomethingWith res.users, res.posts
# translates as equivalent to
async.parallel {
  users: (cb) -> getTable "users", cb
  posts: (cb) -> getTable "posts", cb
}, (err, res) ->
  doSomethingWith res.users, res.posts

Might be a bit much, but currently backcalls don't particularly help much with async.parallel-type operations, and we don't have general partial application.

shesek commented 11 years ago

@00Davo I think it might be too confusing too have the <- operator mean partial application in some places, and backcalls in others.

You could still do that pretty easily with a partial application function (assuming passing arguments with indentation works?) with something like:

(err, res) <- async.parallel
  users: partial getTable, "users"
  posts: partial getTable, "posts"
doSomethingWith res.users, res.posts

And if a partial application operator is ever added, it should fit well with that syntax too.

00dani commented 11 years ago

@shesek Good point. That's still pretty readable, and it's less async-specific than my proposal, which relies pretty heavily on exactly how async.parallel's arguments work. (Passing arguments by indentation works in current CoffeeScript, and I think we shouldn't let backcalls interfere with that feature.)

What does <- mean in the context of a function argument, however? Will it just be treated as a syntax error, or will it work kind of like the "decorator" syntax you proposed earlier, or should there be some other interpretation entirely?

shesek commented 11 years ago

What does <- mean in the context of a function argument, however? Will it just be treated as a syntax error, or will it work kind of like the "decorator" syntax you proposed earlier, or should there be some other interpretation entirely?

It depends - are backcalls meant to be used as expressions or only as pure statements? If its a pure statement, than it should be a syntax error. If they work as expressions (and just return the synchronous return value of the asynchronous function), than backcalls as arguments should probably just be regular backcalls, and pass the return value of the backcall as the argument.

But I'm not sure how they'll work as expressions in some contexts... for example, with something like:

 foo
  bar: <- baz
  qux: <- corge

What would be the body of the function passed to baz? if it consumes the rest of the block, it should be the qux: <- corge, but that doesn't make much sense :-\ Backcalls effect the rest of the block, so its kinda odd to use them in the middle of another expression. Perhaps they should be statements?

edit also, any thoughts on http://irclogger.com/.coffeescript/2013-03-06#1362609333 ?

00dani commented 11 years ago

I don't particularly like putting (args) <- on the end of the line, as suggested in that IRC log. Making backcalls closer to other expressions is a good idea, but by requiring them to be located like normal callbacks you lose the visual similarity of x <- operation and x = operation.

It does afford the ability to decorate backcall functions much more simply, and it extends to variable assignments, without complicating the setup with placeholders, though. I'm not sure those advantages are worth giving up the original "magic =" arrangement, however.

michaelficarra commented 11 years ago

@jashkenas: Can we get an official go-ahead on this feature? I find myself wanting it nearly every day I write CoffeeScript. I want to make sure it will be accepted before putting in the time to implement it.

g0t4 commented 11 years ago

Just FYI https://github.com/Sage/streamlinejs has some interesting implications for this, particularly in how it also adds capacity to get back to try/catch/finally blocks, IMO error handling is as much of a problem with the verbosity of callbacks as other things mentioned above.

bjmiller commented 11 years ago

+1 for backcalls usable as expressions, if possible. One of the guiding principles is that everything should be an expression that returns something if it can, right?

jashkenas commented 11 years ago

@michaelficarra -- I'm afraid I can't give you the go ahead (for this particular version of the compiler, at least). I've skimmed/read through this ticket several times, but need to sit down and more thoroughly tinker with some of the examples, and think through it a bit more.

At the moment, I'm fairly strongly leaning towards leaving them out ... but that isn't final.

mehcode commented 11 years ago

Something I'd like to add is that if this ever gets added It'll be strange if we can't have unbound back calls (from what I can see <- in its in current form is always bound). <= is a bit too ambiguous unfortunately to allow <- to not be bound. I'd vote for switching to <~ and ~> for bound calls if it's decided to include back calls.

ashtuchkin commented 11 years ago

For the record: we extensively use a error handling helper along the lines of:

linesInFile = (name, cb) ->
  fs.readFile name, "utf8", errTo cb, (text) ->  # errTo sends error straight to cb.
    cb null, text.split("\n").length

With backcalls, it would be nice to keep this convenience, something like this (rewriting real-world example above):

compile = (files, cb) ->
  sorted <- async.sortBy files, @sort, errTo cb
  mapped <- async.map sorted, @map, errTo cb
  reduced <- async.reduce mapped, null, @reduce, errTo cb
  <- @write reduced, errTo cb
  do @log
  cb? null, @getClassName()

To keep it, we need to be sure that the backcalls are applied to the last function of the line (errTo in our case).

vendethiel commented 11 years ago

To keep it, we need to be sure that the backcalls are applied to the last function of the line (errTo in our case).

I'm afraid this is not possible :(.

<- a b would then mean a b -> which is confusing and makes them pretty unusable.

00dani commented 11 years ago

It can be done, slightly more verbosely, given a placeholder. For example, with ^ as placeholder, that example would be written like this:

compile = (files, cb) ->
  sorted <- async.sortBy files, @sort, errTo cb, ^
  mapped <- async.map sorted, @map, errTo cb, ^
  reduced <- async.reduce mapped, null, @reduce, errTo cb, ^
  <- @write reduced, errTo cb, ^
  do @log
  cb? null, @getClassName()
shesek commented 11 years ago

@ashtuchkin heh, I'm using a function [1] exactly like your errTo, only with a different name. I asked about using that with backcalls a few comments ago [2]. I really want to be able to use it with backcalls, too :)

As @Nami-Doc said, the syntax you proposed already has other valid meaning in the current proposal, so it wouldn't work. I suggested an alternative syntax at [2], and its also possible to do that with placeholders, if they're added.

Also, you can do that with an higher-order function, like that:

# I should probably find a shorter name for that
bound_error_handler = (fn, errcb) -> (a..., succcb) -> fn a..., iferr errcb, succcb

sorted <- (bound_error_handler async.sortBy, cb) files, @sort

edit: another way to do that is with the "hug operator": (sorted) <- (_) -> async.sortBy files, @sort, iferr cb, _

[1] https://gist.github.com/shesek/eff0c0abd31ad8457de8 [2] github.com/jashkenas/coffee-script/issues/2762#issuecomment-14414831

ashtuchkin commented 11 years ago

Thanks for the explanation. I then vote for placeholder support, because the majority of callbacks in my code use errTo (i.e. all interactions with node, express request handlers, etc.) and this feature has no value for me if I cant use it or it makes the code uglier.

michaelficarra commented 11 years ago

Wow, have none of you bothered reading through the thread? We dropped placeholder support because it can always be replaced with a function literal.

edit: Just saw @shesek's edit above where he mentions it.

ashtuchkin commented 11 years ago

You dropped it because non-last callback was seen as a rare case, which it is. Here we have a different case - error handling helper, which is arguably more common and writing the hug operator on almost every callback is definitely a show stopper for me. It could be just my pain, I don't know. And I definitely want to make the language as simple as possible. But without placeholders, maybe optional ones, this feature has very limited usage in my case.

michaelficarra commented 11 years ago

@ashtuchkin: Ah, okay. I agree, that is a more common case, but also a case where a higher order function like @shesek's would be more meaningful. I would opt to use that function since it works as an annotation, showing intent better than either the inline function or the placeholders.

g0t4 commented 11 years ago

Perhaps we should just use https://github.com/Sage/streamlinejs, it already takes care of this problem.

On Fri, Mar 22, 2013 at 10:59 AM, Michael Ficarra notifications@github.comwrote:

@ashtuchkin https://github.com/ashtuchkin: Ah, okay. I agree, that is a more common case, but also a case where a higher order function like @shesek https://github.com/shesek's would be more meaningful. I would opt to use that function since it works as an annotation, showing intent better than either the inline function or the placeholders.

— Reply to this email directly or view it on GitHubhttps://github.com/jashkenas/coffee-script/issues/2762#issuecomment-15304930 .

ashtuchkin commented 11 years ago
sorted <- (bound_error_handler async.sortBy, cb) files, @sort

In my view it hides whats really happening (async.sortBy), placing too much attention to error handling, especially with those additional parentheses :( With placeholder (or the way I suggested initially) it's just a suffix that you pay little attention to when reviewing the code, but still notice when its absent and easily understand what happens when error arises.

jamesonquinn commented 11 years ago

I like this syntax, and think it could fit with mainline coffeescript.

And I like iced, and don't think that could fit with mainline coffeescript.

But as far as I can see this gives you only about half of what iced-cs gives you. That is, even with this, there is no clean, DRY way to build iced-cs as a library, with "await" and "defer" as functions instead of keywords.

But I think that the gap is small. The only thing missing is a way for "defer" to sneak variables from an inner to an outer context. I believe that a small bit of extra syntax could do this.

This is not easy to think out, and I'm sure my first proposal will be lacking. But I'm thinking along the lines of syntax that would let you write a library to make the iced-cs:

await
  for item, index in someList
    item.getValue defer values[index]
...

as:

<- await@@ (defer) ->
  for item, index in someList
    item.getValue defer (@@values[index]) -> 
...

The idea is that the @@ in await@@ sets up an outer context which all inner @@variables will be declared. So:

  1. "await" would just be a library function such that "await f1, f2" calls "f1 defer", where defer is a function such that "defer f3" returns a "satisfied" function which simply passes its arguments to f3. When f1 is done, "await" waits until all the "satisfied" functions which were created have been called, then calls f2.
  2. The syntax (@@x) -> ... is shorthand for (x) -> @@x = x; .... Thus in the code above, the "satisfied" function returned by defer, sets values[index] in the context of the outer await.

I'm not satisfied with the above syntax (for instance, the @@ as it stands doesn't allow nested await statements; you could patch that with "await@@x" and then "x@@varName" but that's getting even uglier), but to me it does serve as a proof of concept that with backcalls plus a bit of syntax for putting variables into outer contexts, you could make it possible to write iced-cs as a library without violating coffeescript's "it's just javascript" philosophy in the way that the current iced-cs does.

vendethiel commented 11 years ago

I don't like your syntax (especially since we don't have @@)

(also edited your message to add coffee highlighting)

jamesonquinn commented 11 years ago

@Nami-Doc: Neither do I; it was intended as a proof-of-concept. My point was that:

  1. I was +1 on adding backcalls to cs.
  2. Even with backcalls, I thought that you couldn't write iced-cs as a library, so I thought that it was premature to close the iced-cs issue on this one.

But on second thought, 2 isn't true. With just backcalls, you could write a library (with a single function "await") so that the following would work:

awaitResult <- await (defer, innerAwaitResult) =>
  innerAwaitResult.values = []
  for item, index in someList
    item.getValue defer (@values[index]) -> 
...
console.log awaitResult.values[2]
...

That is, instead of inventing ugly new syntax to put variables into outer contexts, you can simply put them as properties onto an object which you then return.

So now that I realize that this would truly make iced-cs unnecessary without breaking the "it's just javascript" philosophy, I am going from +1 on this issue to +2.

jamesonquinn commented 11 years ago

So the more I think about this, the more I am enamored of its awesome power. @jashkenas : what would it take to help convince you? A working patch? A draft version of iced-coffeescript-as-a-library (which would work with a smoothed-out version of the syntax I proposed just above)? More votes for this issue? Time to think without us pressuring you?

jamesonquinn commented 11 years ago

Trivial question: what should the indentation be like in the compiled javascript?

vendethiel commented 11 years ago

same as a function call.

michaelficarra commented 11 years ago

@jamesonquinn: I love the enthusiasm. I think we can pretty much call this feature request accepted at this point. We've gotten a lot of support, shown its usefulness, and discovered some useful idioms (such as the one you mentioned or the anonymous function for functions that take the callback in non-final positions). I could almost guarantee a good pull request would be accepted, but it would be great if @jashkenas confirmed that. I'll open an issue in CoffeeScriptRedux to implement them.

jashkenas commented 11 years ago

Time, I'm afraid, is not on my side ... but. In general, for any feature, what we need is a clean, well-written pull request, with tests, and a demonstration of how the feature works and is useful in real-world code in the ticket. Those PRs are easy to simply merge quickly. (Perhaps too easy ;)

That said, I'm still not sold on the parallelism between back-calls and the usual straightline syntax of programming without the continuation. In particular, this bit:

simply use an anonymous function to force the position of the callback.

(a, b) <- (_) -> fn c, _, d

... seems far, far too cryptic to add to CoffeeScript, even if it's a rare case that the callback happens to not be in the tail position. Is there a better way, or is Michael's summary at the top still state of the art?

vendethiel commented 11 years ago

Things have been proposed for this case, not sure what you think about them.

shesek commented 11 years ago

@jashkenas what you think about #issuecomment-14413104?

jashkenas commented 11 years ago

@shesek: A pretty nasty hack, no? You wouldn't want to tell or teach a beginner to do that ... would you?

michaelficarra commented 11 years ago

@jashkenas: There were suggestions for a placeholder syntax, but I rejected them in favour of keeping the language simpler. I don't think that anonymous function looks so bad. Is the _ making it look cryptic to you?

shesek commented 11 years ago

@jashkenas Well, I wouldn't wanna teach a beginner how to write that higher-order function... but just learning to use that function shouldn't be that problematic. It could feel very "syntaxy" if its given proper names, like <- callpos load_user, id, _, true (underscore being the "marker", callpos being still not a very good name, but I couldn't think of anything better)

bjmiller commented 11 years ago

Let's take that example. How would you read that off to someone in English (or, your own local spoken tongue)? If you can break it down in one sentence to a beginner with CS, then I think maybe that meet's JA's criteria. (As always, I could be wrong.)

jashkenas commented 11 years ago

Perhaps I just don't understand the example well enough. Can someone write @paulmillr's compile example, from the bottom of Michael's post, using this syntax ... as if the callback was the first argument to each function, instead of the last one?

vendethiel commented 11 years ago
compile = (files, callback) ->
  (error, sorted) <- async.sortBy _, files, @sort
  return @logError error if error?
  (error, mapped) <- async.map _, sorted, @map
  return @logError error if error?
  (error, reduced) <- async.reduce _, mapped, null, @reduce
  return @logError error if error?
  error <- @write _, reduced
  return @logError error if error?
  do @log
  callback? @getClassName()
shesek commented 11 years ago

@Nami-Doc that relies on the placeholder that was canceled (tho it was ^, not _, which is already a valid identifier). I think he meant with the "hug operator", or possibly with the higher-order function.

Edit: wording and attempted mind reading

vendethiel commented 11 years ago

Yes, that's what the code is about. The symbol doesn't matter.

jashkenas commented 11 years ago

@Nami-Doc -- that doesn't seem like what Michael is suggesting with the anonymous function trick. That looks like a placeholder.

vendethiel commented 11 years ago

AH, I misread. Ok :

compile = (files, callback) ->
  (error, sorted) <- (_) -> async.sortBy _, files, @sort
  return @logError error if error?
  (error, mapped) <- (_) -> async.map _, sorted, @map
  return @logError error if error?
  (error, reduced) <- (_) -> async.reduce _, mapped, null, @reduce
  return @logError error if error?
  error <- (_) -> @write _, reduced
  return @logError error if error?
  do @log
  callback? @getClassName()
shesek commented 11 years ago

Edit: @Nami-Doc beat me to it.

I don't find it very readable myself, too.

Edit 2: Not really related to the callback position, but I really dislike all the error handling you'd have to use. I really think we should consider a better way to handle that (see #issuecomment-14414831, #issuecomment-15292734 and this).

Edit 3: We can use a much nicer higher-order function than what I wrote above. I think that looks much better and is an acceptable solution for error handling:

iferr = (errcb, fn, a..., cb) -> fn a..., (err, b...) ->
  if err? then errcb err else cb b...

compile = (files, callback) ->
  sorted <- iferr @logError, async.sortBy, files, @sort
  mapped <- iferr @logError, async.map, sorted, @map
  reduced <- iferr @logError, async.reduce, mapped, null, @reduce
  <- iferr @logError, @write, reduced
jashkenas commented 11 years ago

One last wrinkle ... bound-function-back-calls? Do we have an acceptable solution? They're pretty common.

I see in this ticket that backcalls are assumed to be bound by default ... but that doesn't seem quite right, as the parallel between <- and -> is broken.

00dani commented 11 years ago

We could use Coco's solution of adding ~> for bound functions and thus an analogous <~, perhaps?

vendethiel commented 11 years ago

I see in this ticket that backcalls are assumed to be bound by default ... but that doesn't seem quite right, as the parallel between <- and -> is broken.

agreed that we can't assume bound, <= is not possible (and we're not using ~>)

michaelficarra commented 11 years ago

Backcalls were assumed bound because you don't want this changing out from under you without a change in indentation. They are supposed to be implicit continuations with explicit scope enrichments. It's unfortunate that there won't be parity between => and <-, but I don't think that's so bad.