search

WiseLibs / better-sqlite3

The fastest and simplest library for SQLite3 in Node.js.
MIT License
5.23k stars 390 forks source link

Add support for electron `v29` prebuilds #1147

Closed m4heshd closed 4 months ago

m4heshd commented 4 months ago

Stable electron v29 just released. Prebuilds were tested here but the tests are failing with the following error.

D:\better-sqlite3\build\src\util\macros.lzz(150,33): error C2664: 'void v8::ObjectTemplate::SetAccessor(v8::Local<v8::String>,v8::AccessorGetterCallback,v8::AccessorSetterCallback,
v8::Local<v8::Value>,v8::PropertyAttribute,v8::SideEffectType,v8::SideEffectType)': cannot convert argument 5 from 'v8::AccessControl' to 'v8::PropertyAttribute' [D:\better-sqlite3\build\better_sqlite3.vcxproj]

Pinging @JoshuaWise.

mceachen commented 4 months ago

Ugh:

https://github.com/WiseLibs/better-sqlite3/actions/runs/7995300838/job/21835431780#step:8:4004

m4heshd commented 4 months ago

https://github.com/WiseLibs/better-sqlite3/actions/runs/7995300838/job/21835431780#step:8:4004

Yup. The same error I mentioned in the note. Needs some work it seems. I wonder if it's time to switch to N-API.

mceachen commented 4 months ago

(I totally missed your comment -- I just looked at the diff and LGTM'ed it in)

mceachen commented 4 months ago

@JoshuaWise https://github.com/WiseLibs/better-sqlite3/issues/1127 seems like it may solve this if those default params are indeed optional.

m4heshd commented 4 months ago

(I totally missed your comment -- I just looked at the diff and LGTM'ed it in)

Oops. That was my guess when you did the release.

mceachen commented 4 months ago

@m4heshd please submit a PR that reverts this, and I'll do another patch-release to roll back.

@JoshuaWise alternatively, you can just roll back the last release: npm dist-tag add better-sqlite3@9.4.1 latest

Sorry I didn't catch your comment!

m4heshd commented 4 months ago

Sorry I didn't catch your comment!

No worries. Sent the PR.