Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding support for generators (yield keyword) #3078

Closed
wants to merge 4 commits into from

Conversation

@almost
Copy link

@almost almost commented Jul 21, 2013

Generators have landed in V8 and Node.JS and they're pretty useful. I think CoffeeScript should have support.

I've hacked in basic support, I'm sure more work is needed but I thought it would be useful to start the discussion.

I'm interested in what the CoffeeScript policy is for features not supported by all JS engines, should they be left out or would it be useful to have a fallback implementation (based on a CPT and a Trampoline?) as a command line option? Neither seem satisfactory but then neither does restricting CoffeeScript user's use of exciting new JavaScript features :)

@almost
Copy link
Author

@almost almost commented Jul 21, 2013

I wrote a blog post about it with an example ported to CoffeeScript: http://almostobsolete.net/coffeescript-generators.html

@vendethiel
Copy link
Collaborator

@vendethiel vendethiel commented Jul 21, 2013

I'm interested in what the CoffeeScript policy is for features not supported by all JS engines

I think it's "later".

@purge
Copy link

@purge purge commented Jul 21, 2013

Has there been a discussion about this policy? I think when features like this drop in node stable, I think it's up to the developer to make an informed choice as to whether they can use them (with a suitable warning in the documentation).

There are engine specific features that can already be used from Coffeescript so this wouldn't be without precedent (though i'm aware experimental keyword feature support is a rabbit hole one might not want to go down).

@maccman
Copy link

@maccman maccman commented Jul 21, 2013

This is going to be super useful in Node applications (indeed Harmony already has yield support). Looking forward to seeing it part of CS.

@michaelficarra
Copy link
Collaborator

@michaelficarra michaelficarra commented Jul 21, 2013

@purge: See #3073 and #2638

@almost
Copy link
Author

@almost almost commented Jul 21, 2013

I also added support for the yieldfrom statement, it's like ES6's "yield*" and lets you combine generators in a more natural way:


range = (start, stop) ->
  yield i for i in [start...stop]

addBeginAndEnd = (generator) ->
  yield "BEGIN"
  yieldfrom generator
  yield "END"

g = addBeginAndEnd(range(0,3))()
// G will now yield BEGIN, 0, 1, 2, END

I haven't included it in this Pull Request yet but the commit is here:

almost@67f2347

Should I include it in this PR? Make another PR? Or should I just leave it out for now?

EDIT: Now included in the PR (along with the iteration syntax)

@jashkenas
Copy link
Owner

@jashkenas jashkenas commented Jul 21, 2013

@almost Feel free to include it in this PR if you dig it. In general, try to make a PR as nice and complete as you're able to.

@almost
Copy link
Author

@almost almost commented Jul 21, 2013

Ok, I've included in with the rest. I'm going to see about adding a for...from syntax now for iteration now..

@almost
Copy link
Author

@almost almost commented Jul 21, 2013

Ok, so apparently "from" is used quite a lot in existing code (including the CS code). Any ideas for another keyword to use?

resvar = o.scope.freeVariable 'result'
refvar = o.scope.freeVariable 'ref'

code = "var #{refvar} = #{[" #{@expression.compile o, LEVEL_PAREN}" if @expression]};\n"

This comment has been minimized.

@vendethiel

vendethiel Jul 21, 2013
Collaborator

I think we can do better than that. We should have methods for temp vars & we can generate AST nodes

@almost
Copy link
Author

@almost almost commented Jul 21, 2013

I've added a iteration syntax for generators (or any iterator that has a .next() that works the same way as a generator). I did initially want to use "for x from y" but found that "from" is popular variable, I could hack the parser so that you can use it as a variable anywhere where it wouldn't be ambiguous but I think that's probably a bad idea.

The syntax I've currently gone for (suggested by Richard Dallaway on Twitter) is "for x outof y" but it would be easy enough to change if anyone can think of a better word.


range = (start, stop) ->
  yield i for i in [start...stop]

for x outof range(0,10)
  console.log(x) 

(which will print the numbers 0 to 9)

The code for this one could probably do with refactoring (the other bits as well, but this one more so) but I'll hold of on that until it's been discussed whether it would actually be wanted.

@almost
Copy link
Author

@almost almost commented Jul 21, 2013

What about for x <- y?

@vendethiel
Copy link
Collaborator

