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

napi-asyncworker-example missing scope #142

Closed
rynz opened this issue Jun 13, 2020 · 3 comments
Closed

napi-asyncworker-example missing scope #142

rynz opened this issue Jun 13, 2020 · 3 comments

Comments

@rynz
Copy link

rynz commented Jun 13, 2020

void SimpleAsyncWorker::OnOK() {
std::string msg = "SimpleAsyncWorker returning after 'working' " + std::to_string(runTime) + " seconds.";
Callback().Call({Env().Null(), String::New(Env(), msg)});
};

This seems to be the only example that doesn't call:

Napi::HandleScope scope(Env());

Should this example include that too?

@gabrielschulhof
Copy link
Contributor

@rynz actually, it should be the other way around. Node.js ensures that all scopes are present by the time the OnOK/OnError callbacks get called. Thanks for noticing the discrepancy!

We have additional documentation here: https://github.com/nodejs/node-addon-api/blob/master/doc/object_lifetime_management.md

@rynz
Copy link
Author

rynz commented Jun 15, 2020

@gabrielschulhof thank you!

@gotnone
Copy link

gotnone commented Jul 4, 2020

Can someone who is more familiar with this than I am check to see if there is still bad example code in node-addon-api/doc/async_worker.md It appears that the OnOK example is using the Napi::HandleScope when it is not necessary.

Thanks

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

3 participants