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

Fixes for Node 12. #410

Merged
merged 1 commit into from
Aug 19, 2019
Merged

Fixes for Node 12. #410

merged 1 commit into from
Aug 19, 2019

Conversation

kjvalencik
Copy link
Member

Resolves #409. There are still warnings that should be resolved. Especially, MakeCallback, which will break domain support.

@kjvalencik
Copy link
Member Author

Failed build, but it worked locally for me. This may require bumping the nan version.

@Abdillah
Copy link

It worked with latest node v12.0 and build-able locally. Unfortunately, the resulting lib (after compiling the neon-runtime) still contains missing symbols (as the original bug report stated). Any hint? I am interested in helping.

$ /home/user/.nvm/versions/node/v12.0.0/bin/electron ../lib/index.js
Error: /home/user/workspace/neonproject/native/index.node: undefined symbol: _ZN2v811HandleScope12CreateHandleEPNS_8internal7IsolateEPNS1_6ObjectE
    at process.func [as dlopen] (electron/js2c/asar.js:155:31)
    at Object.Module._extensions..node (internal/modules/cjs/loader.js:850:18)
    at Object.func [as .node] (electron/js2c/asar.js:155:31)
    at Module.load (internal/modules/cjs/loader.js:677:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:609:12)
    at Function.Module._load (internal/modules/cjs/loader.js:601:3)
    at Module.require (internal/modules/cjs/loader.js:715:19)
    at require (internal/modules/cjs/helpers.js:14:16)
    at Object.<anonymous> (/home/user/workspace/lib/index.js:4:15)
    at Module._compile (internal/modules/cjs/loader.js:808:30)

Versions

$ /home/user/.nvm/versions/node/v12.0.0/bin/electron -v
v5.0.2
$ node -v
v12.0.0
$ npm -v
6.9.0

@Abdillah
Copy link

Oh, seems my case above is a runtime problem. Using this patch compilation is working fine. But, something in my system prevent the module from being loaded successfully. Thanks.

Resolves neon-bindings#409. There are still warnings that should be resolved. Especially, `MakeCallback`, which will break domain support.
@kjvalencik
Copy link
Member Author

@Abdillah Thanks for the reminder that I needed to update my PR to include the latest nan!

@kjvalencik
Copy link
Member Author

@dherman Can we merge / release this?

Copy link
Collaborator

@dherman dherman left a comment

Choose a reason for hiding this comment

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

:shipit: !

@kjvalencik kjvalencik merged commit 440f928 into neon-bindings:master Aug 19, 2019
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.

Unresolved External Symbols with Node 12
3 participants