developit / greenlet

🦎 Move an async function into its own thread.
https://npm.im/greenlet
4.67k stars 100 forks source link

Added Generators and Async Generators support #50

Open johnsonjo4531 opened 4 years ago

johnsonjo4531 commented 4 years ago

I created this as a draft pull request, so that you (the maintainers) would know I'm willing to rewrite a large portion of it due to feedback. Feel free to make any edits, give any suggestions or the like.

I tried to make the async generator returned from the greenlet function act as much like an async generator would if it were just a normal one without being in the webworker. If I missed some part that could make it compatible with generator functions it would be nice to have some tests. I sadly added around double the lines getting this done, so if anyone has any ideas on how to make it shorter that would be nice.

Also I'm not sure if I broke IE in the process of creating this, because I use someFunctionInstance.constructor.name === "GeneratorFunction" or "AsyncGeneratorFunction" to check if a function is a generator or async generator function, and apparently that doesn't always work well for transpiled code. The other way to know if it's a generator function is by it's return value, but I'm not sure the best way to do that. We could pass a message and then execute it to find out it's return value (checking whether it has property Symbol.iterator or Symbol.asyncIterator should be good enough), but then we would have to return a promise of the async generator function, but then it would also work for other types of generators.

Also FYI: there's this error/warning that pops up when I run the tests and I'm not sure if I need to fix that or how to:

Warning: React version not specified in eslint-plugin-react settings. See https://github.com/yannickcr/eslint-plugin-react#configuration . SyntaxError: The keyword 'yield' is reserved (100:57) 96 : result = $await_7; 97 : if (result.done) { 98 : return [1]; 99 : } 100 : value = (yield result.value);

COMMIT MESSAGE:

fixes https://github.com/developit/greenlet/issues/35

tldr; Mainly wrapped the existing promise api to get it to work with async generators. I took atleast two iterations to get to this point. At first I thought to do the job with a readable Stream implementing async iterable on the main thread, but then was afraid of inconsistencies that would arise between the two apis. For example Readable Stream when finished will only return { done: true, value: undefined } whereas async iterables can return { done: true, value: any } when any is any value.

So, then I decided to make a async generator that could talk to the worker for better compatibility. One thing to note is that the worker data onmessage receives an extra piece for the status to cause the iterator to use. This is similar to the Promise status, but for generators.

johnsonjo4531 commented 4 years ago

Also, I added an outward API change with an options object as the second argument to the greenlet function. In hindsight this probably should have been added in a seperate pull-request. It only has one option for now which allows you to turn transferables off. This could be useful if you didn't want to transfer the transferables zero-copy although it seems it might be a bit limiting that way for the moment, because you can't turn it back on between calls. The way it is now you could always copy the transferable yourself before you send it, so it doesn't disappear on your side of the worker divide.

johnsonjo4531 commented 4 years ago

Actually I decided not to make this a draft for the reason that I'm sometimes slow to respond to github notifications, as I use bitbucket at work. So, still feel free to comment or edit, but I'm marking this as ready for review if you all are fine with the way it is.

johnsonjo4531 commented 4 years ago

By the way I think this error is the bundler so you might not want to get this merged until that's fixed, otherwise there will be no code output. I gave edit rights to my fork to maintainers incase any of you know how to fix it:

Warning: React version not specified in eslint-plugin-react settings. See https://github.com/yannickcr/eslint-plugin-react#configuration .
SyntaxError: The keyword 'yield' is reserved (100:57)
96 : result = $await_7;
97 : if (result.done) {
98 : return [1];
99 : }
100 : value = (yield result.value);
johnsonjo4531 commented 4 years ago

I was able to get around the bundler issue by the way by not using the yield keyword, but instead using the iterator protocol. Let me know, if you need anything else done on this or whatever you decide to do with it is fine with me 👍.