Skip to content

Fix V8 external API usage for Electron 42#1475

Merged
JoshuaWise merged 1 commit into
WiseLibs:masterfrom
tstone-1:fix-electron-42-v8-api
Jun 13, 2026
Merged

Fix V8 external API usage for Electron 42#1475
JoshuaWise merged 1 commit into
WiseLibs:masterfrom
tstone-1:fix-electron-42-v8-api

Conversation

@tstone-1

Copy link
Copy Markdown
Contributor

Summary

  • add compatibility macros for tagged v8::External creation/access on V8 14+
  • pass nullptr for missing SetNativeDataProperty setter to avoid Electron 42 overload ambiguity
  • follow the existing V8 version-guard pattern used by prior Electron compatibility work such as fix: use HolderV2() for PropertyCallbackInfo on V8 >= 12.5 #1459

Verification

  • npm run build-debug
  • npm test
  • minimal Electron 42.1.0 rebuild from source with @electron/rebuild:
    npx electron-rebuild -v 42.1.0 --types dev,prod,optional --force --build-from-source

Fixes #1474

Electron 42 ships V8 14.8, where v8::External::New and v8::External::Value require an ExternalPointerTypeTag. This breaks native rebuilds with Electron 42 headers.

Add small compatibility macros for v8::External creation/access, following the existing V8 version-guard pattern used for prior Electron compatibility fixes such as WiseLibs#1459. Also pass nullptr to SetNativeDataProperty to avoid ambiguous overload resolution for a missing setter.

Verified with npm test and an Electron 42.1.0 rebuild from source.
@tstone-1 tstone-1 force-pushed the fix-electron-42-v8-api branch from 9050b50 to 29f2ecd Compare May 17, 2026 12:09
@tstone-1

Copy link
Copy Markdown
Contributor Author

Related prior work: #1471 also targets newer V8 / Electron 42 build compatibility and touches the same API area.

This PR differs in two details found while working through CI:

  • The tagged v8::External API is guarded on NODE_MODULE_VERSION >= 146 rather than V8_MAJOR_VERSION, because Node 25 reports V8 14 but still exposes the old two-argument External::New() and zero-argument External::Value() APIs.
  • The default external pointer tag is passed as 0 rather than v8::kExternalPointerTypeTagDefault, because Node 25 headers do not expose that constant.

That keeps Node 25 on the old API while enabling Electron 42 / Node 26 headers, and the current CI matrix passes for Node 22/24/25/26 across Linux, macOS, and Windows.

@jimmyn

jimmyn commented May 18, 2026

Copy link
Copy Markdown

This is very much needed!

@zl939144892

Copy link
Copy Markdown

Hi @JoshuaWise , when can this PR be merged and published?

@wisdomstar94

Copy link
Copy Markdown

I hope this PR be merged soon.

filiphsps added a commit to filiphsps/plucker that referenced this pull request Jun 2, 2026
…n tests

better-sqlite3 12.10.0 fails to compile against Electron 42's V8 (External::New
and External::Value now require an ExternalPointerTypeTag; SetNativeDataProperty's
setter arg must be nullptr). Apply the upstream version-guarded fix from
WiseLibs/better-sqlite3#1475 via pnpm patchedDependencies so install-app-deps
builds the native module for Electron. A pretest guard rebuilds it for Node's ABI
when needed so Vitest (which runs under Node, not Electron) can load it.
@DmitriiKholkin

Copy link
Copy Markdown

This is an important PR. I'm looking forward to its completion.

pawelangelow added a commit to redis/RedisInsight that referenced this pull request Jun 4, 2026
better-sqlite3 12.8.0 (and through the current latest 12.10.0) fails to
compile against Electron 42's V8 headers. Electron 42 ships V8 13.x /
14.x where v8::External::New and v8::External::Value require an
ExternalPointerTypeTag, and v8::Template::SetNativeDataProperty dropped
the deprecated overload accepting `int` for the missing-setter slot.
@electron/rebuild falls back to source compilation (the project's
prebuilt binaries do not yet cover Electron 42's NMV 146) and the
compile errors surface as:

  - v8::External::Value: function does not take 0 arguments
  - v8::External::New: function does not take 2 arguments
  - v8::Template::SetNativeDataProperty: ambiguous call

Apply the upstream version-guarded fix from
WiseLibs/better-sqlite3#1475 as a patch-package
patch in both sub-packages that depend on better-sqlite3 directly:
redisinsight/ (the Electron app bundle, packaged by electron-builder)
and redisinsight/api/ (used at runtime and by API tests). The patch is
gated on NODE_MODULE_VERSION >= 146 so pre-Electron-42 builds compile
unchanged.

Upstream references:
  - WiseLibs/better-sqlite3#1474 (build failure starting with Electron 42)
  - WiseLibs/better-sqlite3#1475 (the fix, approved but not yet merged)

This patch can be removed once #1475 is merged and we bump
better-sqlite3 to a release that includes it.
Seventysevendays pushed a commit to Seventysevendays/better-sqlite3 that referenced this pull request Jun 5, 2026
@luojunhui1

Copy link
Copy Markdown

important pr,we need this pr to make plugin adapt to recently released vscode (version 1.123.0)

@DmitriiKholkin

Copy link
Copy Markdown

Guys, you can temporarily use the prebuilds I made for the main platforms - https://github.com/DmitriiKholkin/RapiDB/releases/tag/rapidb-patched-sqlite

@pabloacevedo

Copy link
Copy Markdown

What are we still waiting on before approving these changes?

@m4heshd

m4heshd commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

What are we still waiting on before approving these changes?

This PR is already approved. The original author of BS3, @JoshuaWise is the sole code owner of the specific native API source files this PR targets. He's busy with life after more than a decade of work on this library. 🤷🏻‍♂️

@nikwen

nikwen commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

If anyone needs this urgently, you can use the approach from #1414 (comment). Just adjust repo name and commit hash.

@JoshuaWise JoshuaWise merged commit 5bb63a2 into WiseLibs:master Jun 13, 2026
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Build failure starting with electron 42.0.1

10 participants