From f537c43ef0c6b9649ebdaecc9b168bf430a477fd Mon Sep 17 00:00:00 2001
From: Ramon Brullo <kingvanti650@gmail.com>
Date: Tue, 16 Jan 2024 10:20:04 +0100
Subject: [PATCH] refactor: reuse function

Logic for adding scope to scoped registry was somewhat duplicated. Partly it was inside `scoped-registry/addScope` and then also replicated in `cmd-add`. Refactored so `cmd-add` also used the `addScope` function.
---
 src/cmd-add.ts               | 11 +++--------
 src/types/scoped-registry.ts |  9 +++++++--
 test/test-scoped-registry.ts | 10 ++++++++--
 3 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/src/cmd-add.ts b/src/cmd-add.ts
index 8427a9ee..b22bb66c 100644
--- a/src/cmd-add.ts
+++ b/src/cmd-add.ts
@@ -23,7 +23,7 @@ import {
   PackageReference,
   splitPackageReference,
 } from "./types/package-reference";
-import { scopedRegistry } from "./types/scoped-registry";
+import { addScope, scopedRegistry } from "./types/scoped-registry";
 import {
   addDependency,
   addScopedRegistry,
@@ -219,15 +219,10 @@ export const add = async function (
         addScopedRegistry(manifest, entry);
         dirty = true;
       }
-      // apply pkgsInScope
-      const scopesSet = new Set(entry.scopes || []);
       pkgsInScope.forEach((name) => {
-        if (!scopesSet.has(name)) {
-          scopesSet.add(name);
-          dirty = true;
-        }
+        const wasAdded = addScope(entry!, name);
+        if (wasAdded) dirty = true;
       });
-      entry.scopes = Array.from(scopesSet).sort();
     }
     if (options.test) addTestable(manifest, name);
     // save manifest
diff --git a/src/types/scoped-registry.ts b/src/types/scoped-registry.ts
index 33b6bfa4..0149a680 100644
--- a/src/types/scoped-registry.ts
+++ b/src/types/scoped-registry.ts
@@ -36,13 +36,18 @@ export function scopedRegistry(
 }
 
 /**
- * Adds a scope to a registry. The list will be sorted alphabetically.
+ * Adds a scope to a registry if it is not already in the list. The scopes will
+ * also be sorted alphabetically.
  * @param registry The registry.
  * @param scope The scope.
+ * @returns Boolean whether the scope was added successfully.
  */
-export function addScope(registry: ScopedRegistry, scope: DomainName) {
+export function addScope(registry: ScopedRegistry, scope: DomainName): boolean {
+  if (registry.scopes.includes(scope)) return false;
+
   registry.scopes.push(scope);
   registry.scopes.sort();
+  return true;
 }
 
 /**
diff --git a/test/test-scoped-registry.ts b/test/test-scoped-registry.ts
index 0a9fc7ac..91c89c28 100644
--- a/test/test-scoped-registry.ts
+++ b/test/test-scoped-registry.ts
@@ -19,10 +19,16 @@ describe("scoped-registry", function () {
   describe("add scope", function () {
     it("should keep scope-list alphabetical", () => {
       const registry = scopedRegistry("test", exampleRegistryUrl);
-      addScope(registry, domainName("b"));
-      addScope(registry, domainName("a"));
+      should(addScope(registry, domainName("b"))).be.true();
+      should(addScope(registry, domainName("a"))).be.true();
       should(registry.scopes).be.deepEqual(["a", "b"]);
     });
+    it("should filter duplicates", () => {
+      const registry = scopedRegistry("test", exampleRegistryUrl);
+      should(addScope(registry, domainName("a"))).be.true();
+      should(addScope(registry, domainName("a"))).be.false();
+      should(registry.scopes).be.deepEqual(["a"]);
+    });
   });
   describe("has scope", function () {
     it("should have scope that was added", () => {