wish: support yield array

[skyblue]

something like that

yield [foo1(), foo2(suspend.resume())]  // foo1/2 could be  "thunkify", promise or  callback style with suspend.resume

should be equal

foo1(suspend.fork())
foo2(suspend.fork())
yield suspend.join()

sorry for my poor English~

[DylanPiercey]

+10

[dashed]

+1. https://github.com/tj/co#yield-array

Should be easy to implement.

Yielding and object would be nice too: https://github.com/tj/co#yield-object

[jmar777]

Ok, I cave :)

The next version of suspend is most likely going to be 1.0.0, as a result of some relatively substantial and breaking changes to the API. I'm just going to dump some of these thoughts here, as I would love feedback:

• No more thunks. With Promises being native in ES6, I don't see their added value anymore. Open to counter-arguments here, but I want to generally slim things down.
• Slim down the top-level API. E.g., suspend.run() is redundant. I want to find the leanest possible API that supports resolving to callbacks and promises, but is less imposing w/ callback conventions, to make it more relevant in browser contexts.
• Beef up concurrency and iteration utilties within the generator function (i.e., more helpers along the lines of .fork() and .join()).
• And, yes, add support for yielding arrays and objects. :)

I get API-design anxiety, so please hit me with any thoughts/suggestions/criticisms you have!

[dashed]

Agreed, there is already a PR at co to remove thunks and support async/await style functions using promises: https://github.com/tj/co/pull/154

Here are my thoughts:

• Remove thunk support.
• suspend(fn*) should return a promise by default; then deprecate suspend.promise(fn*).
• From above, you may keep suspend.fn(fn*). Though I don't really see a point to keep this, since anything that doesn't consume function return values would be fine to just use suspend(fn*) that return a promise anyways.
• Deprecate suspend.run. Use suspend.async instead.
• "Smarter" suspend.resumeRaw(). If there is only 1 argument, return that value, instead of inside an array. Otherwise, to keep it as an array, use suspend.resumeRaw(true).

[jmar777]

@Dashed Thanks for taking some time to brainstorm on this. I'm initially agreeing with most of that, but with a few changes.

Rather than starting with how to morph the current API into something new, I wanted to quickly identify what are the different kinds of GeneratorFunction wrappers that are absolutely required. I think we really just have 3:

• One that wraps a GeneratorFunction, with the resulting function returning a Promise.
• One that wraps a GeneratorFunction, with the resulting function transparently working with node-style callbacks.
• One that wraps a GeneratorFunction, and doesn't care about Promises or callbacks.

I was initially in favor of your suggestion to have suspend(fn*)() return a Promise, but I have at least one major reservation with that. Unlike, say, Bluebird Promises, where unhandled rejections get thrown during the next turn on the event loop, standard ES6 Promises will just remain silent. If there's a way to use ES6 Promises, and somehow enforce the Bluebird-like behavior on them before returning the Promise (help, @spion?), I'm still in favor of your suggestion here. I'm just not comfortable with making the most-often used API method (suspend(fn*)) swallow errors by default.

That error-swallowing behavior is also why I don't think we can just view suspend.fn(fn*) as being redundant with suspend.promise(fn*). It's true that normally consumers won't care if you return a Promise that they don't intend to use, but they will care if errors just disappear.

Anyway, going back to those basic needs, currently we can address them (respectively) with the following:

• suspend.promise(fn*)
• suspend.async(fn*)
• suspend.fn(fn*)

So that quickly and easily eliminates suspend.run(), like you said.

Regarding suspend.async(fn*), it still serves an important purpose in node-land, but in retrospect I think I named it poorly:

• With async/await coming in ES7 (which are structured around Promises), I don't want there to be confusion in thinking that suspend.async() is a parallel of that.
• In the interest of making suspend useful in the browser, I don't want to push a node-only convention on a generic term like "async", either.

So, suspend.async() may need to be deprecated under that name, but still lives on under a new name. I just don't have any brilliant ideas on what that name should be. We have good terms for Promises, thunks, and callbacks, but no good term for "a function that conforms to node-style callback conventions", unfortunately.

So, to summarize, that leaves us with:

