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.

Sign up for GitHub

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

Jump to bottom

Fix broken builds on Node.js 12.x due to changes to the V8 API. #80

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix broken builds on Node.js 12.x due to changes to the V8 API. #80

wants to merge 1 commit into from

Conversation

tylerc
Copy link

@tylerc tylerc commented Jul 23, 2019

I was attempting to use noble-uwp with Node.js v12.0.0 (actually, it was Electron v5.0.7 which is based on that), but some differences in the V8 API prevented it from compiling successfully.

I have made the following changes to get things working:

  • v8::Handle is now called v8::Local, all usages of v8::Handle have been updated.
  • A number of functions now require isolates, so we now pass in v8::Isolate::GetCurrent().
  • A number of functions now require a context, so we now pass in Nan::GetCurrentContext().
  • A number of functions now return Maybe, so we now call .FromJust() on those results to get the raw value rather than the Maybe.

As far as I can tell, things are working well with those changes.

The only problem I'm aware of is that I don't believe these changes will compile on older versions of Node.js, which isn't ideal... But I'm not sure how to work around that, short of creating two sets of mostly duplicated files in the repo or something.

I'm happy to answer any questions or make tweaks to these changes!

@tylerc
Fix broken builds on Node.js 12.x due to changes to the V8 API.
e3ff983 
The changes are:

- v8::Handle is now called v8::Local.
- A number of functions now require isolates, so we pass in v8::Isolate::GetCurrent()
- A number of functions now require a context, so we pass in Nan::GetCurrentContext()
- A number of functions now return Maybe<T>, so call .FromJust() on those results.
@jasongin
Copy link
Owner

All the code files changed in this PR are actually autogenerated by NodeRT. So I'd rather not merge this because the changes would all be lost if the code was generated again.

Unfortunately it looks like NodeRT is not being actively maintained any longer; I don't think anyone is updating it to generate code that works with Node 12. See also #4

@tylerc
Copy link
Author

tylerc commented Jul 31, 2019

@jasongin Makes sense to me. #4 would probably be better, but I don't have time at the moment to pursue that one personally. I'll leave my fork out there in case anyone using Node.js v12 wants to use noble-uwp like me, but otherwise feel free to close this PR if you don't think you'll ever merge it in.

@jasongin
Copy link
Owner

I noticed a fix was merged into NodeRT (months after we last looked at this), so we should be able to update that dependency and regenerate the code to resolve the issue.

@jasongin jasongin mentioned this pull request Apr 23, 2020
Closed
@yuerugou
Copy link

