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

Create example for AsyncProgressWorker & AsyncProgressQueueWorker #164

Open
mastergberry opened this issue Nov 4, 2020 · 4 comments
Open

Comments

@mastergberry
Copy link

There is currently on example for these progress workers. The documentation is kind of a bit confusing here: https://github.com/nodejs/node-addon-api/blob/master/doc/async_worker_variants.md

It doesn't really explain how to use the progress callback in the OnProgress function...just randomly makes a Callback().Call({Env().Null(), Env().Null(), Number::New(Env(), *data)}); call with two Env().Null()'s with no explanation as to what this is doing. I guess by passing two nulls it forces this to the Progress callback in javascript...but it's all a bit confusing.

Would be nice to get a proper example on here put together for these progressworkers. In the nan library you have to specify each the callback and the progresscallback to the worker, but it doesn't seem to be the case in the new node-addon-api, example/test can be found here: https://github.com/nodejs/nan/blob/e222068f9d99967897a8cefa6c505534b36116ad/test/cpp/asyncprogressworker.cpp

I am porting a project from nan -> node-addon-api currently, and feel an example on how the progress callbacks work would be beneficial to me and others also.

Thanks.

@mastergberry
Copy link
Author

Upon a bit of further inspection of the source code, I found a similar test which shows something that makes more sense on how to use the async progress workers: nodejs/node-addon-api@ab01844#diff-3f536315f565c436ece66924a9d8073b7e0aa43366dedf4cda5ac9ac1a1e991eR18

Seems like the documentation is wrong and out of date possibly. Makes more sense now that I see two Function's being passed. Still think an example in this repo would not hurt.

@mhdawson
Copy link
Member

mhdawson commented Nov 5, 2020

@mastergberry any chance you would want to submit a PR to fix the documentation?

@mastergberry
Copy link
Author

Hey @mhdawson

I have gone ahead and created a PR to clean up the documentation. Maybe later if I get some extra time I will convert this into a proper example here as well (or someone else can feel free to if they wish as well).

nodejs/node-addon-api#831

@mhdawson
Copy link
Member

mhdawson commented Nov 9, 2020

@mastergberry thanks for doing that. PR is landed :)

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

No branches or pull requests

2 participants