tj / co

The ultimate generator based flow-control goodness for nodejs (supports thunks, promises, etc)
MIT License
11.88k stars 788 forks source link

possible to yield Map object? #299

Open siyangbi opened 8 years ago

siyangbi commented 8 years ago

Hi I got this error, when yield a Map object,

'You may only yield a function, promise, generator, array, or object, but the following object was passed: "[object Map]"'

Is that possible to support Map?

Thanks

freiit commented 8 years ago

Hi,

Workaround would be:

yield Array.from(map.values())

\C

Am 05.10.2016 um 02:33 schrieb Siyang Bi notifications@github.com:

Hi I got this error, when yield a Map object,

'You may only yield a function, promise, generator, array, or object, but the following object was passed: "[object Map]"'

Is that possible to support Map?

Thanks

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

DeShadow commented 8 years ago

Or yield [...map.values()]

harshitgupta commented 8 years ago

Update: This solution has some unnecessary overhead. Please have a look at @DeShadow comment.

It works perfectly fine with .map.

For Example:

const result = yield array.map(x => task(x));

Where task() returns a promise and looks like:

screen shot 2016-10-13 at 6 41 24 pm

or even more simpler

screen shot 2016-10-13 at 6 43 00 pm

thus .map returns an array of promises. once all the promises are fulfilled result will be referring to array of all the bs

DeShadow commented 8 years ago

@harshitgupta Your code is a bit overhead. co(function*(){...}) returns Promise ;) That's why much simpler:

const task = x => co(function*(){
    const a = yield someTask(x);
    const b = yield someTask(y);
   return b;
});

const result = yield array.map(x => task(x))
// or simpler const result = yield array.map(task)

But co have function wrap. That's why the best solution is:

const task = co.wrap(function*(x){
   //somework
   return b;
});

const result = yield array.map(task);
harshitgupta commented 8 years ago

Awesome!!! Thanks a lot for pointing it out.

I didn't know that.

I am little bit confused about the error handling. I have catch CB with my version of co. Would it just be like:

.catch(error => error);

It doesn't seem right. :/

How would you handle it?

Thanks again.

DeShadow commented 8 years ago

@harshitgupta If you use co(function*(){ ... }):

co(function*(){
...
}).catch(err => console.error(err));

If you use co.wrap:

let handler = co.wrap(function*(p){
try {
...
} catch (err) {
console.error(err);
}
});

But if you don't want to catch errors in function:

let handler = co.wrap(function*(p){ ... });
handler(123).catch(err => console.error(err));
vaz commented 8 years ago

OP is about Map (the type), not Array.prototype.map).

+1, Map is even more appropriate than object for this.

In fact, "array or object" should really just be "iterable", as in, yielding an iterable for parallel execution.

I actually think this is a pretty big deal : co should be symmetrical with the newer builtin APIs and syntaxes like Map and Set (and weak variants), as well as for-of and yield* (it would be natural to want to yield* promises from an iterable in a co-generator)... and especially symmetry with Promise.all which is very relevant for parallel execution.

Objects are not actually iterables, though:

> for (let x of {});
TypeError: (intermediate value)[Symbol.iterator] is not a function

Dropping support for yielding objects would be a big breaking change, but it's not a big deal to say "iterables and plain objects". I would however suggest that non-iterable objects with a prototype other than Object or null should not be supported at all (not sure if that's the case now or not)