From 9af027ad3c35ba5832b63769898ed1db0c59ddae Mon Sep 17 00:00:00 2001 From: Aaron Meriwether Date: Tue, 11 Jan 2022 14:59:57 -0600 Subject: [PATCH 1/4] Add `NonConstructor` to allow customization of ObjectWrap behavior --- napi-inl.h | 10 +++++-- napi.h | 1 + test/binding.cc | 2 ++ test/binding.gyp | 1 + test/objectwrap_nonconstructor.cc | 46 +++++++++++++++++++++++++++++++ test/objectwrap_nonconstructor.js | 22 +++++++++++++++ 6 files changed, 80 insertions(+), 2 deletions(-) create mode 100644 test/objectwrap_nonconstructor.cc create mode 100644 test/objectwrap_nonconstructor.js diff --git a/napi-inl.h b/napi-inl.h index bd54feb67..280f1340d 100644 --- a/napi-inl.h +++ b/napi-inl.h @@ -4451,6 +4451,11 @@ inline ClassPropertyDescriptor ObjectWrap::StaticValue(Symbol name, return desc; } +template +inline Value ObjectWrap::NonConstructor(const Napi::CallbackInfo& info) { + NAPI_THROW(TypeError::New(info.Env(), "Class constructors cannot be invoked without 'new'"), Napi::Value()); +}; + template inline void ObjectWrap::Finalize(Napi::Env /*env*/) {} @@ -4464,8 +4469,9 @@ inline napi_value ObjectWrap::ConstructorCallbackWrapper( bool isConstructCall = (new_target != nullptr); if (!isConstructCall) { - napi_throw_type_error(env, nullptr, "Class constructors cannot be invoked without 'new'"); - return nullptr; + return details::WrapCallback([&] { + return T::NonConstructor(CallbackInfo(env, info)); + }); } napi_value wrapper = details::WrapCallback([&] { diff --git a/napi.h b/napi.h index 286430643..2c2253531 100644 --- a/napi.h +++ b/napi.h @@ -2199,6 +2199,7 @@ namespace Napi { static PropertyDescriptor StaticValue(Symbol name, Napi::Value value, napi_property_attributes attributes = napi_default); + static Napi::Value NonConstructor(const Napi::CallbackInfo& info); virtual void Finalize(Napi::Env env); private: diff --git a/test/binding.cc b/test/binding.cc index 57be51592..dc307120c 100644 --- a/test/binding.cc +++ b/test/binding.cc @@ -65,6 +65,7 @@ Object InitTypedArray(Env env); Object InitGlobalObject(Env env); Object InitObjectWrap(Env env); Object InitObjectWrapConstructorException(Env env); +Object InitObjectWrapNonConstructor(Env env); Object InitObjectWrapRemoveWrap(Env env); Object InitObjectWrapMultipleInheritance(Env env); Object InitObjectReference(Env env); @@ -152,6 +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_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 44f124b01..3011f9e7c 100644 --- a/test/binding.gyp +++ b/test/binding.gyp @@ -66,6 +66,7 @@ 'typedarray.cc', 'objectwrap.cc', 'objectwrap_constructor_exception.cc', + 'objectwrap_nonconstructor.cc', 'objectwrap_removewrap.cc', 'objectwrap_multiple_inheritance.cc', 'object_reference.cc', diff --git a/test/objectwrap_nonconstructor.cc b/test/objectwrap_nonconstructor.cc new file mode 100644 index 000000000..e7f798177 --- /dev/null +++ b/test/objectwrap_nonconstructor.cc @@ -0,0 +1,46 @@ +#include +#include "test_helper.h" +#include + +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 new file mode 100644 index 000000000..c605a0ecf --- /dev/null +++ b/test/objectwrap_nonconstructor.js @@ -0,0 +1,22 @@ +'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); From b337aea1a85f8ea011dd519035b21769a1331d70 Mon Sep 17 00:00:00 2001 From: Aaron Meriwether Date: Tue, 11 Jan 2022 17:29:13 -0600 Subject: [PATCH 2/4] Appease the pedantic gcc and eslint --- napi-inl.h | 12 +++++++----- test/objectwrap_nonconstructor.cc | 18 +++++++++--------- test/objectwrap_nonconstructor.js | 2 +- 3 files changed, 17 insertions(+), 15 deletions(-) diff --git a/napi-inl.h b/napi-inl.h index 280f1340d..978f42dcd 100644 --- a/napi-inl.h +++ b/napi-inl.h @@ -4453,8 +4453,11 @@ inline ClassPropertyDescriptor ObjectWrap::StaticValue(Symbol name, template inline Value ObjectWrap::NonConstructor(const Napi::CallbackInfo& info) { - NAPI_THROW(TypeError::New(info.Env(), "Class constructors cannot be invoked without 'new'"), Napi::Value()); -}; + NAPI_THROW( + TypeError::New(info.Env(), + "Class constructors cannot be invoked without 'new'"), + Napi::Value()); +} template inline void ObjectWrap::Finalize(Napi::Env /*env*/) {} @@ -4469,9 +4472,8 @@ inline napi_value ObjectWrap::ConstructorCallbackWrapper( bool isConstructCall = (new_target != nullptr); if (!isConstructCall) { - return details::WrapCallback([&] { - return T::NonConstructor(CallbackInfo(env, info)); - }); + return details::WrapCallback( + [&] { return T::NonConstructor(CallbackInfo(env, info)); }); } napi_value wrapper = details::WrapCallback([&] { diff --git a/test/objectwrap_nonconstructor.cc b/test/objectwrap_nonconstructor.cc index e7f798177..616952432 100644 --- a/test/objectwrap_nonconstructor.cc +++ b/test/objectwrap_nonconstructor.cc @@ -1,21 +1,20 @@ #include -#include "test_helper.h" #include +#include "test_helper.h" -class NonConstructorTest : - public Napi::ObjectWrap { -public: - NonConstructorTest(const Napi::CallbackInfo& info) : - Napi::ObjectWrap(info) {} +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())) { + 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]); + for (size_t i = 0; i < info.Length(); i++) args.push_back(info[i]); return MaybeUnwrap(GetConstructor(info.Env()).New(args)); } @@ -37,7 +36,8 @@ class NonConstructorTest : } }; -std::unordered_map NonConstructorTest::constructors = {}; +std::unordered_map + NonConstructorTest::constructors = {}; Napi::Object InitObjectWrapNonConstructor(Napi::Env env) { Napi::Object exports = Napi::Object::New(env); diff --git a/test/objectwrap_nonconstructor.js b/test/objectwrap_nonconstructor.js index c605a0ecf..110ee3827 100644 --- a/test/objectwrap_nonconstructor.js +++ b/test/objectwrap_nonconstructor.js @@ -3,7 +3,7 @@ const assert = require('assert'); const testUtil = require('./testUtil'); -function test(binding) { +function test (binding) { return testUtil.runGCTests([ 'objectwrap nonconstructor', () => { From f0581a73faf3876ccd0c725b802489996123a843 Mon Sep 17 00:00:00 2001 From: Aaron Meriwether Date: Fri, 4 Feb 2022 22:20:30 -0600 Subject: [PATCH 3/4] Rename NonConstructor to OnCalledAsFunction and add docs --- doc/object_wrap.md | 22 +++++++++++++++ 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, 98 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..38ac88d26 100644 --- a/doc/object_wrap.md +++ b/doc/object_wrap.md @@ -212,6 +212,28 @@ property of the `Napi::CallbackInfo`. Returns a `Napi::Function` representing the constructor function for the class. +### OnCalledAsFunction + +The default behavior when a `Napi::ObjectWrap` class is called from +JavaScript as a function (without the **new** operator) is to throw a +`Napi::TypeError` with the message `Class constructors cannot be invoked +without 'new'`. That default behavior can be altered by defining this +static method. + +For example, you could internally re-call the JavaScript contstructor _with_ +the **new** operator (via +`Napi::Function::New(const std::vector &args)`), and return the +resulting object. Or you might do something else entirely such as the way +[`Date()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date#constructor) +produces a string when called as a function. + +```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); From cd0abbb36933e9da9c6a8a462a11dd8ce9ec518d Mon Sep 17 00:00:00 2001 From: Aaron Meriwether Date: Fri, 4 Feb 2022 22:49:18 -0600 Subject: [PATCH 4/4] Massaging the docs --- doc/object_wrap.md | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/doc/object_wrap.md b/doc/object_wrap.md index 38ac88d26..0b9dacdc9 100644 --- a/doc/object_wrap.md +++ b/doc/object_wrap.md @@ -214,16 +214,17 @@ Returns a `Napi::Function` representing the constructor function for the class. ### OnCalledAsFunction -The default behavior when a `Napi::ObjectWrap` class is called from -JavaScript as a function (without the **new** operator) is to throw a -`Napi::TypeError` with the message `Class constructors cannot be invoked -without 'new'`. That default behavior can be altered by defining this -static method. +Provides an opportunity to customize the behavior when a `Napi::ObjectWrap` +class is called from JavaScript as a function (without the **new** operator). + +The default behavior in this scenario is to throw a `Napi::TypeError` with the +message `Class constructors cannot be invoked without 'new'`. Define this +public method on your derived class to override that behavior. For example, you could internally re-call the JavaScript contstructor _with_ the **new** operator (via `Napi::Function::New(const std::vector &args)`), and return the -resulting object. Or you might do something else entirely such as the way +resulting object. Or you might do something else entirely, such as the way [`Date()`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date#constructor) produces a string when called as a function.