search

electron-userland / electron-spellchecker

Implement spellchecking, correctly
MIT License
239 stars 83 forks source link

spellchecker rebuild fail for electron 7.x #164

Closed pashvin closed 5 years ago

pashvin commented 5 years ago

Download latest electron 6.x and latest spell checker and try to rebuild and it shows following errors

[21:26:36][Step 3/5] ..\src\main.cc(104): error C2661: 'v8::Object::Set': no overloaded function takes 2 arguments [C:\BuildAgent\work\7f86723334d6a2e3\Client\src\node_modules\@felixrieseberg\spellchecker\build\spellchecker.vcxproj] [21:26:36][Step 3/5] ..\src\main.cc(105): error C2661: 'v8::Object::Set': no overloaded function takes 2 arguments [C:\BuildAgent\work\7f86723334d6a2e3\Client\src\node_modules\@felixrieseberg\spellchecker\build\spellchecker.vcxproj] [21:26:36][Step 3/5] ..\src\main.cc(106): error C2661: 'v8::Object::Set': no overloaded function takes 2 arguments [C:\BuildAgent\work\7f86723334d6a2e3\Client\src\node_modules\@felixrieseberg\spellchecker\build\spellchecker.vcxproj] [21:26:36][Step 3/5] ..\src\main.cc(159): error C2661: 'v8::Object::Set': no overloaded function takes 2 arguments [C:\BuildAgent\work\7f86723334d6a2e3\Client\src\node_modules\@felixrieseberg\spellchecker\build\spellchecker.vcxproj] [21:26:36][Step 3/5] ..\src\main.cc(187): error C2661: 'v8::Object::Set': no overloaded function takes 2 arguments [C:\BuildAgent\work\7f86723334d6a2e3\Client\src\node_modules\@felixrieseberg\spellchecker\build\spellchecker.vcxproj] [21:26:36][Step 3/5] win_delay_load_hook.cc [21:26:36][Step 3/5] gyp ERR! build error

pashvin commented 5 years ago

it started working with 6.0.12 electron so don't know what is going on. It still gives error for Electron 7 beta but that is OK as it is still not released so possible some changes required for Electron 7.x

mjurkowski commented 5 years ago

Electron 7 has been released

pashvin commented 5 years ago

Yes I just noticed that in electron website and tried with latest electron 7 and spell checker rebuild fails with the same error. Do we need fix in spell checker?

pashvin commented 5 years ago

By the way it fails on both (Windows and mac).

quanglam2807 commented 5 years ago

It fails on Linux, too. electron-builder install-app-deps passes through without reporting any problem, but ./build/Release/spellchecker.node is not generated.

Here is the source code of main.cc: https://github.com/felixrieseberg/node-spellchecker/blob/master/src/main.cc

quanglam2807 commented 5 years ago

Actually, there's a PR to fix it: https://github.com/atom/node-spellchecker/pull/128

Updated: I applied the PR and it worked.

pashvin commented 5 years ago

Thanks. Hope to get a new build with this PR early next week.

fxha commented 5 years ago

I created a PR at @felixrieseberg native library but I don't know whether this library is still active maintained. We're currently using the patch with Electron 7 without any problems.

quanglam2807 commented 5 years ago

@fxha Awesome. Also your patch looks much simpler than the one I’m using. Would you mind checking it there are any differences as I’m not very familiar with C++: https://github.com/quanglam2807/webcatalog/pull/344/files

pashvin commented 5 years ago

Is it possible to get new build with patch?

pashvin commented 5 years ago

latest version works now so closing this issue.