jashkenas / coffeescript

Unfancy JavaScript
https://coffeescript.org/
MIT License
16.49k stars 1.99k forks source link

Add syntax for handling nested callbacks #1704

Closed paulmillr closed 12 years ago

paulmillr commented 12 years ago

The main problem of node.js apps are nested callbacks. You use them A LOT everywhere. Code becomes like a ladder etc. I propose to add a simple operator for their handling.

With "<-" operator, these two snippets of code would be identical. Coco had already added this thing and it's very comfortable / easy-to-use.

(found) <- path.exists mainFilePath
return unless found

(error, data) <- fs.readFile mainFilePath, "utf8"
return helpers.logError error if error
compiler = stylus(data)
  .set("filename", mainFilePath)
  .set("compress", true)
  .include(@getPath "src")
compiler.use nib if nib

(error, css) <- compiler.render
return helpers.logError error if error
mainCss = @getBuildPath "web/css/main.css"

(error) <- fs.writeFile mainCss, css, "utf8"
return helpers.logError error if error
helpers.logSuccess "[Stylus]: compiled main.css."

path.exists mainFilePath, (found) ->
  return unless found
  fs.readFile mainFilePath, "utf8", (error, data) ->
    return helpers.logError error if error
    compiler = stylus(data)
      .set("filename", mainFilePath)
      .set("compress", true)
      .include(@getPath "src")
    compiler.use nib if nib
    compiler.render (error, css) ->
      return helpers.logError error if error
      mainCss = @getBuildPath "web/css/main.css"
      fs.writeFile mainCss, css, "utf8", (error) ->
        return helpers.logError error if error
        helpers.logSuccess "[Stylus]: compiled main.css."
Tharabas commented 12 years ago

Would you suggest do implement the <= (left fat arrow) for context bound continuations as well?

paulmillr commented 12 years ago

@Tharabas yes, that's a great idea too

So, any :+1:s? I could start working on the patch, if the maintainers wouldn't be against the idea.

friggeri commented 12 years ago

@Tharabas @paulmillr you mean the left fat arrow <= otherwise known as the lesser or equal operator ?

showell commented 12 years ago

I'm -1 on this idea. I actually find the second snippet of code to be more readable. I know beauty is in the eye of the beholder, but CS has too much syntax already, and I'd also like to see more evolution of async-beautification schemes that happen within CS itself. If you go with this syntax, then "<=" is obviously gonna be tricky to explain to folks.

paulmillr commented 12 years ago

@afriggeri I guess, we need to find a better operator for context bound continuations.

@showell it's because the second snippet has sort of logical separations. If you'll add some of them (e.g. blank lines) in the first example, you'll probably have the same level of readability (or even better one).

and I'd also like to see more evolution of async-beautification schemes that happen within CS itself

Do you know any good way to handle things like these in js besides monads and this monadic spike? Well, there are JS1.7 coroutines, but you'll need FX js engine to use them. And nodejs system things (fs.readFile etc.) hadn't been built with coroutines anyway.

Also it's pretty hard to fit in "80 symbols / line" limit with the async ladders.

If you go with this syntax, then "<=" is obviously gonna be tricky to explain to folks.

"=>" is not much more trickest to explain.

satyr commented 12 years ago

Coco chose to swap => with ~> for backcall. I guess Coffee can't do that at this point.

benekastah commented 12 years ago

After reading the extensive discussion on the topic in previous issues #241, #287 and #350 (and I know there's more), I have been lead to believe this problem needs to be solved in a different way. After all, callbacks are an integral part of the logic behind asynchronous code. To make the code path look linear is deceptive, and I imagine this would be very difficult for many people to grok, especially newcomers. I could be wrong, but I think that asynchronous programming requires extra thought/effort by design.

I can't speak for everyone, but essentially a good solution for me would have the following properties:

I started using futures from @coolaj86 recently, and I have to say I'm pretty satisfied with how well it mitigates these issues with async programming.

Am I missing any part of the problem? I'm curious as to the types of issues others are experiencing.

showell commented 12 years ago

@benekastah Your analysis seems spot on to me.

Asynchronous programming is very hard semantically, so I tend to be pessimistic about syntactical remedies. (-- unlike a lot of other nuisances in JS, which really are just cases of JS making life unnecessarily hard on us.)

Syntax tends to take on a life of its own, and it can well outlive its usefulness. Think about the semicolon. Think about Perl. I feel like the bar should be pretty high when it comes to adding syntax to a language.

My objection to the "<-" syntax is mostly about timing. I think CS is slowly coming out of the early adopters phase, and now we're reaching out to folks who are inherently skeptical and conservative about exotic constructs. You never see people on Twitter say "I'm not gonna try CS until they add a few more bells and whistles."

tpetry commented 12 years ago

The problem is not syntax, there have been some disscusions before naming special keywords like defer. The problem is the very complex implementation: Asychronous assignments in loops, errror management and all those nifty edge cases

jashkenas commented 12 years ago

Yep, I'm afraid this has already been discussed -- including this syntax -- a great many times. Scanning down this unline-broken block of code:

(found) <- path.exists mainFilePath
return unless found
(error, data) <- fs.readFile mainFilePath, "utf8"
return helpers.logError error if error
compiler = stylus(data).set("filename", mainFilePath).set("compress", true)
compiler.use nib if nib
(error, css) <- compiler.render
return helpers.logError error if error
mainCss = @getBuildPath "web/css/main.css"
(error) <- fs.writeFile mainCss, css, "utf8"
return helpers.logError error if error
helpers.logSuccess "[Stylus]: compiled main.css."

... it's really unclear, at a glance, that there are four different execution contexts there, and four different places where race conditions can start to happen. The <= amibiguity is just icing on the cake. CoffeeScript having significant whitespace means that indentation isn't something to shy away from ... if you're passing a function body, we want to be indenting that block.

paulmillr commented 12 years ago

@jashkenas could you show me, please, when exactly the race conditions could happen?

jashkenas commented 12 years ago

All I was saying is that state may change via other callbacks in between every async call. That single "block" of code gets handed off to the event loop and resumed four different times.

coolaj86 commented 12 years ago

One problem is that people typically implement callbacks as the last parameter of a function instead of the first (which is reliable, but ugly).

Another problem is that not every function uses a callback.

Another problem is that there's no way to feature-detect whether or not a function uses a callback unless you're using a library like FuturesJS that makes it possible (and effortless).

I think the best solution would be if, as part of the language, there were a built-in promise (or defer) for every function, just like every function has a built-in arguments.

Also, there needs to be a built-in chain, sequence, and join.

However, since those features aren't part of the language and there's no clear winner in the realm of flow-control libraries (although there are a few most popular), it's always up to the developer.

CoffeeScript could chose a convention to implement for code written in CoffeeScript, but all libraries pulled into that code written in vanilla JavaScript would still need a separate syntax.

That said, I think it would be awesome if CoffeeScript would say "screw it, we're blazing our own trail" and use the power of benevolent dictatorship to lay down the law; "like it or not, this is how we handle promises and we believe it's the best and that's what you get so deal with it".

A lesson we can all learn from python is that even if you don't agree with the syntax 111%, it's still better to have dependable syntax and language features rather than ambiguity.

michaelficarra commented 12 years ago

update: I've turned the contents of this comment into a full proposal over at #1710.

A few months ago, I was checking out tamejs, liked the ideas, and started thinking about how it could be incorporated into coffeescript. For the uninitiated: tamejs basically just takes the JS you write and compiles it to use continuation-passing style. So the output's not so pretty. Anyway, I took a few of the examples from the website, pasted them into a gist, and rewrote them in what I called "imaginary-coffeescript-with-defer". I'll include one gist inline and just link to the other.

Imaginary CoffeeScript

{resolve} = require "dns"

do_one = (host, cb) ->
  (err, ip) <- resolve host, "A", *
  console.log if err then "ERROR! #{err}" else "#{host} -> #{ip}"
  cb() if typeof cb is 'function'

do_all = (hosts) ->
  defer
    for host, i in hosts
      do_one host, null
  return

do_all process.argv[2..]

Original tamejs Example

var dns = require("dns");

function do_one (ev, host) {
  var err, ip;
  twait { dns.resolve (host, "A", mkevent (err, ip));}
  if (err) { console.log ("ERROR! " + err); }
  else { console.log (host + " -> " + ip); }
  ev();
}

function do_all (lst) {
  twait {
    for (var i = 0; i < lst.length; i++) {
      do_one (mkevent (), lst[i]);
    }
  }
}

do_all (process.argv.slice (2));

There's a pretty simple mapping from the added coffeescript constructs to the tamejs additions.

Now I'm not sure how appropriate it would be to add to CS because of the possibly irreparably ugly compilation. But I think it's worth a discussion even considering the numerous, extremely lengthy tickets on defer-style constructs. Hell, I think people would sacrifice the readable output for a powerful feature like that. And they would only need to do so when using that feature.

I think it makes a really good use of both <- and defer. That syntax just really seems to fit their functionality.

@jashkenas: do you think I should open an issue and make this a full proposal?

jashkenas commented 12 years ago

Sure, go for it, and invite the TameJS folks.

benekastah commented 12 years ago

After thinking about this discussion a bit, I made steps. It's a nice little async library (I think). You can see JS examples in the readme, but you can check out my test file, which is in CS (it's obviously much nicer in CS).