• Deprecate suspend.run(fn*)
• Remove support for thunks entirely
• Figure out if we can deprecate suspend.promise(fn*) in favor of suspend(fn*), dependent on figuring out how to address unhandled rejections.
• Find a better name for suspend.async(fn*)
• Looks like suspend.fn(fn*) makes it through unscathed, miraculously.
• Add support for yielding arrays and objects (should be easy... they just piggy back on fork()/join() internally.

Regarding suspend.resumeRaw([bool]), I'll need to dedicate a little more brain power to that tomorrow. :)

So, progress?

[jmar777]

@Dashed K, after reading through copious amounts of es-discuss threads, I think I've decided not to sweat the unhandled rejections too much. This is a known property of Promises, and the current TC39 consensus (along with support from vendors) is to have engines track them for you, and notify the developer through, e.g., console logging.

I still find the lack of a promise.onUnhandledRejection(cb) API to be rather conspicuous, but for now I'll take it on faith that the situation will improve especially considering that async/await will rely on Promises as well. Either the community will make enough noise for TC39 to address it, or it'll prove to not be as big of an issue as I'm imagine.

So, long story slightly less long, that means I'm good with suspend(fn*)() returning a Promise now, which leaves us with the following:

• suspend(fn*): Wraps fn* with a function that returns a Promise.
• suspend.callback(fn*): Wraps fn* with a function that accepts a callback.
• suspend.fn(fn*): Wraps fn* with a function that doesn't care about Promises or callbacks.

Note the rename from suspend.async(fn*) -> suspend.callback(fn*). Still hoping to arrive at a better name than than before the next release, but I do want the name to imply that it's specific to the node callback convention, and not something to be generally used in other contexts like the browser.

All in all, though, I'm happy with reducing the top-level wrapper APIs down to just 3 methods.

[spion]

Not sure if we're likely to see console logging in node any time soon. Chrome does log unhandled rejections to the console, but I don't think that node will add a mechanism for that :/

Edit: I must add that this is an assumption based on the fact that node core has never been interested about Promises. I'm not even sure how V8 unhandled promise rejection tracking works - I suspect it might be possible to write a native addon that adds rejection tracking/logging even if one doesn't come with node core

[jmar777]

That is just the worst.

[jmar777]

That might be sufficient motivation for something like:

var suspend = require('suspend')({ promiseConstructor: require('bluebird') });

[spion]

See my edit above :)

In any case, a configurable promise constructor could definitely do a good job during the transitional period, great idea! +1

[dashed]

Wow. I didn't know ES6 promises silently swallow unhandled errors. I hope that changes before ES6 lands.

suspend.resumeRaw([bool]) was only a suggestion; I'm fine with it explicitly returning an array.

suspend.callback(fn*) is a good rename; one would need to keep in mind of the node-style callback conventions. Bluebird has this API: .nodeify([Function callback] [, Object options])

Then you can have: suspend.nodeify(fn*). You'll be "nodeify-ing" a function generator into a normal function to be called with Node's callback conventions.

[dashed]

@jmar777 I really like: var suspend = require('suspend')({ promiseConstructor: require('bluebird') });

The reason being is that I use suspend along with Facebook's regenerator in client-side apps.

[jmar777]

@Dashed unfortunately it seems extremely unlikely that the silent swallowing behavior is going to change. For better or worse, ES6 Promises are designed to allow fulfillment and rejection handlers to be registered asynchronously for the entire life of the Promise. That's a handy feature, but has the obvious drawback of not knowing whether a rejection is truly unhandled until the Promise is about to be GC'd. And because GC behavior is implementation-specific and usually non-deterministic, TC39 opted not to introduce a non-deterministic .onUnhandledRejection(cb)-style API. I don't think that's quite a complete picture, but that seemed to be the gist of why the behavior is that way from what I could tell.

[jmar777]

@spion, @Dashed:

Getting the 1.0.0 API ready. In the interest of not using generic terms for node-specific callback conventions, what are your thoughts on making suspend.callback() variadic, such that it works as the wrapper and the "resume" factory? E.g.,

suspend.callback(function*() {
    var foo = yield db.get('foo', suspend.callback());
});

// also, wanting to make `suspend.cb` an alias:
suspend.cb(function*() {
    var foo = yield db.get('foo', suspend.cb());
});

[dashed]

Interesting. I like it. From a glance, it seems explicit that db.get would traditionally take a callback function. But at the same time, you'd have to keep in mind the usage convention of suspend.callback() in this instance. Unsure if others prefer the explicit name suspend.resume(), since it's the duality of suspend/yield.

Is this an alias for suspend.resume/suspend.resumeRaw, or are you renaming them?

I assume this is true, but would the following also work?

suspend.callback(function*() {

    var foo = yield db.get('foo', suspend.callback());

    var gen = function*() {};
    var bar = yield takesNodeStyleFunc('bar', suspend.callback(gen));
});

[jmar777]

Is this an alias for suspend.resume/suspend.resumeRaw, or are you renaming them?

At this point it would probably be a rename. I've been really fond of the suspend() / resume() terminology, but just like we're renaming suspend.async() to something callback-specific, suspend.resume() has the same problem of sounding more generic than it really is. Given that it really is just a node-style callback factory, suspend.callback() arguably makes more sense there than it does for the actual suspend.callback(fn*) wrapper method. But, alas, we still don't have a better name for the wrapper either. The main thing we gain here is that if you're trying to follow node's callback conventions for anything, it's always some form of suspend.callback().

Regarding suspend.resumeRaw(), I was tempted to refactor that into something more like suspend.callback({ raw: true }), but that feels a little too verbose (even with the suspend.cb alias). I'm now thinking a more ergonomic solution would be something like:

• suspend.callback() / suspend.cb(): node-style (only) callback factory
• suspend.continue(): a generic continuation that behaves like suspend.resumeRaw() but resolves to just the first value (as has often been requested).
• suspend.continue({ multi: true }): for those rare occasions where you actually need the original suspend.resumeRaw() behavior that resolves to an array.
• suspend.resume() / suspend.resumeRaw(): deprecated.

I assume this is true, but would the following also work?

Probably not. takeNodeStyleFunc() would accept an error-first callback, whereas suspend.callback(gen) would return a function that also accepts an error-first callback (but itself not having that same expectation). So, if takesNodeStyleFunc() were to supply an error as the first argument, then that would equate to something like suspend.callback(gen)(theError), and since the error would be the last parameter as well, then the function returned from suspend.callback(gen) would attempt to invoke theError as a callback. So, ultimately it would just be a TypeError.

[dashed]

If we have suspend.continue(options), can we also have suspend.continue({ multi: true, errorFirst: true }) for APIs that take callbacks of the following: callback(err, foo, bar)? Mayhaps, suspend.continue({ errorFirst: true }) can behave like suspend.callback()? Probably also need to consider weirder APIs that pass error as the last argument.

Then suspend.continue()/ suspend.continue({ multi: true }) would resolve to the first non-error value / value(s).

---

Regarding takesNodeStyleFunc, I had meant that takesNodeStyleFunc has some usage of the following: takesNodeStyleFunc('bar', fs.rmdir.bind(null, path)). It would pass a node-style callback internally and handle it appropriately. Albeit, I never saw this done in the wild.

Overall, my point was that I was wondering if one would be able to use suspend.callback(fn*) within a generator since it has two behaviours. In other words, if suspend.callback() can only be used as a callback factory within a generator.

[ItalyPaleAle]

@jmar777 I'm sorry for asking, but why are you thinking about removing thunks? I understand your "they're not necessary" logic, but I don't think I'm the only one having code that is using thunks. If thunk support doesn't cause troubles, why should it be removed? (unless, of course, it does cause troubles)

[DylanPiercey]

@EgoAleSum I think they are trying to ease the way for async/await.

[ItalyPaleAle]

@DylanPiercey forgive me for my ignorance, but how would having thunks affect that?

[jmar777]

@EgoAleSum, support for thunks was originally added for the sake of compatibility with a lot of the co-<whatever> wrapper libraries. At the same time, ES6 Promises weren't even spec'd at the time suspend was released. Given that even co is distancing itself from thunks in favor of Promises, and that Promises are spec'd now, it makes more sense to move (as @DylanPiercey said) in a more async/await style direction.

Realistically with async/await being immanent, the best favor that suspend can do for the community is encourage code that lets you then actually remove suspend in most cases (with minimal restructuring to do so). I think suspend will still have place, though, as async/await and Promises are going to remain pretty feature-light for the foreseeable future. There's probably more value in providing convenient abstractions around more complex workflows (similar to how caolan's async does, without introducing new control-flow primitives).

Towards that end, I'm even tempted to remove support for callbacks and move that into a shim/support library. However, support for thunks is still pretty easy and doesn't really hurt anything, and I'm not looking to pull the rug out from anyone, either. The more feedback I get that people still want thunk support, the more likely I'll be to leave it in... :)

[ItalyPaleAle]

@jmar777 I totally get your feedback. I honestly do like suspend a lot, and I prefer it over co because of thunk and Node.js-style callbacks support.

I'm working on a project that has recently been updated to Node.js 0.12 and so we are now allowed to use generators. However, the project is quite big and has been developed in months: as a consequence, there is still a lot of "legacy" (if you can call it so...) code that uses the traditional callbacks and some parts use thunks too. While all components we're writing now use generators, promises and more modern design principles, we are not looking forward refactoring the entire codebase. When I had the chance, I did personally refactor some parts if I had to make substantial changes. However, unless there's a specific reason for that, updating the code to remove "old" patterns would just be a waste of time.

Thus, I'd like to ask you to please reconsider keeping thunk support, unless they are actually preventing you from advancing the library adding new, interesting features.

I totally agree with your goals, but I would propose a more "soft" approach, writing in the documentation that certain patterns are to be considered outdated and thus are discouraged for new implementations, albeit still supported for compatibility reasons.

[bugs181]

@EgoAleSum My 2 cents would be to stick with a version of suspend that supports thunks. It sounds kind of absurd to leave an abstraction in because some old "legacy" code depends on it. Everything in time needs to be updated/upgraded. I'm in favor of reducing code complexity and clutter. Backwards compatibility shouldn't be a predisposition based on old code requirements