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

doc: update ObjectWrap example #754

Conversation

gabrielschulhof
Copy link
Contributor

  • Remove the global static reference to the constructor
  • Use Napi::Env::SetInstanceData to store the constructor, and
  • Add a static method that uses Napi::FunctionReference::New to
    create a new instance of the class by retrieving the constructor
    using Napi::Env::GetInstanceData and using
    Napi::FunctionReference::New to create the new instance.

Fixes: #711

@gabrielschulhof gabrielschulhof force-pushed the issue-711-update-objectwrap-doc branch from 28e53d1 to 6252e48 Compare June 29, 2020 19:58
Copy link
Member

@NickNaso NickNaso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one thing to do. The rest seems good.

// Retrieve the instance data we stored during `Init()`. We only stored the
// constructor there, so we retrieve it here to create a new instance of the
// JS class the constructor represents.
Napi::FunctionReference* constructor = info.Env().GetInstanceData<Example>();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be:

Napi::FunctionReference* constructor = info.Env().GetInstanceData<Napi::FunctionReference>();

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right! Good catch! Thanks!

* Remove the global static reference to the constructor
* Use `Napi::Env::SetInstanceData` to store the constructor, and
* Add a static method that uses `Napi::FunctionReference::New` to
  create a new instance of the class by retrieving the constructor
  using `Napi::Env::GetInstanceData` and using
  `Napi::FunctionReference::New` to create the new instance.

Fixes: nodejs#711
@gabrielschulhof gabrielschulhof force-pushed the issue-711-update-objectwrap-doc branch from 6252e48 to e095c01 Compare July 1, 2020 17:49
Copy link
Member

@NickNaso NickNaso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

gabrielschulhof pushed a commit that referenced this pull request Jul 2, 2020
* Remove the global static reference to the constructor
* Use `Napi::Env::SetInstanceData` to store the constructor, and
* Add a static method that uses `Napi::FunctionReference::New` to
  create a new instance of the class by retrieving the constructor
  using `Napi::Env::GetInstanceData` and using
  `Napi::FunctionReference::New` to create the new instance.

Fixes: #711
PR-URL: #754
Reviewed-By: Nicola Del Gobbo <nicoladelgobbo@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@gabrielschulhof
Copy link
Contributor Author

Landed in ef16dfb.

@gabrielschulhof gabrielschulhof deleted the issue-711-update-objectwrap-doc branch July 2, 2020 16:01
artemp added a commit to mapnik/node-mapnik that referenced this pull request Aug 14, 2020
kevindavies8 added a commit to kevindavies8/node-addon-api-Develop that referenced this pull request Aug 24, 2022
* Remove the global static reference to the constructor
* Use `Napi::Env::SetInstanceData` to store the constructor, and
* Add a static method that uses `Napi::FunctionReference::New` to
  create a new instance of the class by retrieving the constructor
  using `Napi::Env::GetInstanceData` and using
  `Napi::FunctionReference::New` to create the new instance.

Fixes: nodejs/node-addon-api#711
PR-URL: nodejs/node-addon-api#754
Reviewed-By: Nicola Del Gobbo <nicoladelgobbo@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Marlyfleitas added a commit to Marlyfleitas/node-api-addon-Development that referenced this pull request Aug 26, 2022
* Remove the global static reference to the constructor
* Use `Napi::Env::SetInstanceData` to store the constructor, and
* Add a static method that uses `Napi::FunctionReference::New` to
  create a new instance of the class by retrieving the constructor
  using `Napi::Env::GetInstanceData` and using
  `Napi::FunctionReference::New` to create the new instance.

Fixes: nodejs/node-addon-api#711
PR-URL: nodejs/node-addon-api#754
Reviewed-By: Nicola Del Gobbo <nicoladelgobbo@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
wroy7860 added a commit to wroy7860/addon-api-benchmark-node that referenced this pull request Sep 19, 2022
* Remove the global static reference to the constructor
* Use `Napi::Env::SetInstanceData` to store the constructor, and
* Add a static method that uses `Napi::FunctionReference::New` to
  create a new instance of the class by retrieving the constructor
  using `Napi::Env::GetInstanceData` and using
  `Napi::FunctionReference::New` to create the new instance.

Fixes: nodejs/node-addon-api#711
PR-URL: nodejs/node-addon-api#754
Reviewed-By: Nicola Del Gobbo <nicoladelgobbo@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
johnfrench3 pushed a commit to johnfrench3/node-addon-api-git that referenced this pull request Aug 11, 2023
* Remove the global static reference to the constructor
* Use `Napi::Env::SetInstanceData` to store the constructor, and
* Add a static method that uses `Napi::FunctionReference::New` to
  create a new instance of the class by retrieving the constructor
  using `Napi::Env::GetInstanceData` and using
  `Napi::FunctionReference::New` to create the new instance.

Fixes: nodejs/node-addon-api#711
PR-URL: nodejs/node-addon-api#754
Reviewed-By: Nicola Del Gobbo <nicoladelgobbo@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
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

Successfully merging this pull request may close these issues.

Document that the ObjectWrap example code doesn't work for multi-context usage
3 participants