Skip to content

Conversation

@codebytere
Copy link
Member

This PR exposes Environment::kNodeContextTagPtr. This was introduced a while back in #19134, and then recently introduced into Environment::GetCurrent.

Electron uses Environment::GetCurrent in several places (such as here), and since kNodeContextTagPtr wasn't exposed, we began to run into undefined symbol errors.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Oct 15, 2018
@addaleax addaleax added the embedding Issues and PRs related to embedding Node.js in another project. label Oct 15, 2018
@addaleax
Copy link
Member

Uh … this is implementation details and I’d really like to avoid exposing it.

Environment::GetCurrent() isn’t public API either, fwiw.

How about exposing a public wrapper for Environment::GetCurrent() instead?

@addaleax
Copy link
Member

@codebytere Would you like to implement the suggestion above? I think it could be something like a Environment* GetNodeEnvironment(v8::Local<v8::Context>) function, maybe near GetCurrentEventLoop() in the hearders/source code?

@codebytere codebytere closed this Oct 15, 2018
@codebytere codebytere deleted the expose-kNodeContextTagPtr branch October 15, 2018 18:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. embedding Issues and PRs related to embedding Node.js in another project.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants