galkahana / HummusJS

Node.js module for high performance creation, modification and parsing of PDF files and streams
http://www.pdfhummus.com
Other
1.15k stars 169 forks source link

Fix Electron 5 and NodeJS 12 compatibility #374

Closed Rosga closed 5 years ago

Rosga commented 5 years ago

Electron 5 is still in beta and will be shipped with Node 12. Node 12 is going to be released tomorrow. There are some breaking changes to native modules: https://electronjs.org/blog/nodejs-native-addons-and-electron-5.

With latest version the command node-gyp rebuild --target=5.0.0-beta.9 --runtime=electron --dist-url=https://atom.io/download/electron --module-name=hummus --module-path=binding fails. This PR fixes build, and it works fine with my Electron application (merging PDF files and settings one as a background of another). All tests are passed as well.

I've tried to fix all other deprecations that have appeared for the newest version, but I might have missed some. There are a lot of deprecation warnings for node.h file, which will probably be fixed with the official NodeJS release.

galkahana commented 5 years ago

Thanks man! i'll be adding node12 to pre-built binaries too.

galkahana commented 5 years ago

published as 102. thanks again.

rbeeger commented 5 years ago

This looks great. But don't the travis and appveyor configs need to be updated to create pre-builts for electron 5 to actually be able to use hummus with electron 5?

galkahana commented 5 years ago

right. didnt take care of electron 5. i leave that, at this point, to a potential contributor.

rbeeger commented 5 years ago

OK then. Let's see if that works out.

galkahana commented 5 years ago

thanks man

rbeeger commented 5 years ago

Unfortunately that doesn't seem to work. The error changed from https://travis-ci.org/galkahana/HummusJS/jobs/474377362 to https://travis-ci.org/galkahana/HummusJS/jobs/527328672 .

According to https://nodejs.org/en/download/releases/ NODE_MODULE_VERSION=72 is used for the current node 12.0.0 and 12.1.0. NODE_MODULE_VERSION=70 is most likely the one used for the node included in electron 5.

Anyone an idea how to get it to use the correct NODE_MODULE_VERSION? This is also the same problem with the pre-builts for electron 4.

Maybe the build configuration does not fit electron > 4. https://github.com/atom/node-keytar is doing it differently, but I just don't understand native module building for node and electron enough to have an idea how to make it work for hummus.