You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Fix v8::External::Value/New for the new ExternalPointerTypeTag API (#1015)
* Fix v8::External::Value/New calls for V8 13
V8 13 added an ExternalPointerTypeTag parameter (uint16_t pointer-tag for the
sandbox) to both v8::External::New and v8::External::Value:
static Local<External> New(Isolate*, void*, ExternalPointerTypeTag);
void* Value(ExternalPointerTypeTag) const;
The old zero-arg Value() and two-arg New(Isolate*, void*) overloads are gone,
so any nan-using addon fails to compile against Electron 42 / Node 24's V8:
nan_callbacks_12_inl.h(189): error C2664:
'void *v8::External::Value(v8::ExternalPointerTypeTag) const':
cannot convert argument 1 from 'v8::Isolate *' to 'v8::ExternalPointerTypeTag'
nan_implementation_12_inl.h(79): error C2660:
'v8::External::New': function does not take 2 arguments
Add two private inline helpers in the existing imp:: namespace:
imp::GetExternalPointer(v8::Local<v8::External>)
imp::NewExternal(v8::Isolate*, void*)
Each selects the V8 13+ signature when V8_MAJOR_VERSION >= 13 (passing
v8::kExternalPointerTypeTagDefault, the documented no-tagging default for
embedders that do not partition externals by type) and falls back to the
legacy signature otherwise. All 28 Value() trampoline callsites in
nan_callbacks_12_inl.h and all 3 New() factory callsites in
nan_implementation_12_inl.h are routed through the helpers. Both helpers
share kExternalPointerTypeTagDefault so the tag matches between create and
read. No new macros are introduced.
Tests
-----
The existing news.cpp::NewExternal and nannew.cpp::testExternal cases call
v8::External::Value() directly (consumer-side raw-V8 calls, not through nan),
so they failed to compile against V8 13 in the same way as nan's headers.
Added the same V8 13 guard inline at those two sites so the tests build and
run on V8 13 while remaining identical on older V8. Together these two
existing tests already cover the round-trip exercised by this fix:
- news.cpp::NewExternal exercises the factory path:
Nan::New<v8::External>(...) -> Factory<v8::External>::New(value)
-> imp::NewExternal(isolate, value)
-> v8::External::New(isolate, value, tag)
- The trampoline path (imp::GetExternalPointer) is exercised every time
any NAN_METHOD test is invoked from JS, including news.cpp::NewExternal
itself, since FunctionCallbackWrapper unpacks the registered callback
pointer through that helper before dispatching.
Scope and what is not changed
-----------------------------
This patch addresses the v8::External break specifically. Other unrelated
V8 13 API changes in nan, if any, are out of scope (mirroring the prior
nan_weak.h fix which was a separate, surgical commit).
nan_callbacks_pre_12_inl.h and nan_implementation_pre_12_inl.h are
intentionally untouched: they target NODE_MODULE_VERSION <=
NODE_0_12_MODULE_VERSION (Node <= 0.12, pre-V8-3.x), where neither overload
exists.
Backwards compatibility
-----------------------
Every change is gated on `defined(V8_MAJOR_VERSION) && V8_MAJOR_VERSION >= 13`.
On V8 <= 12 the else branch reproduces the original code byte-for-byte, so
the patch is a no-op for every V8 version nan currently supports.
* Expose Nan::GetExternalValue for the tagged v8::External::Value() API
Promotes the read-side helper from imp:: to public Nan:: so addons can
read v8::External values portably across V8 versions without their own
V8_EXTERNAL_POINTER_TAG_COUNT branch. Tests drop the conditional.
Reads the pointer stored in a `v8::External`. On V8 versions that require an `ExternalPointerTypeTag` on `v8::External::Value()`, the default tag is applied (matching the tag used when the External is created via `Nan::New<v8::External>()`).
0 commit comments