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

Building with Electron 13 does not work on Windows #782

Closed
andiliu-gh opened this issue Aug 30, 2021 · 4 comments · Fixed by #785
Closed

Building with Electron 13 does not work on Windows #782

andiliu-gh opened this issue Aug 30, 2021 · 4 comments · Fixed by #785

Comments

@andiliu-gh
Copy link
Contributor

andiliu-gh commented Aug 30, 2021

It seems as if building with Electron 13 is broken on Windows due to this: electron/electron#29893

Error from neon:

    neon.obj : error LNK2019: unresolved external symbol "__declspec(dllimport) public: class std::shared_ptr<class v8::BackingStore> __thiscall v8::ArrayBuffer::GetBackingStore(void)" (__imp_?GetBackingStore@ArrayBuffer@v8@@QAE?AV?$shared_ptr@VBackingStore@v8@@@std@@XZ) referenced in function _Neon_ArrayBuffer_Data [C:\Users\andyl\-\target\release\build\neon-sys-ab7d83299aae5b27\out\native\build\neon.vcxproj]
    C:\Users\-\target\release\build\neon-sys-ab7d83299aae5b27\out\native\build\Release\neon.node : fatal error LNK1120: 1 unresolved externals [C:\Users\-\target\release\build\neon-sys-ab7d83299aae5b27\out\native\build\neon.vcxproj]

I think it's cause neon uses nan or that it builds with msvc...? So the header signature is mismatched. The Electron issue link above mentions building with libc++ to avoid this issue or to replace nan.h w N-API?

@kjvalencik
Copy link
Member

kjvalencik commented Aug 31, 2021

I would be willing to merge a fix for this, but none of the core devs are Windows developers which would make this difficult.

However, Neon with the napi backend (the default when creating a project with npm init neon my-project) does not have this issue. That backend already uses Node-API (formerly N-API).

Since the legacy backend (neon-sys) is no longer being actively developed, it is unlikely that this will be fixed.

I'm happy to chat in our Slack channel about possible fixes, but I most likely won't have chance to work on it myself.

A couple possible solutions:

  • (Simple) If you don't need ArrayBuffer, would be to put it behind a feature flag so that users can disable it for Electron compatibility
  • (Complicated) Convert ArrayBuffer to Buffer and use their content APIs

@andiliu-gh
Copy link
Contributor Author

Oh I was totally unaware that we were still using the legacy backend. I assumed that since we were updated that it would be backed by the napi backend. I also asked my co-worker regarding that and that's what he concluded as well. I'll get back to you if we're still facing issues w the napi backend!

@kjvalencik
Copy link
Member

@andiliu-gh Let me know! We chose not to change the default backend on existing projects since it would needlessly cause breaking changes. We may add a compiler warning in the future (or default to no backend). Instead, new projects will default to the napi backend.

There are a bunch of breaking changes between the backends. The migration guide may be helpful.

@kjvalencik
Copy link
Member

@andiliu-gh I still highly suggest switching to the napi backend, but if you would like to try it, this may fix the issue:

#785

kjvalencik added a commit that referenced this issue Sep 13, 2021
…use Buffer to get the contents of an ArrayBuffer

Fixes #782
kjvalencik added a commit that referenced this issue Sep 13, 2021
…use Buffer to get the contents of an ArrayBuffer

Fixes #782
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 a pull request may close this issue.

2 participants