showell commented 12 years ago

Here's a way to rewrite the code in the OP so that most of the logic is not indented. It doesn't require any special libraries or syntax.

path_step = (mainFilePath, cb) ->
  path.exists mainFilePath, (found) ->
    return unless found
    cb(mainFilePath)

read_step = (path, cb) ->
  fs.readFile path, "utf8", (error, data) ->
    return helpers.logError error if error
    cb(data)

compile_step = (data, cb) ->
  compiler = stylus(data)
    .set("filename", mainFilePath)
    .set("compress", true)
    .include(@getPath "src")
  compiler.use nib if nib
  compiler.render (error, css) ->
    return helpers.logError error if error
    cb(css)

write_step = (css) ->
  mainCss = @getBuildPath "web/css/main.css"
  fs.writeFile mainCss, css, "utf8", (error) ->
    return helpers.logError error if error
    helpers.logSuccess "[Stylus]: compiled main.css."

path_step mainFilePath, (path) ->
  read_step path, (data) ->
    compile_step data, (css) ->
      write_step(css)
paulmillr commented 12 years ago

@showell I see that write step here cannot use variables from compile / read steps. That's bad.

showell commented 12 years ago

@paulmillr, You say that it's a bad thing that write_step cannot use variable from the compile/read steps. Why? The only variable that write_step needs from compile/read is css, so why pollute its scope?

paulmillr commented 12 years ago

@showell well, you're right.

I've thought about examples where last steps require data from the first steps, but no examples of them come to my head right now.

coolaj86 commented 12 years ago

I think it's important to get rid of the notion of passing a callback function in. It creates more logic and confusion.

If we're going to pass callbacks, it should be a first parameter, not a last that is sometimes 2nd, sometimes, 3rd, sometimes 4th, etc.

You end up with this crap:

// brackets [foo] denote optional parameter, not literal syntax
function foo(name, [age], [sex], [callback]) {
  if ('function' === typeof sex) {
    ...
  }
  // same for age
  ...
}

But this is just ugly, so I don't think passing it in as the first parameter really makes sense either:

function doStuff(function (err) {
  !err && console.log('callback complete') || throw err;
}, name, age, sex);

I really feel that the best way around this is to write all callback functions using a returnable defer / promise object.

Imagine that result were a built-in function like arguments that fulfilled a promise:

function add(a, b) {
  var finalResult = result;
  setTimeout(function () {
    finalResult(null, a + b);
  }, 100);
}

add(a, b).when(function (err, sum) {
  console.log(sum);
});

With CoffeeScript, there's the opportunity to correct this deficiency of the language and make it simple. It's just a matter of defining the most basic cases. I believe those cases are:

One thing for sure, is that we need a solution that enough people agree on a convention and a benevolent dictator to back it so that we have fewer flow-control libraries. Everyone keeps re-writing the same thing over and over and over with their own syntax and their own style. I believe the CoffeeScript community could have the power to end the confusion and disarray.

I'm working on an article to examine the issues more thoroughly. I'll post back when I've got the thoughts more organized.

benekastah commented 12 years ago

@showell That's a really nice way to do it, too.

showell commented 12 years ago

@paulmillr In cases where the steps do need to share variables, and where it's awkward to pass them down as parameters, you can always initialize the variables in the outer scope.

(FYI I edited my comment after your reply to it, but it was just to better quote the context.)

benekastah commented 12 years ago

@coolaj86 Please do post back. I'd like to read your article. I think you are starting a valuable discussion when it comes to having something like promises baked-in to CS. Probably it should be proposed in a new issue, so we can really focus on it.

tpetry commented 12 years ago

@coolaj86: How do you wan't to integrate this defered callback style with all yet existing node.js JavaScript libraries? If it would be incompatible those defered CS callbacks would be only usable for the small part of CS libraries.

coolaj86 commented 12 years ago

@tpetry:

See here: https://github.com/jashkenas/coffee-script/issues/1710#issuecomment-2130110

I think the solution should be to use a library that works outside of CS and apply syntactic sugar such that the callback style appears built-in.

I'm working on an article I hope to share this evening or tomorrow morning.

benekastah commented 12 years ago

@michaelficarra Just had a look at TameJS. It looks awesome. I would definitely be interested in seeing a branch on that. I would help work on it as well (although it would be slow going for me at first -- haven't been able to really grok the language-building code yet).

satyr commented 12 years ago

Note that backcall isn't limited to async stuff. It's similar to the mofor proposal (#1032), only more primitive and generic.

coolaj86 commented 12 years ago

Update: not quite done with the article, just haven't had the time to finish it. Here's to tomorrow...