search

charto / nbind

:sparkles: Magical headers that make your C++ library accessible from JavaScript :rocket:
MIT License
1.98k stars 119 forks source link

Support node 12 #130

Closed shawwn closed 5 years ago

shawwn commented 5 years ago

This PR updates nbind's codebase to support node 12. The changes are backwards compatible.

Currently, attempting to build e.g. yoga-layout against node 12 results in various compile errors. The errors are because nbind calls v8 APIs directly rather than using nan APIs, and the v8 apis have changed significantly in node 12.

shawwn commented 5 years ago

The checks were failing because the typescript dev dependency was outdated. I've updated typescript's version to ^3.4.3

ISNIT0 commented 5 years ago

Would love to see this merged, we have the same problem with MWOffliner https://github.com/openzim/mwoffliner/issues/707

ISNIT0 commented 5 years ago

@shawwn Is it worth updating the .travis.yml file to test on Node 12?

ISNIT0 commented 5 years ago

Linking to an actual issue: https://github.com/charto/nbind/issues/131

ISNIT0 commented 5 years ago

@jjrv 👋

ISNIT0 commented 5 years ago

@shawwn I'm getting some errors regarding v8::Handle - did you forget to commit some changes by any chance?

shawwn commented 5 years ago

@ISNIT0 Hmm, it's possible. What sort of errors are you getting?

ISNIT0 commented 5 years ago

@shawwn It seems v8::Handle was removed in node 12, @jjrv has made another commit (https://github.com/charto/nbind/commit/fe3abe05462d1b7559e0933e7f83802e8f05af27) replacing them with v8::Local, all seems to work well now 😄