@vendethiel vendethiel commented Jul 21, 2013

I think it'd be syntictally ambiguous with the proposed syntax for backcalls.

@almost
Copy link
Author

@almost almost commented Jul 22, 2013

Hmm yeah, that's true.

Another option is to not have an explicit syntax for it but leave it to a utility function. Maybe _'s _.map and _.each could be extended to recognise iterators or new versions of those functions could be added...

@akloster
Copy link

@akloster akloster commented Jul 22, 2013

It should be possible to emulate generators without breaking compatibility, by emulating the iterator/generator protocol like an ES6 to ES3 compiler would do. That way, a generator function would be compiled to a function which returns an iterator, and the iterator would be an object with a next method etc.

Reframing a generator function into that format can get tricky. Traceur for example doesn't support generators currently. Continuum (https://github.com/Benvie/continuum) claims to support this.

@almost
Copy link
Author

@almost almost commented Jul 22, 2013

@akloster While it is possible it really involves mangling the code, you first have to do a continuation passing transform where you split the function up into multiple bits (broken wherever there's a yield statement). CoffeeScript currently relies heavily on JavaScript's underlying semantics (that's one of it's major selling points) and it would lose a lot of the benefits of this. You'd have to reimplement loops, try...catch and other control flows in a way that was compatible with the new style.

@akloster
Copy link

@akloster akloster commented Jul 22, 2013

@almost Yes, I've thought about this tradeoff. I think it might be worth the trouble, especially with sourcemaps as a help, and this only affects generator functions, but it's not up to me to decide this. Another option is to offer an es6 mode in coffeescript and then use an es6 to es3 compiler downstream for those who need it.

@almost
Copy link
Author

@almost almost commented Jul 22, 2013

I like that as an option (having an es6 to es3 compiler), it reduces duplication of work. Looks like traceur does now support generators (at the cost of producing a massive state-machine instead of normal JS code):

http://bit.ly/119f7sr

@almost
Copy link
Author

@almost almost commented Jul 22, 2013

Another question, should CoffeeScript have generator expressions (which right now would compile to generator functions) and if so what would the syntax look like?

@bjmiller
Copy link

@bjmiller bjmiller commented Aug 5, 2013

I would expect ->* to result in an ordinary ES6 generator.

@almost
Copy link
Author

@almost almost commented Aug 5, 2013

@bjmiller The approach I've taken here is to detect generators by the presence of the yield keyword (as Python does). I think this fits with CoffeeScript's approach a little better than having a special function syntax but this is definitely something that could be argued either way.

@bgw
Copy link

@bgw bgw commented Aug 7, 2013

@almost, I don't have much experience to back this, but I feel like your approach of automatic detection would be better and more coffeescript-y.

@bjmiller
Copy link

@bjmiller bjmiller commented Aug 7, 2013

After thinking about it for a while, I think that I would prefer it to be more explicit. The idea being that it should be easy to see looking at the code and have the difference jump out at you as you're reading it. You're right about one thing, though. There are definitely good arguments for either side.

@ricardobeat
Copy link
Contributor

@ricardobeat ricardobeat commented Aug 9, 2013

is there any interest in having a shortcut for yield? like so (in lieu of backcalls)

app.run (request) ->
  body = <- request.parseContent()
  <- sleep(200)
  JSON.stringify(body)
@bgw
Copy link

@bgw bgw commented Aug 9, 2013

@ricardobeat, I think the shortcut syntaxes you present trade readability for characters. It seems like something livescript would do, but I don't think it would be a good fit for a syntactically minimal language like coffeescript.

@MaxDesiatov
Copy link

@MaxDesiatov MaxDesiatov commented Aug 15, 2013

Is this PR available anywhere rebased on top of 1.6.3? Pulling @jashkenas/coffee-script/master fails with conflicts :(

@nfour
Copy link

@nfour nfour commented Dec 20, 2013

I think something just needs to be decided on already and implemented, and 1.7.0 needs to be released with yield and the new chaining syntax asap.

@mklement0
Copy link
Contributor

@mklement0 mklement0 commented Dec 20, 2013

@xixixao: It's actually function* ..., e.g.: function* fibonacci() ... - see http://wiki.ecmascript.org/doku.php?id=harmony:generators

Agreed that CS doesn't have to mimic JS, but if the same symbol (*) IS used, it should go in the analogous place; otherwise you're inviting confusion.

@alubbe
Copy link
Contributor

@alubbe alubbe commented Dec 20, 2013

@gunta @mklement0 @MadRabbit @slezica come over to 'vote' for your syntax at this more up to date PR #3240

@mklement0
Copy link
Contributor

@mklement0 mklement0 commented Dec 20, 2013

Thanks, @alubbe.

@mikesmullin
Copy link

@mikesmullin mikesmullin commented Jun 14, 2014

what's the status here? still at the cryptic syntax argument stage? how about ()=*> the diamond dick?

btw es6 featuresets are here. we need to get support for let const yield function* and the real for of added or just get coffeescript out of the way and let us inject keywords wherever we want using backticks or macros.

we can start supporting the features everybody agrees on today. we don't need to wait for perfection. we're losing mindshare. this should be implemented by December 2014.

until then there is BlackCoffee, or...shudder....JavaScript.

@xixixao
Copy link
Contributor

@xixixao xixixao commented Jun 15, 2014

I wholeheartedly agree we need to get these features implemented (the ones which are not provided by CS already). I think there should be a version of CS which includes the ES6 features but can compile to JS runnable in old IE (current main branch of CS must). Whether this will be in the main repo or in a fork (CS nightly) will largely depend on Jeremy. The main problem is time. Whoever is willing to implement these features will surely be supported by the community (but I don't see many people lining up).

@xixixao
Copy link
Contributor

@xixixao xixixao commented Jun 15, 2014

To be clear, the ones I know of we need are export, yield, for..of. Any others?

@mikesmullin
Copy link

@mikesmullin mikesmullin commented Jun 15, 2014

compare the featureset here with green labels "Consensus":
https://developer.mozilla.org/en-US/docs/Web/JavaScript/ECMAScript_6_support_in_Mozilla

to your build of node.js here:
node --v8-options | grep harmony

note that the stable 0.10.x branch has fewer features than the latest nightly 0.11.x branch, as discussed here:
http://stackoverflow.com/questions/17379277/destructuring-in-node-js

there is also a nice one-pager here:
http://espadrine.github.io/New-In-A-Spec/es6/

@xixixao
Copy link
Contributor

@xixixao xixixao commented Jul 1, 2014

Looking over the one-pager I don't see any other features CS doesn't have already (+ classes should be compatible).

@mkoryak
Copy link

@mkoryak mkoryak commented Jul 2, 2014

I think the burden should be put on the developers to make sure that they dont write coffeescript that will compile down to es6 if they want that javascript to run browsers that dont support it.
It has always been this way with all other javascript features... want to use the File API? Go ahead, but just make sure you don't deploy that code to <IE9

Maybe implement this feature to only work when passing --harmony-generators to the coffeescript compiler?

Personally I love using coffeescript for nodejs development and I am sad that crappy browser support has found a way to make my life more difficult even there.

@iki
Copy link

@iki iki commented Aug 1, 2014

@xpepermint
Copy link

@xpepermint xpepermint commented Aug 4, 2014

status on this feature?

@nicohvi
Copy link

@nicohvi nicohvi commented Aug 14, 2014

+1

1 similar comment
@DylanPiercey
Copy link
Contributor

@DylanPiercey DylanPiercey commented Aug 15, 2014

+1

@skrat
Copy link

@skrat skrat commented Aug 15, 2014

@jashkenas There wasn't any feedback from the devs for over 7 months on this issue. It certainly would be nice and warmly welcomed if someone could give one.

@darthmaim
Copy link

@darthmaim darthmaim commented Aug 15, 2014

@skrat check this more active pull request with feedback from the devs: #3240

@GabrielRatener
Copy link
Contributor

@GabrielRatener GabrielRatener commented Aug 29, 2014

Shouldn't support for async/await be added as sugar ontop of yield with promises. This would really make async programming easy. Any ideas?

I like Dart's draft specification which also includes support for async gennerators and a special for loop to iterate asynchronously through streams produced by such gennerators.

Dart's spec: "https://www.dartlang.org/docs/spec/Asyncdraft-TC52.pdf"

@Artazor
Copy link
Contributor

@Artazor Artazor commented Oct 8, 2014

@GabrielRatener I think, syntactic sugar is not necessary. For example, with so library the example from so could be rewritten as follows:

so = require 'so'
fs = require 'then-fs'

readJSON = so (path) ->* 
    JSON.parse yield fs.readFile path, 'utf8'

main = so ->*
    a = yield readJSON 'a.json'
    b = yield readJSON 'b.json'
    console.log {a,b}

main().catch (e) ->*
    console.log e.stack ? e.message ? e 

Note that I'm against inferring generators from yield usage. It should be explicit other syntax. I think ->* and =>* are the best choices.

@vendethiel vendethiel closed this Oct 8, 2014
@vendethiel
Copy link
Collaborator

@vendethiel vendethiel commented Oct 8, 2014

This has been added already :)

@Artazor
Copy link
Contributor

@Artazor Artazor commented Oct 8, 2014

Oh! Great news! Can you point to any docs? Did generators landed in 1.8.0?

@vendethiel
Copy link
Collaborator

@vendethiel vendethiel commented Oct 8, 2014

#3240 for now!

@Artazor
Copy link
Contributor

@Artazor Artazor commented Oct 8, 2014

Hm... I see.

And I'm disappointed.

Decision to infer generatorness from yield yields to orthogonality violation and will be a "Bad Part" of the CoffeScript itself, that will be compensated with unnecessary dirty hacks in third-party libraries (that is against the spirit of CoffeeScript).

For example lets consider so — the tiny and very generic co-routine library. The following js snippet is semantically homogenous and correct:

var so = require('so');
var fs = require('then-fs');

var case1 = so(function*(){
    var value = 'Immediate';
    return value;
});

var case2 = so(function*(){
    var value = yield fs.readFile('somefile.txt', 'utf8');
    return value;
});

var combined = so(function*(){
    var a = yield case1();
    var b = yield case2();
    return {a:a, b:b};
});

var main = so(function*(){
    console.log(yield combined());
});

main().done();

This is achieved because so has signature described as follows:

type PromiseGen a = a -> [Promise] -- consider generators that yields Promises
so::PromiseGen a -> (a -> Promise) -- convert generator to the function that returns a Promise
-- Here monadic nature of Promise is omitted for simplicity

In contrast, the following CoffeScript counterpart will fail without changing the semantics of so()

so = require 'so'
fs = require 'then-fs'

case1 = so -> 'Immediate' # fails since argument is not a GeneratorFunction
case2 = so -> yield fs.readFile 'somefile.txt', 'utf8'

combined = so -> 
    a = yield case1()
    b = yield case2()
    {a, b}

main = so -> console.log yield combined()

main().done();

I understand that this is too late to argue, but who knows. It would be nice to hear some words from @jashkenas and @ForbesLindesay, because I believe that co, so or similar generator transformer will become a mainstream soon, and chosen principle effectively spoils the implementation (the unnecessary check for argument's type to be a GeneratorFunction).

Generators without yield are strange in the case of generating sequences (i.e. using generators as intended), but are pretty valid corner case for asynchronous flow control based on generators.

I think so.

see Artazor/so#2

@jashkenas
Copy link
Owner

@jashkenas jashkenas commented Oct 8, 2014

@Artazor — You didn't read closely enough.

case1 = so -> yield return 'Immediate'

Voila.

@Artazor
Copy link
Contributor

@Artazor Artazor commented Oct 8, 2014

@jashkenas

That matters, thank you very much!
Btw, interesting solution!

Waiting for yield support on npm.

@lydell
Copy link
Collaborator

@lydell lydell commented Oct 8, 2014

Waiting for yield support on npm.

npm install jashkenas/coffeescript

@GabrielRatener
Copy link
Contributor

@GabrielRatener GabrielRatener commented Oct 8, 2014

yield will be limited in use until we figure out how to handle expression closures.

See bug #3665

@micchyboy
Copy link

@micchyboy micchyboy commented Oct 10, 2014

Guys I need help. I'm following a book (Node.js the Right Way) and there's a part where generators are used, but whenever I run the file with (node --harmony countdown.js) it stil throws a (SyntaxError: Unexpected token *). Any suggestions on this? My version is v0.10.32 :(

@xpepermint
Copy link

@xpepermint xpepermint commented Oct 10, 2014

@micchyboy you are using the wrong version of node :). Use 0.11.9+.

@micchyboy
Copy link

@micchyboy micchyboy commented Oct 11, 2014

@xpepermint Yes thats the solution :). Thank you so much

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet