apply review suggestions

Author: geeksilva97

doc/api/sqlite.md

@@ -248,9 +248,9 @@ added: REPLACEME
 -->
 
 * `dbName` {string} Name of the database. This can be `'main'` (the default primary database) or any other
-  database that have been added with [`ATTACH DATABASE`][] **Default:** `'main'`.
-* Returns: {string} The location of the database file. When using an `in-memory` database,
-  this method returns an empty string.
+  database that has been added with [`ATTACH DATABASE`][] **Default:** `'main'`.
+* Returns: {string | null} The location of the database file. When using an in-memory database,
+  this method returns null.
 
 This method is a wrapper around [`sqlite3_db_filename()`][]
 

src/node_sqlite.cc

@@ -1181,7 +1181,7 @@ void DatabaseSync::Location(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
   THROW_AND_RETURN_ON_BAD_STATE(env, !db->IsOpen(), "database is not open");
 
-  std::string db_name = "main";
+  std::string_view db_name = "main";
   if (!args[0]->IsUndefined()) {
     if (!args[0]->IsString()) {
       THROW_ERR_INVALID_ARG_TYPE(env->isolate(),
@@ -1193,11 +1193,17 @@ void DatabaseSync::Location(const FunctionCallbackInfo<Value>& args) {
   }
 
   const char* db_filename =
-      sqlite3_db_filename(db->connection_, db_name.c_str());
+      sqlite3_db_filename(db->connection_, std::string(db_name).c_str());
+  if (!db_filename || db_filename[0] == '\0') {
+    args.GetReturnValue().Set(Null(env->isolate()));
+    return;
+  }
 
-  args.GetReturnValue().Set(
-      String::NewFromUtf8(env->isolate(), db_filename, NewStringType::kNormal)
-          .ToLocalChecked());
+  Local<String> ret;
+  if (String::NewFromUtf8(env->isolate(), db_filename).ToLocal(&ret)) {
+    args.GetReturnValue().Set(ret);
+    return;
+  }
 }
 
 void DatabaseSync::AggregateFunction(const FunctionCallbackInfo<Value>& args) {
@@ -1320,7 +1326,7 @@ void DatabaseSync::AggregateFunction(const FunctionCallbackInfo<Value>& args) {
       return;
     }
 
-    // Subract 1 because the first argument is the aggregate value.
+    // Subtract 1 because the first argument is the aggregate value.
     argc = js_len.As<Int32>()->Value() - 1;
     if (!inverseFunc.IsEmpty() &&
         !inverseFunc->Get(env->context(), env->length_string())
@@ -2632,7 +2638,8 @@ static void Initialize(Local<Object> target,
   SetProtoMethod(isolate, db_tmpl, "prepare", DatabaseSync::Prepare);
   SetProtoMethod(isolate, db_tmpl, "exec", DatabaseSync::Exec);
   SetProtoMethod(isolate, db_tmpl, "function", DatabaseSync::CustomFunction);
-  SetProtoMethod(isolate, db_tmpl, "location", DatabaseSync::Location);
+  SetProtoMethodNoSideEffect(
+      isolate, db_tmpl, "location", DatabaseSync::Location);
   SetProtoMethod(
       isolate, db_tmpl, "aggregate", DatabaseSync::AggregateFunction);
   SetProtoMethod(

test/parallel/test-sqlite-database-sync.js

@@ -339,6 +339,7 @@ suite('DatabaseSync.prototype.location()', () => {
 
   test('throws if provided dbName is not string', (t) => {
     const db = new DatabaseSync(nextDb());
+    t.after(() => { db.close(); });
 
     t.assert.throws(() => {
       db.location(null);
@@ -348,16 +349,16 @@ suite('DatabaseSync.prototype.location()', () => {
     });
   });
 
-  test('returns empty string when connected to in-memory database', (t) => {
+  test('returns null when connected to in-memory database', (t) => {
     const db = new DatabaseSync(':memory:');
-    t.assert.equal(db.location(), '');
+    t.assert.strictEqual(db.location(), null);
   });
 
   test('returns db path when connected to a persistent database', (t) => {
     const dbPath = nextDb();
     const db = new DatabaseSync(dbPath);
     t.after(() => { db.close(); });
-    t.assert.equal(db.location(), dbPath);
+    t.assert.strictEqual(db.location(), dbPath);
   });
 
   test('returns that specific db path when attached', (t) => {
@@ -372,6 +373,6 @@ suite('DatabaseSync.prototype.location()', () => {
     const escapedPath = otherPath.replace("'", "''");
     db.exec(`ATTACH DATABASE '${escapedPath}' AS other`);
 
-    t.assert.equal(db.location('other'), otherPath);
+    t.assert.strictEqual(db.location('other'), otherPath);
   });
 });