Update PR based on comments received

nodejs · Apr 28, 2021 · 0366264 · 0366264
1 parent f49c894
commit 0366264
Show file tree

Hide file tree

Showing 4 changed files with 18 additions and 16 deletions.
diff --git a/doc/symbol.md b/doc/symbol.md
@@ -34,7 +34,7 @@ If an error occurs, a `Napi::Error` will get thrown. If C++ exceptions are not
 being used, callers should check the result of `Napi::Env::IsExceptionPending` before
 attempting to use the returned value.
 
-### Wellkown
+### WellKnown
 ```cpp
 static Napi::Symbol Napi::Symbol::WellKnown(napi_env env, const std::string& name);
 ```
@@ -47,12 +47,15 @@ Returns a `Napi::Symbol` representing a well-known `Symbol` from the
 
 ### For
 ```cpp
-static Napi::Symbol Napi::Symbol::WellKnown(napi_env env, const std::string& name);
+static Napi::Symbol Napi::Symbol::For(napi_env env, const std::string& description);
+static Napi::Symbol Napi::Symbol::For(napi_env env, const char* description = nullptr);
+static Napi::Symbol Napi::Symbol::For(napi_env env, String description);
+static Napi::Symbol Napi::Symbol::For(napi_env env, napi_value description);
 ```
 
 - `[in] env`: The `napi_env` environment in which to construct the `Napi::Symbol` object.
-- `[in] name`: The C++ string representing the `Napi::Symbol` to retrieve.
+- `[in] description`: The C++ string representing the `Napi::Symbol` in the global registry to retrieve.
 
-Register `Napi::Symbol` in the global registry. If symbol already exist retrieve said symbol. Equivalent to `Symbol.for("symb")` called from JS.
+Searches in the global registry for existing symbol with the given name. If the symbol already exist it will be returned, otherwise a new symbol will be created in the registry. It's equivalent to Symbol.for() called from JavaScript.
 
-[`Napi::Name`]: ./name.md
+[`Napi::Name`]: ./name.md
diff --git a/napi-inl.h b/napi-inl.h
@@ -1011,8 +1011,7 @@ inline Symbol Symbol::For(napi_env env, const char* description) {
 }
 
 inline Symbol Symbol::For(napi_env env, String description) {
-  napi_value descriptionValue = description;
-  return Symbol::For(env, descriptionValue);
+  return Symbol::For(env, static_cast<napi_value>(description));
 }
 
 inline Symbol Symbol::For(napi_env env, napi_value description) {

diff --git a/napi.h b/napi.h
@@ -539,7 +539,7 @@ namespace Napi {
    static Symbol WellKnown(napi_env, const std::string& name);
 
    // Create a symbol in the global registry, UTF-8 Encoded cpp string
-   static Symbol For(napi_env env, const std::string& name);
+   static Symbol For(napi_env env, const std::string& description);
 
    // Create a symbol in the global registry, C style string (null terminated)
    static Symbol For(napi_env env, const char* description = nullptr);

diff --git a/test/symbol.js b/test/symbol.js
@@ -53,14 +53,14 @@ async function test(binding)
     assertSymbolAreUnique("symbol"); 
     assertSymbolIsNotWellknown("testing");
 
-    for(const wellknownProperty of wellKnownSymbolFunctions)
-    {
-      assertSymbolIsWellknown(wellknownProperty);
-    }
+     for(const wellknownProperty of wellKnownSymbolFunctions)
+     {
+       assertSymbolIsWellknown(wellknownProperty);
+     }
 
-    assertCanCreateOrFetchGlobalSymbols("data", binding.symbol.getSymbolFromGlobalRegistry);
-    assertCanCreateOrFetchGlobalSymbols("CppKey", binding.symbol.getSymbolFromGlobalRegistryWithCppKey);
-    assertCanCreateOrFetchGlobalSymbols("CKey", binding.symbol.getSymbolFromGlobalRegistryWithCKey);
+     assertCanCreateOrFetchGlobalSymbols("data", binding.symbol.getSymbolFromGlobalRegistry);
+     assertCanCreateOrFetchGlobalSymbols("CppKey", binding.symbol.getSymbolFromGlobalRegistryWithCppKey);
+     assertCanCreateOrFetchGlobalSymbols("CKey", binding.symbol.getSymbolFromGlobalRegistryWithCKey);
 
     assert(binding.symbol.createNewSymbolWithNoArgs() === undefined);
-}
+}