Skip to content

Commit 1d4e3b7

Browse files
MarlyfleitasMarlyfleitas
committed
Refactor property setting
Previously, various utility overloads of `Object::Set()` were provided. This was problematic, because accidental unexpected conversions could happen (e.g. `const char*` to `bool`) and overloads were ambiguous (e.g. `int` could be cast to `double` and `bool` and therefore could not be passed directly). To address this, pass a template parameter instead, and introduce a helper `Value::From` method that could create a value based on the type of object it receives, and that uses clear precedence rules for determining the kind of JS value to create. Fixes: nodejs/node-addon-api#86 PR-URL: nodejs/node-addon-api#123 Reviewed-By: Benjamin Byholm <[email protected]>
1 parent a6f7f97 commit 1d4e3b7

File tree

4 files changed

+199
-146
lines changed

4 files changed

+199
-146
lines changed

‎napi-inl.h

Lines changed: 119 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
// Note: Do not include this file directly! Include "napi.h" instead.
1111

1212
#include <cstring>
13+
#include <type_traits>
1314

1415
namespace Napi {
1516

@@ -599,6 +600,106 @@ inline Symbol::Symbol() : Name() {
599600
inline Symbol::Symbol(napi_env env, napi_value value) : Name(env, value) {
600601
}
601602

603+
////////////////////////////////////////////////////////////////////////////////
604+
// Automagic value creation
605+
////////////////////////////////////////////////////////////////////////////////
606+
607+
namespace details {
608+
template <typename T>
609+
struct vf_number {
610+
static Number From(napi_env env, T value) {
611+
return Number::New(env, static_cast<double>(value));
612+
}
613+
};
614+
615+
template<>
616+
struct vf_number<bool> {
617+
static Boolean From(napi_env env, bool value) {
618+
return Boolean::New(env, value);
619+
}
620+
};
621+
622+
struct vf_utf8_charp {
623+
static String From(napi_env env, const char* value) {
624+
return String::New(env, value);
625+
}
626+
};
627+
628+
struct vf_utf16_charp {
629+
static String From(napi_env env, const char16_t* value) {
630+
return String::New(env, value);
631+
}
632+
};
633+
struct vf_utf8_string {
634+
static String From(napi_env env, const std::string& value) {
635+
return String::New(env, value);
636+
}
637+
};
638+
639+
struct vf_utf16_string {
640+
static String From(napi_env env, const std::u16string& value) {
641+
return String::New(env, value);
642+
}
643+
};
644+
645+
template <typename T>
646+
struct vf_fallback {
647+
static Value From(napi_env env, const T& value) {
648+
return Value(env, value);
649+
}
650+
};
651+
652+
template <typename...> struct disjunction : std::false_type {};
653+
template <typename B> struct disjunction<B> : B {};
654+
template <typename B, typename... Bs>
655+
struct disjunction<B, Bs...>
656+
: std::conditional<bool(B::value), B, disjunction<Bs...>>::type {};
657+
658+
template <typename T>
659+
struct can_make_string
660+
: disjunction<typename std::is_convertible<T, const char *>::type,
661+
typename std::is_convertible<T, const char16_t *>::type,
662+
typename std::is_convertible<T, std::string>::type,
663+
typename std::is_convertible<T, std::u16string>::type> {};
664+
}
665+
666+
template <typename T>
667+
Value Value::From(napi_env env, const T& value) {
668+
using Helper = typename std::conditional<
669+
std::is_integral<T>::value || std::is_floating_point<T>::value,
670+
details::vf_number<T>,
671+
typename std::conditional<
672+
details::can_make_string<T>::value,
673+
String,
674+
details::vf_fallback<T>
675+
>::type
676+
>::type;
677+
return Helper::From(env, value);
678+
}
679+
680+
template <typename T>
681+
String String::From(napi_env env, const T& value) {
682+
struct Dummy {};
683+
using Helper = typename std::conditional<
684+
std::is_convertible<T, const char*>::value,
685+
details::vf_utf8_charp,
686+
typename std::conditional<
687+
std::is_convertible<T, const char16_t*>::value,
688+
details::vf_utf16_charp,
689+
typename std::conditional<
690+
std::is_convertible<T, std::string>::value,
691+
details::vf_utf8_string,
692+
typename std::conditional<
693+
std::is_convertible<T, std::u16string>::value,
694+
details::vf_utf16_string,
695+
Dummy
696+
>::type
697+
>::type
698+
>::type
699+
>::type;
700+
return Helper::From(env, value);
701+
}
702+
602703
////////////////////////////////////////////////////////////////////////////////
603704
// Object class
604705
////////////////////////////////////////////////////////////////////////////////
@@ -705,58 +806,32 @@ inline Value Object::Get(const std::string& utf8name) const {
705806
return Get(utf8name.c_str());
706807
}
707808

708-
inline void Object::Set(napi_value key, napi_value value) {
709-
napi_status status = napi_set_property(_env, _value, key, value);
809+
template <typename ValueType>
810+
inline void Object::Set(napi_value key, const ValueType& value) {
811+
napi_status status =
812+
napi_set_property(_env, _value, key, Value::From(_env, value));
710813
NAPI_THROW_IF_FAILED(_env, status);
711814
}
712815

713-
inline void Object::Set(Value key, Value value) {
714-
napi_status status = napi_set_property(_env, _value, key, value);
816+
template <typename ValueType>
817+
inline void Object::Set(Value key, const ValueType& value) {
818+
napi_status status =
819+
napi_set_property(_env, _value, key, Value::From(_env, value));
715820
NAPI_THROW_IF_FAILED(_env, status);
716821
}
717822

718-
inline void Object::Set(const char* utf8name, Value value) {
719-
napi_status status = napi_set_named_property(_env, _value, utf8name, value);
823+
template <typename ValueType>
824+
inline void Object::Set(const char* utf8name, const ValueType& value) {
825+
napi_status status =
826+
napi_set_named_property(_env, _value, utf8name, Value::From(_env, value));
720827
NAPI_THROW_IF_FAILED(_env, status);
721828
}
722829

723-
inline void Object::Set(const char* utf8name, napi_value value) {
724-
napi_status status = napi_set_named_property(_env, _value, utf8name, value);
725-
NAPI_THROW_IF_FAILED(_env, status);
726-
}
727-
728-
inline void Object::Set(const char* utf8name, const char* utf8value) {
729-
Set(utf8name, String::New(Env(), utf8value));
730-
}
731-
732-
inline void Object::Set(const char* utf8name, bool boolValue) {
733-
Set(utf8name, Boolean::New(Env(), boolValue));
734-
}
735-
736-
inline void Object::Set(const char* utf8name, double numberValue) {
737-
Set(utf8name, Number::New(Env(), numberValue));
738-
}
739-
740-
inline void Object::Set(const std::string& utf8name, napi_value value) {
741-
Set(utf8name.c_str(), value);
742-
}
743-
744-
inline void Object::Set(const std::string& utf8name, Value value) {
830+
template <typename ValueType>
831+
inline void Object::Set(const std::string& utf8name, const ValueType& value) {
745832
Set(utf8name.c_str(), value);
746833
}
747834

748-
inline void Object::Set(const std::string& utf8name, std::string& utf8value) {
749-
Set(utf8name.c_str(), String::New(Env(), utf8value));
750-
}
751-
752-
inline void Object::Set(const std::string& utf8name, bool boolValue) {
753-
Set(utf8name.c_str(), Boolean::New(Env(), boolValue));
754-
}
755-
756-
inline void Object::Set(const std::string& utf8name, double numberValue) {
757-
Set(utf8name.c_str(), Number::New(Env(), numberValue));
758-
}
759-
760835
inline bool Object::Delete(napi_value key) {
761836
bool result;
762837
napi_status status = napi_delete_property(_env, _value, key, &result);
@@ -793,32 +868,13 @@ inline Value Object::Get(uint32_t index) const {
793868
return Value(_env, value);
794869
}
795870

796-
inline void Object::Set(uint32_t index, napi_value value) {
797-
napi_status status = napi_set_element(_env, _value, index, value);
871+
template <typename ValueType>
872+
inline void Object::Set(uint32_t index, const ValueType& value) {
873+
napi_status status =
874+
napi_set_element(_env, _value, index, Value::From(_env, value));
798875
NAPI_THROW_IF_FAILED(_env, status);
799876
}
800877

801-
inline void Object::Set(uint32_t index, Value value) {
802-
napi_status status = napi_set_element(_env, _value, index, value);
803-
NAPI_THROW_IF_FAILED(_env, status);
804-
}
805-
806-
inline void Object::Set(uint32_t index, const char* utf8value) {
807-
Set(index, static_cast<napi_value>(String::New(Env(), utf8value)));
808-
}
809-
810-
inline void Object::Set(uint32_t index, const std::string& utf8value) {
811-
Set(index, static_cast<napi_value>(String::New(Env(), utf8value)));
812-
}
813-
814-
inline void Object::Set(uint32_t index, bool boolValue) {
815-
Set(index, static_cast<napi_value>(Boolean::New(Env(), boolValue)));
816-
}
817-
818-
inline void Object::Set(uint32_t index, double numberValue) {
819-
Set(index, static_cast<napi_value>(Number::New(Env(), numberValue)));
820-
}
821-
822878
inline bool Object::Delete(uint32_t index) {
823879
bool result;
824880
napi_status status = napi_delete_element(_env, _value, index, &result);

‎napi.h

Lines changed: 35 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,21 @@ namespace Napi {
120120
Value(); ///< Creates a new _empty_ Value instance.
121121
Value(napi_env env, napi_value value); ///< Wraps a N-API value primitive.
122122

123+
/// Creates a JS value from a C++ primitive.
124+
///
125+
/// `value` may be any of:
126+
/// - bool
127+
/// - Any integer type
128+
/// - Any floating point type
129+
/// - const char* (encoded using UTF-8, null-terminated)
130+
/// - const char16_t* (encoded using UTF-16-LE, null-terminated)
131+
/// - std::string (encoded using UTF-8)
132+
/// - std::u16string
133+
/// - napi::Value
134+
/// - napi_value
135+
template <typename T>
136+
static Value From(napi_env env, const T& value);
137+
123138
/// Converts to a N-API value primitive.
124139
///
125140
/// If the instance is _empty_, this returns `nullptr`.
@@ -268,6 +283,16 @@ namespace Napi {
268283
size_t length ///< Length of the string in 2-byte code units
269284
);
270285

286+
/// Creates a new String based on the original object's type.
287+
///
288+
/// `value` may be any of:
289+
/// - const char* (encoded using UTF-8, null-terminated)
290+
/// - const char16_t* (encoded using UTF-16-LE, null-terminated)
291+
/// - std::string (encoded using UTF-8)
292+
/// - std::u16string
293+
template <typename T>
294+
static String From(napi_env env, const T& value);
295+
271296
String(); ///< Creates a new _empty_ String instance.
272297
String(napi_env env, napi_value value); ///< Wraps a N-API value primitive.
273298

@@ -420,75 +445,31 @@ namespace Napi {
420445
) const;
421446

422447
/// Sets a property.
448+
template <typename ValueType>
423449
void Set(
424450
napi_value key, ///< Property key primitive
425-
napi_value value ///< Property value primitive
451+
const ValueType& value ///< Property value primitive
426452
);
427453

428454
/// Sets a property.
455+
template <typename ValueType>
429456
void Set(
430457
Value key, ///< Property key
431-
Value value ///< Property value
432-
);
433-
434-
/// Sets a named property.
435-
void Set(
436-
const char* utf8name, ///< UTF-8 encoded null-terminated property name
437-
napi_value value ///< Property value primitive
458+
const ValueType& value ///< Property value
438459
);
439460

440461
/// Sets a named property.
462+
template <typename ValueType>
441463
void Set(
442464
const char* utf8name, ///< UTF-8 encoded null-terminated property name
443-
Value value ///< Property value
444-
);
445-
446-
/// Sets a named property to a string value.
447-
void Set(
448-
const char* utf8name, ///< UTF-8 encoded null-terminated property name
449-
const char* utf8value ///< UTF-8 encoded null-terminated property value
450-
);
451-
452-
/// Sets a named property to a boolean value.
453-
void Set(
454-
const char* utf8name, ///< UTF-8 encoded null-terminated property name
455-
bool boolValue ///< Property value
456-
);
457-
458-
/// Sets a named property to a number value.
459-
void Set(
460-
const char* utf8name, ///< UTF-8 encoded null-terminated property name
461-
double numberValue ///< Property value
462-
);
463-
464-
/// Sets a named property.
465-
void Set(
466-
const std::string& utf8name, ///< UTF-8 encoded property name
467-
napi_value value ///< Property value primitive
465+
const ValueType& value
468466
);
469467

470468
/// Sets a named property.
469+
template <typename ValueType>
471470
void Set(
472471
const std::string& utf8name, ///< UTF-8 encoded property name
473-
Value value ///< Property value
474-
);
475-
476-
/// Sets a named property to a string value.
477-
void Set(
478-
const std::string& utf8name, ///< UTF-8 encoded property name
479-
std::string& utf8value ///< UTF-8 encoded property value
480-
);
481-
482-
/// Sets a named property to a boolean value.
483-
void Set(
484-
const std::string& utf8name, ///< UTF-8 encoded property name
485-
bool boolValue ///< Property value
486-
);
487-
488-
/// Sets a named property to a number value.
489-
void Set(
490-
const std::string& utf8name, ///< UTF-8 encoded property name
491-
double numberValue ///< Property value
472+
const ValueType& value ///< Property value primitive
492473
);
493474

494475
/// Delete property.
@@ -522,39 +503,10 @@ namespace Napi {
522503
) const;
523504

524505
/// Sets an indexed property or array element.
506+
template <typename ValueType>
525507
void Set(
526508
uint32_t index, ///< Property / element index
527-
napi_value value ///< Property value primitive
528-
);
529-
530-
/// Sets an indexed property or array element.
531-
void Set(
532-
uint32_t index, ///< Property / element index
533-
Value value ///< Property value
534-
);
535-
536-
/// Sets an indexed property or array element to a string value.
537-
void Set(
538-
uint32_t index, ///< Property / element index
539-
const char* utf8value ///< UTF-8 encoded null-terminated property value
540-
);
541-
542-
/// Sets an indexed property or array element to a string value.
543-
void Set(
544-
uint32_t index, ///< Property / element index
545-
const std::string& utf8value ///< UTF-8 encoded property value
546-
);
547-
548-
/// Sets an indexed property or array element to a boolean value.
549-
void Set(
550-
uint32_t index, ///< Property / element index
551-
bool boolValue ///< Property value
552-
);
553-
554-
/// Sets an indexed property or array element to a number value.
555-
void Set(
556-
uint32_t index, ///< Property / element index
557-
double numberValue ///< Property value
509+
const ValueType& value ///< Property value primitive
558510
);
559511

560512
/// Deletes an indexed property or array element.

0 commit comments

Comments
 (0)