Added Generators and Async Generators support #50
Conversation
fixes developit#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.
|
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. |
|
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. |
|
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:/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); |
fixes developit#38 First thing to note about this is that I broke the microbundle build. I'll attempt to fix the generator function to not use things microbundle doesn't expect, but I'm not sure how far I'll get with that. Which that's also something tying up this pull request: developit/greenlet#50 One other thing to note about this is that I decided on returning a promise of the asyncIterator from the generator function. This is due to the fact that the function from the user only ever gets evaluated as an executable function on the worker side.
Microbundle wouldn't build, because the previous commits used async generator syntax. This is now fixed since we are now using the async iterator protocol instead.
|
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 👍. |
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:
COMMIT MESSAGE:
fixes #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
anyis 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.