Build fails with node 12

josephg commented 5 years ago

The build is failing under node 12 due to deprecated APIs which have been removed.

This is high priority since node 12 hit current a couple of weeks ago.

josephg commented 5 years ago

The main issue seems to be in nan, which doesn't support node 12 yet.

Tracking issue for node 12 support in nan.

bnoordhuis commented 5 years ago

@josephg Going by the line numbers you're not using nan@2.13.2, the latest release. v2.13.2 should build against Node.js v12.x.

josephg commented 5 years ago

Thanks, you’re right that nan is the wrong culprit here. I tried updating nan, but it didn’t fix the build problems.

I had a closer look - the real issue is that the C bindings here have been written by several people over several years, and they aren’t using nan consistently anyway. Some of the build issues on node 12 are from code that’s consuming v8 / node’s APIs directly. I think the right answer is to use this as an excuse to port everything to n-api or node-addon-api. With napi backported to node 6 I think it’s a fine api to target. I’ll leave the uv_async threading code as is for now though, inefficient as it is because the threading parts of napi are only available from node 11 onwards. (And libuv seems like a very stable target).

For anyone following along, I’ve got the core ported across in a branch - though I haven’t pushed it yet. It’s now a process of going through the whole api surface area and converting everything else.

josephg commented 5 years ago

Oh, no, I can't use n-api before node 11 because its impossible to create a napi_env from inside my own uv_async callback. If I'm going to use napi I need the threadsafe function APIs.

Option 1: Lean harder into nan and make the build work with node 12 via nan 2.13.2
Option 2: Finish the n-api port. Use n-api for node 11 and node 12, but the older native code for earlier node versions. This seems like it would require less work in the long run, but in the medium term it doubles the surface area for bugs, because users running node 8-10 will be running quite different code from users running node 11+.

josephg commented 5 years ago

I've got the tests passing with napi on node 12. I'll do a release in a few days.

josephg commented 5 years ago

📦 foundationdb@0.10.1 which fixes this issue.

josephg / node-foundationdb

Build fails with node 12 #29