prebuilt: add electron 9.0, 8.3 and 7.3

[mtgto]

https://github.com/electron/electron/releases/tag/v9.0.0
https://github.com/electron/electron/releases/tag/v8.3.0
https://github.com/electron/electron/releases/tag/v7.3.0

I don't test with new electron version.
Do you wish to continue to support prebuilt for Electron v6?

[mtgto]

Segmentation fault for Electron v9 happens in my macOS environment.
I am finding the reason.

[mtgto]

'open and close memory database queuing' of test/open_close.test.js causes segmentation fault.
https://github.com/mapbox/node-sqlite3/blob/v4.2.0/test/open_close.test.js#L133-L150

Here is smallest example to reproduce segmentation fault with Electron v9.0.0 and macOS 10.14.6.

    describe('sample of segmentation fault', function() {
        var db;
        it('should open the database', function(done) {
            db = new sqlite3.Database(':memory:', sqlite3.OPEN_READONLY, function(err) {
                assert.ok(false);
                done();
            });
        });
    });

[mtgto]

In short: this project builds napi version library for electron test, not electron, even if we set --runtime=electron.
Electron test with napi version node_sqlite3.node always crash in 'open and close memory database queuing' of test/open_close.test.js.

Details

CI test for electron build using scripts/build_against_electron.sh.
I run with macOS 10.14.6, this script generates these files:

$ ELECTRON_VERSION=9.0.0 NODE_VERSION=12 TRAVIS_OS_NAME=osx ./scripts/build_against_electron.sh

$ find . -name "*.node"
./build-tmp-napi-v3/Release/node_sqlite3.node
./node_modules/fsevents/fsevents.node
./lib/binding/napi-v3-darwin-x64/node_sqlite3.node

I install node-sqlite3 outside of node-sqlite3:

$ mkdir electron-9-sqlite3-sample && cd electron-9-sqlite3-sample
$ npm init -y
$ npm install sqlite3 --runtime=electron --target=9.0.0 --dist-url=https://electronjs.org/headers
$ find . -name "*.node"
./node_modules/sqlite3/lib/binding/electron-v9.0-darwin-x64/node_sqlite3.node
./node_modules/sqlite3/build/Release/node_sqlite3.node

I find paths of node_sqlite3.node of each project are different.
I overwrite node_sqlite3.node from outside of node-sqlite3.

$ cp ../electron-9-sqlite3-sample/node_modules/sqlite3/lib/binding/electron-v9.0-darwin-x64/node_sqlite3.node \
lib/binding/napi-v3-darwin-x64/node_sqlite3.node

Now, electron-mocha don't crash.

$ cat ./test/hoge.test.js
var sqlite3 = require('..');
describe('sample', function() {
    var db;
    it('should open the database', function(done) {
        db = new sqlite3.Database(':memory:', sqlite3.OPEN_READONLY, function(err) {
            assert.ok(false);
            done();
        });
    });
});

$ yarn run electron-mocha ./test/hoge.test.js

  sample of test
    1) should open the database


  0 passing (56ms)
  1 failing

  1) sample
       should open the database:
     Uncaught ReferenceError: assert is not defined
      at Database.<anonymous> (test/hoge.test.js:6:13)

IMO, it needs to overwrite binary entry of package.json to build binary for electron when CI runs for electron.

[kewde]

Thank you, once again, for your contributions @mtgto
Strangely, the CI seems to fail to find the right binary for the test suite that uses the shared SQLite3 headers. The version shipped with the application (and from which the builds are published) is running fine however.

It does seem like the paths are wrong, this will require some investigation.
Our configuration does not seem to use {node_napi_label}, instead it uses an different configuration option called napi_build_version.

Anyways, we have fully converted to N-API but the pathing still seems off.

[kewde]

Technically, with the new N-API support we should already be able to support electron 9.0.0 without adding it to our build matrix. (Same goes for any future Electron release, even 8.3 & 7.3, etc..)

[mtgto]

Technically, with the new N-API support we should already be able to support electron 9.0.0 without adding it to our build matrix.

Wow, it is nice.

---

I found I had a mistake in my last comment:

In short: this project builds napi version library for electron test, not electron, even if we set --runtime=electron.

I install node-sqlite3 v4.2.0, which is latest released version, outside of node-sqlite3 repo.
So sqlite3 v4.2.0 use nan, not node-addon-api.

I checkout sqlite3 v4.2.0 tag, build for Electron v9.0 with nan.
It does not cause Electron crash.

$ git clone [email protected]:mapbox/node-sqlite3.git node-sqlite3-v42 && cd node-sqlite3-v42
$ git checkout v4.2.0
$ npm install --runtime=electron --target=9.0.0 --dist-url=https://electronjs.org/headers --build-from-source --clang=1
$ electron --version
v9.0.0

$ cat test/hoge.test.js
var sqlite3 = require('..');

describe('sample of segmentation fault', function() {
    var db;
    it('should open the database', function(done) {
        db = new sqlite3.Database(':memory:', sqlite3.OPEN_READONLY, function(err) {
            throw new Error("THIS IS UNHANDLED");
        });
    });
});

$ electron-mocha -R spec test/hoge.test.js


  sample of segmentation fault
    1) should open the database


  0 passing (10ms)
  1 failing

  1) sample of segmentation fault
       should open the database:
     Uncaught Error: THIS IS UNHANDLED
      at Database.<anonymous> (test/hoge.test.js:7:19)

[daniellockyer]

@mtgto I'm not sure if this is still relevant/needed 🤔 I'm going to close it for now but please let me know if the issue persists on sqlite3 v5, particularly after the upcoming release 🙂