From 124d582308b62c15498dab6fa95d05e3ba338adb Mon Sep 17 00:00:00 2001 From: Aaron Meriwether Date: Fri, 4 Feb 2022 22:20:30 -0600 Subject: [PATCH] Rename NonConstructor to OnCalledAsFunction and add docs --- doc/object_wrap.md | 15 ++++++++++ napi-inl.h | 7 +++-- napi.h | 3 +- test/binding.cc | 4 +-- test/binding.gyp | 2 +- test/objectwrap_function.cc | 45 ++++++++++++++++++++++++++++++ test/objectwrap_function.js | 22 +++++++++++++++ test/objectwrap_nonconstructor.cc | 46 ------------------------------- test/objectwrap_nonconstructor.js | 22 --------------- 9 files changed, 91 insertions(+), 75 deletions(-) create mode 100644 test/objectwrap_function.cc create mode 100644 test/objectwrap_function.js delete mode 100644 test/objectwrap_nonconstructor.cc delete mode 100644 test/objectwrap_nonconstructor.js diff --git a/doc/object_wrap.md b/doc/object_wrap.md index 0d3ef9856..7c7bbec52 100644 --- a/doc/object_wrap.md +++ b/doc/object_wrap.md @@ -212,6 +212,21 @@ property of the `Napi::CallbackInfo`. Returns a `Napi::Function` representing the constructor function for the class. +### OnCalledAsFunction + +The default behavior when the constructor is called from JavaScript as a +function (without the `new` keyword) is to throw a `Napi::TypeError` with the +message `Class constructors cannot be invoked without 'new'`. + +This default behavior can be altered by defining this static method. + +```cpp +static Napi::Value OnCalledAsFunction(const Napi::CallbackInfo& callbackInfo); +``` + +- `[in] callbackInfo`: The object representing the components of the JavaScript +request being made. + ### Finalize Provides an opportunity to run cleanup code that requires access to the diff --git a/napi-inl.h b/napi-inl.h index 978f42dcd..64d4f5aa0 100644 --- a/napi-inl.h +++ b/napi-inl.h @@ -4452,9 +4452,10 @@ inline ClassPropertyDescriptor ObjectWrap::StaticValue(Symbol name, } template -inline Value ObjectWrap::NonConstructor(const Napi::CallbackInfo& info) { +inline Value ObjectWrap::OnCalledAsFunction( + const Napi::CallbackInfo& callbackInfo) { NAPI_THROW( - TypeError::New(info.Env(), + TypeError::New(callbackInfo.Env(), "Class constructors cannot be invoked without 'new'"), Napi::Value()); } @@ -4473,7 +4474,7 @@ inline napi_value ObjectWrap::ConstructorCallbackWrapper( bool isConstructCall = (new_target != nullptr); if (!isConstructCall) { return details::WrapCallback( - [&] { return T::NonConstructor(CallbackInfo(env, info)); }); + [&] { return T::OnCalledAsFunction(CallbackInfo(env, info)); }); } napi_value wrapper = details::WrapCallback([&] { diff --git a/napi.h b/napi.h index 2c2253531..429e6a622 100644 --- a/napi.h +++ b/napi.h @@ -2199,7 +2199,8 @@ namespace Napi { static PropertyDescriptor StaticValue(Symbol name, Napi::Value value, napi_property_attributes attributes = napi_default); - static Napi::Value NonConstructor(const Napi::CallbackInfo& info); + static Napi::Value OnCalledAsFunction( + const Napi::CallbackInfo& callbackInfo); virtual void Finalize(Napi::Env env); private: diff --git a/test/binding.cc b/test/binding.cc index dc307120c..f7a96e9b3 100644 --- a/test/binding.cc +++ b/test/binding.cc @@ -65,7 +65,7 @@ Object InitTypedArray(Env env); Object InitGlobalObject(Env env); Object InitObjectWrap(Env env); Object InitObjectWrapConstructorException(Env env); -Object InitObjectWrapNonConstructor(Env env); +Object InitObjectWrapFunction(Env env); Object InitObjectWrapRemoveWrap(Env env); Object InitObjectWrapMultipleInheritance(Env env); Object InitObjectReference(Env env); @@ -153,7 +153,7 @@ Object Init(Env env, Object exports) { exports.Set("objectwrap", InitObjectWrap(env)); exports.Set("objectwrapConstructorException", InitObjectWrapConstructorException(env)); - exports.Set("objectwrap_nonconstructor", InitObjectWrapNonConstructor(env)); + exports.Set("objectwrap_function", InitObjectWrapFunction(env)); exports.Set("objectwrap_removewrap", InitObjectWrapRemoveWrap(env)); exports.Set("objectwrap_multiple_inheritance", InitObjectWrapMultipleInheritance(env)); exports.Set("objectreference", InitObjectReference(env)); diff --git a/test/binding.gyp b/test/binding.gyp index 3011f9e7c..a0470bde5 100644 --- a/test/binding.gyp +++ b/test/binding.gyp @@ -66,7 +66,7 @@ 'typedarray.cc', 'objectwrap.cc', 'objectwrap_constructor_exception.cc', - 'objectwrap_nonconstructor.cc', + 'objectwrap_function.cc', 'objectwrap_removewrap.cc', 'objectwrap_multiple_inheritance.cc', 'object_reference.cc', diff --git a/test/objectwrap_function.cc b/test/objectwrap_function.cc new file mode 100644 index 000000000..be55ff3b4 --- /dev/null +++ b/test/objectwrap_function.cc @@ -0,0 +1,45 @@ +#include +#include +#include "test_helper.h" + +class FunctionTest : public Napi::ObjectWrap { + public: + FunctionTest(const Napi::CallbackInfo& info) + : Napi::ObjectWrap(info) {} + + static Napi::Value OnCalledAsFunction(const Napi::CallbackInfo& info) { + // If called with a "true" argument, throw an exeption to test the handling. + if (!info[0].IsUndefined() && MaybeUnwrap(info[0].ToBoolean())) { + NAPI_THROW(Napi::Error::New(info.Env(), "an exception"), Napi::Value()); + } + // Otherwise, act as a factory. + std::vector args; + for (size_t i = 0; i < info.Length(); i++) args.push_back(info[i]); + return MaybeUnwrap(GetConstructor(info.Env()).New(args)); + } + + // Constructor-per-env map in a static member because env.SetInstanceData() + // would interfere with Napi::Addon + static std::unordered_map constructors; + + static void Initialize(Napi::Env env, Napi::Object exports) { + const char* name = "FunctionTest"; + Napi::Function func = DefineClass(env, name, {}); + constructors[env] = Napi::Persistent(func); + env.AddCleanupHook([env] { constructors.erase(env); }); + exports.Set(name, func); + } + + static Napi::Function GetConstructor(Napi::Env env) { + return constructors[env].Value(); + } +}; + +std::unordered_map + FunctionTest::constructors = {}; + +Napi::Object InitObjectWrapFunction(Napi::Env env) { + Napi::Object exports = Napi::Object::New(env); + FunctionTest::Initialize(env, exports); + return exports; +} diff --git a/test/objectwrap_function.js b/test/objectwrap_function.js new file mode 100644 index 000000000..7bcf6c087 --- /dev/null +++ b/test/objectwrap_function.js @@ -0,0 +1,22 @@ +'use strict'; + +const assert = require('assert'); +const testUtil = require('./testUtil'); + +function test (binding) { + return testUtil.runGCTests([ + 'objectwrap function', + () => { + const { FunctionTest } = binding.objectwrap_function; + const newConstructed = new FunctionTest(); + const functionConstructed = FunctionTest(); + assert(newConstructed instanceof FunctionTest); + assert(functionConstructed instanceof FunctionTest); + assert.throws(() => (FunctionTest(true)), /an exception/); + }, + // Do on gc before returning. + () => {} + ]); +} + +module.exports = require('./common').runTest(test); diff --git a/test/objectwrap_nonconstructor.cc b/test/objectwrap_nonconstructor.cc deleted file mode 100644 index 616952432..000000000 --- a/test/objectwrap_nonconstructor.cc +++ /dev/null @@ -1,46 +0,0 @@ -#include -#include -#include "test_helper.h" - -class NonConstructorTest : public Napi::ObjectWrap { - public: - NonConstructorTest(const Napi::CallbackInfo& info) - : Napi::ObjectWrap(info) {} - - static Napi::Value NonConstructor(const Napi::CallbackInfo& info) { - // If called with a "true" argument, throw an exeption to test the handling. - if (!info[0].IsUndefined() && MaybeUnwrap(info[0].ToBoolean())) { - NAPI_THROW(Napi::Error::New(info.Env(), "an exception"), Napi::Value()); - } - // Otherwise, act as a factory. - std::vector args; - for (size_t i = 0; i < info.Length(); i++) args.push_back(info[i]); - return MaybeUnwrap(GetConstructor(info.Env()).New(args)); - } - - static std::unordered_map constructors; - - static void Initialize(Napi::Env env, Napi::Object exports) { - const char* name = "NonConstructorTest"; - Napi::Function func = DefineClass(env, name, {}); - Napi::FunctionReference* constructor = new Napi::FunctionReference(); - *constructor = Napi::Persistent(func); - constructors[(napi_env)env] = constructor; - exports.Set(name, func); - } - - static Napi::Function GetConstructor(Napi::Env env) { - Napi::EscapableHandleScope scope(env); - Napi::Function func = constructors[(napi_env)env]->Value(); - return scope.Escape(napi_value(func)).As(); - } -}; - -std::unordered_map - NonConstructorTest::constructors = {}; - -Napi::Object InitObjectWrapNonConstructor(Napi::Env env) { - Napi::Object exports = Napi::Object::New(env); - NonConstructorTest::Initialize(env, exports); - return exports; -} diff --git a/test/objectwrap_nonconstructor.js b/test/objectwrap_nonconstructor.js deleted file mode 100644 index 110ee3827..000000000 --- a/test/objectwrap_nonconstructor.js +++ /dev/null @@ -1,22 +0,0 @@ -'use strict'; - -const assert = require('assert'); -const testUtil = require('./testUtil'); - -function test (binding) { - return testUtil.runGCTests([ - 'objectwrap nonconstructor', - () => { - const { NonConstructorTest } = binding.objectwrap_nonconstructor; - const newConstructed = new NonConstructorTest(); - const factoryConstructed = NonConstructorTest(); - assert(newConstructed instanceof NonConstructorTest); - assert(factoryConstructed instanceof NonConstructorTest); - assert.throws(() => (NonConstructorTest(true)), /an exception/); - }, - // Do on gc before returning. - () => {} - ]); -} - -module.exports = require('./common').runTest(test);