nodejs v12.16.0, electron v8.2.2
when I use npm rebuild noble-uwp --runtime=electron --target=8.2.2 --arch=x64 --rebuild --disturl=https://atom.io/download/electron --build_from_source=true, it was wrong
`noble-uwp@0.6.2 install E:\working\mbuilder\build\node_modules\noble-uwp

node install.js

Installing NodeRT UWP adapter for Windows.Foundation
�ڴ˽��������һ������һ����Ŀ����Ҫ���ò������ɣ������ӡ�/m�����ء�
_nodert_generated.cpp
NodeRtUtils.cpp
OpaqueWrapper.cpp
CollectionsConverterUtils.cpp
win_delay_load_hook.cc
C:\Users\mk10261\AppData\Local\node-gyp\Cache\8.2.2\include\node\v8.h(8923): warning C4996: 'v8::MicrotasksCompletedCallback': Use *WithData version. (compiling source file ..\NodeRtUtils.cpp) [E:\working\mbuilder\build\node_modules\noble-uwp\uwp\windows.foundation\build\binding.vcxproj]
c:\users\mk10261\appdata\local\node-gyp\cache\8.2.2\include\node\v8.h(8923): warning C4996: 'v8::MicrotasksCompletedCallback': Use *WithData version. (compiling source file ..\OpaqueWrapper.cpp) [E:\working\mbuilder\build\node_modules\noble-uwp\uwp\windows.foundation\build\binding.vcxproj]
c:\users\mk10261\appdata\local\node-gyp\cache\8.2.2\include\node\v8.h(7238): note: see declaration of 'v8::MicrotasksCompletedCallback' (compiling source file ..\OpaqueWrapper.cpp)
c:\users\mk10261\appdata\local\node-gyp\cache\8.2.2\include\node\v8.h(8931): warning C4996: 'v8::MicrotasksCompletedCallback': Use *WithData version. (compiling source file ..\OpaqueWrapper.cpp)C:\Users\mk10261\AppData\Local\node-gyp\Cache\8.2.2\include\node\v8.h(7238): note: see declaration of 'v8::MicrotasksCompletedCallback' (compiling source file ..\NodeRtUtils.cpp) [E:\working\mbuilder\build\node_modules\noble-uwp\uwp\windows.foundation\build\binding.vcxproj]

C:\Users\mk10261\AppData\Local\node-gyp\Cache\8.2.2\include\node\v8.h(8931): warning C4996: 'v8::MicrotasksCompletedCallback': Use *WithData version. (compiling source file ..\NodeRtUtils.cpp) [E:\working\mbuilder\build\node_modules\noble-uwp\uwp\windows.foundation\build\binding.vcxproj]
C:\Users\mk10261\AppData\Local\node-gyp\Cache\8.2.2\include\node\v8.h(7238): note: see declaration of 'v8::MicrotasksCompletedCallback' (compiling source file ..\NodeRtUtils.cpp)
c:\users\mk10261\appdata\local\node-gyp\cache\8.2.2\include\node\v8.h(7238): note: see declaration of 'v8::MicrotasksCompletedCallback' (compiling source file ..\OpaqueWrapper.cpp)
C:\Users\mk10261\AppData\Local\node-gyp\Cache\8.2.2\include\node\v8.h(8923): warning C4996: 'v8::MicrotasksCompletedCallback': Use *WithData version. (compiling source file .._nodert_generated.cpp) [E:\working\mbuilder\build\node_modules\noble-uwp\uwp\windows.foundation\build\binding.vcxproj]
C:\Users\mk10261\AppData\Local\node-gyp\Cache\8.2.2\include\node\v8.h(7238): note: see declaration of 'v8::MicrotasksCompletedCallback' (compiling source file .._nodert_generated.cpp)
C:\Users\mk10261\AppData\Local\node-gyp\Cache\8.2.2\include\node\v8.h(8931): warning C4996: 'v8::MicrotasksCompletedCallback': Use *WithData version. (compiling source file .._nodert_generated.cpp) [E:\working\mbuilder\build\node_modules\noble-uwp\uwp\windows.foundation\build\binding.vcxproj]
C:\Users\mk10261\AppData\Local\node-gyp\Cache\8.2.2\include\node\v8.h(7238): note: see declaration of 'v8::MicrotasksCompletedCallback' (compiling source file .._nodert_generated.cpp)
..\NodeRtUtils.cpp(109): warning C4996: 'Nan::MakeCallback': was declared deprecated [E:\working\mbuilder\build\node_modules\noble-uwp\uwp\windows.foundation\build\binding.vcxproj]
E:\working\mbuilder\build\node_modules\noble-uwp\node_modules\nan\nan.h(1047): note: see declaration of 'Nan::MakeCallback'
..\NodeRtUtils.cpp(189): warning C4996: 'Nan::ForceSet': was declared deprecated [E:\working\mbuilder\build\node_modules\noble-uwp\uwp\windows.foundation\build\binding.vcxproj]
e:\working\mbuilder\build\node_modules\noble-uwp\node_modules\nan\nan_maybe_43_inl.h(117): note: see declaration of 'Nan::ForceSet'
..\NodeRtUtils.cpp(268): warning C4996: 'Nan::ForceSet': was declared deprecated [E:\working\mbuilder\build\node_modules\noble-uwp\uwp\windows.foundation\build\binding.vcxproj]
e:\working\mbuilder\build\node_modules\noble-uwp\node_modules\nan\nan_maybe_43_inl.h(117): note: see declaration of 'Nan::ForceSet'
..\NodeRtUtils.cpp(273): warning C4996: 'Nan::ForceSet': was declared deprecated [E:\working\mbuilder\build\node_modules\noble-uwp\uwp\windows.foundation\build\binding.vcxproj]
e:\working\mbuilder\build\node_modules\noble-uwp\node_modules\nan\nan_maybe_43_inl.h(117): note: see declaration of 'Nan::ForceSet'
e:\working\mbuilder\build\node_modules\noble-uwp\uwp\windows.foundation\node-async.h(245): warning C4996: 'Nan::MakeCallback': was declared deprecated (compiling source file .._nodert_generated.cpp) [E:\working\mbuilder\build\node_modules\noble-uwp\uwp\windows.foundation\build\binding.vcxproj]
E:\working\mbuilder\build\node_modules\noble-uwp\node_modules\nan\nan.h(1047): note: see declaration of 'Nan::MakeCallback' (compiling source file .._nodert_generated.cpp)
e:\working\mbuilder\build\node_modules\noble-uwp\uwp\windows.foundation\node-async.h(285): warning C4996: 'Nan::MakeCallback': was declared deprecated (compiling source file .._nodert_generated.cpp) [E:\working\mbuilder\build\node_modules\noble-uwp\uwp\windows.foundation\build\binding.vcxproj]
E:\working\mbuilder\build\node_modules\noble-uwp\node_modules\nan\nan.h(1047): note: see declaration of 'Nan::MakeCallback' (compiling source file .._nodert_generated.cpp)
e:\working\mbuilder\build\node_modules\noble-uwp\uwp\windows.foundation\node-async.h(321): error C2661: 'v8::Object::Get': no overloaded function takes 1 arguments (compiling source file .._nodert_generated.cpp) [E:\working\mbuilder\build\node_modules\noble-uwp\uwp\windows.foundation\build\binding.vcxproj]
gyp ERR! build error
gyp ERR! stack Error: C:\Program Files (x86)\MSBuild\14.0\bin\MSBuild.exe failed with exit code: 1
gyp ERR! stack at ChildProcess.onExit (D:\Programs\nvm\nvm\v12.16.0\node_modules\npm\node_modules\node-gyp\lib\build.js:194:23)
gyp ERR! stack at ChildProcess.emit (events.js:321:20)
gyp ERR! stack at Process.ChildProcess._handle.onexit (internal/child_process.js:275:12)
gyp ERR! System Windows_NT 6.1.7601
gyp ERR! command "d:\Program Files\nodejs\node.exe" "D:\Programs\nvm\nvm\v12.16.0\node_modules\npm\node_modules\node-gyp\bin\node-gyp.js" "build" "--fallback-to-build" "--msvs_version=2015" "--module=E:\working\mbuilder\build\node_modules\noble-uwp\uwp\windows.foundation\binding\electron-v8.2-win32-x64\binding.node" "--module_name=binding" "--module_path=E:\working\mbuilder\build\node_modules\noble-uwp\uwp\windows.foundation\binding\electron-v8.2-win32-x64"
gyp ERR! cwd E:\working\mbuilder\build\node_modules\noble-uwp\uwp\windows.foundation
gyp ERR! node -v v12.16.0
gyp ERR! node-gyp -v v5.0.5
gyp ERR! not ok
node-pre-gyp ERR! build error
node-pre-gyp ERR! stack Error: Failed to execute 'd:\Program Files\nodejs\node.exe D:\Programs\nvm\nvm\v12.16.0\node_modules\npm\node_modules\node-gyp\bin\node-gyp.js build --fallback-to-build --msvs_version=2015 --module=E:\working\mbuilder\build\node_modules\noble-uwp\uwp\windows.foundation\binding\electron-v8.2-win32-x64\binding.node --module_name=binding --module_path=E:\working\mbuilder\build\node_modules\noble-uwp\uwp\windows.foundation\binding\electron-v8.2-win32-x64' (1)
node-pre-gyp ERR! stack at ChildProcess. (E:\working\mbuilder\build\node_modules\noble-uwp\node_modules\node-pre-gyp\lib\util\compile.js:83:29)
node-pre-gyp ERR! stack at ChildProcess.emit (events.js:321:20)
node-pre-gyp ERR! stack at maybeClose (internal/child_process.js:1021:16)
node-pre-gyp ERR! stack at Process.ChildProcess._handle.onexit (internal/child_process.js:286:5)
node-pre-gyp ERR! System Windows_NT 6.1.7601
node-pre-gyp ERR! command "d:\Program Files\nodejs\node.exe" "E:\working\mbuilder\build\node_modules\noble-uwp\node_modules\node-pre-gyp\bin\node-pre-gyp" "install" "--fallback-to-build" "--msvs_version=2015"
node-pre-gyp ERR! cwd E:\working\mbuilder\build\node_modules\noble-uwp\uwp\windows.foundation
node-pre-gyp ERR! node -v v12.16.0
node-pre-gyp ERR! node-pre-gyp -v v0.6.36
node-pre-gyp ERR! not ok
Failed to execute 'd:\Program Files\nodejs\node.exe D:\Programs\nvm\nvm\v12.16.0\node_modules\npm\node_modules\node-gyp\bin\node-gyp.js build
--fallback-to-build --msvs_version=2015 --module=E:\working\mbuilder\build\node_modules\noble-uwp\uwp\windows.foundation\binding\electron-v8.2-win32-x64\binding.node --module_name=binding --module_path=E:\working\mbuilder\build\node_modules\noble-uwp\uwp\windows.foundation\binding\electron-v8.2-win32-x64' (1)
noble-uwp@0.6.2 E:\working\mbuilder\build\node_modules\noble-uwp`

what can i do to fix this, thank you very much

@acstacey acstacey mentioned this pull request Sep 24, 2020
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tylerc @jasongin @yuerugou