refactor: extract npm client param (#100)

* refactor: make comment jsdoc * doc: document function params * refactor: extract parameter Make `fetchPackument` function take the npm-client as a parameter. That way it will not be recreated for each call. * refactor: make comment jsdoc * doc: document function params * refactor: make function param required The function gains nothing by the param being undefiend. Make it required. * refactor: extract parameter Make `fetchPackageDependencies` function take the npm-client as a parameter. That way it will not be recreated for each call.
openupm · Jan 14, 2024 · 1bba178 · 1bba178
1 parent 432d923
commit 1bba178
Show file tree

Hide file tree

Showing 5 changed files with 50 additions and 19 deletions.
diff --git a/src/cmd-add.ts b/src/cmd-add.ts
@@ -11,7 +11,11 @@ import {
   compareEditorVersion,
   tryParseEditorVersion,
 } from "./types/editor-version";
-import { fetchPackageDependencies, fetchPackument } from "./registry-client";
+import {
+  fetchPackageDependencies,
+  fetchPackument,
+  getNpmClient,
+} from "./registry-client";
 import { DomainName } from "./types/domain-name";
 import { SemanticVersion } from "./types/semantic-version";
 import {
@@ -52,6 +56,8 @@ export const add = async function (
   const env = await parseEnv(options, true);
   if (env === null) return 1;
 
+  const client = getNpmClient();
+
   const addSingle = async function (pkg: PackageReference): Promise<AddResult> {
     // dirty flag
     let dirty = false;
@@ -69,9 +75,9 @@ export const add = async function (
     const pkgsInScope: DomainName[] = [];
     if (version === undefined || !isPackageUrl(version)) {
       // verify name
-      let packument = await fetchPackument(env.registry, name);
+      let packument = await fetchPackument(env.registry, name, client);
       if (!packument && env.upstream) {
-        packument = await fetchPackument(env.upstreamRegistry, name);
+        packument = await fetchPackument(env.upstreamRegistry, name, client);
         if (packument) isUpstreamPackage = true;
       }
       if (!packument) {
@@ -150,7 +156,8 @@ export const add = async function (
           env.upstreamRegistry,
           name,
           version,
-          true
+          true,
+          client
         );
         // add depsValid to pkgsInScope.
         depsValid

diff --git a/src/cmd-deps.ts b/src/cmd-deps.ts
@@ -1,6 +1,6 @@
 import log from "./logger";
 import { parseEnv } from "./utils/env";
-import { fetchPackageDependencies } from "./registry-client";
+import { fetchPackageDependencies, getNpmClient } from "./registry-client";
 import { isPackageUrl } from "./types/package-url";
 import {
   packageReference,
@@ -26,6 +26,8 @@ export const deps = async function (
   const env = await parseEnv(options, false);
   if (env === null) return 1;
 
+  const client = getNpmClient();
+
   const [name, version] = splitPackageReference(pkg);
 
   if (version !== undefined && isPackageUrl(version))
@@ -36,7 +38,8 @@ export const deps = async function (
     env.upstreamRegistry,
     name,
     version,
-    options.deep
+    options.deep || false,
+    client
   );
   depsValid
     .filter((x) => !x.self)

diff --git a/src/cmd-view.ts b/src/cmd-view.ts
@@ -3,7 +3,7 @@ import log from "./logger";
 import assert from "assert";
 import { tryGetLatestVersion, UnityPackument } from "./types/packument";
 import { parseEnv } from "./utils/env";
-import { fetchPackument } from "./registry-client";
+import { fetchPackument, getNpmClient } from "./registry-client";
 import { DomainName } from "./types/domain-name";
 import {
   packageReference,
@@ -23,6 +23,8 @@ export const view = async function (
   // parse env
   const env = await parseEnv(options, false);
   if (env === null) return 1;
+  const client = getNpmClient();
+
   // parse name
   const [name, version] = splitPackageReference(pkg);
   if (version) {
@@ -33,9 +35,9 @@ export const view = async function (
     return 1;
   }
   // verify name
-  let packument = await fetchPackument(env.registry, name);
+  let packument = await fetchPackument(env.registry, name, client);
   if (!packument && env.upstream)
-    packument = await fetchPackument(env.upstreamRegistry, name);
+    packument = await fetchPackument(env.upstreamRegistry, name, client);
   if (!packument) {
     log.error("404", `package not found: ${name}`);
     return 1;

diff --git a/src/registry-client.ts b/src/registry-client.ts
@@ -112,27 +112,42 @@ export const getNpmClient = (): NpmClient => {
     adduser: normalizeClientFunction(client, client.adduser),
   };
 };
-// Fetch package info json from registry
+
+/**
+ * Fetch package info json from registry
+ * @param registry The registry from which to get the packument
+ * @param name The name of the packument
+ * @param client The client to use for fetching
+ */
 export const fetchPackument = async function (
   registry: Registry,
-  name: DomainName
+  name: DomainName,
+  client: NpmClient
 ): Promise<UnityPackument | undefined> {
   const pkgPath = `${registry.url}/${name}`;
-  const client = getNpmClient();
   try {
     return await client.get(pkgPath, { auth: registry.auth || undefined });
   } catch (err) {
     /* empty */
   }
 };
 
-// Fetch package dependencies
+/**
+ * Fetch package dependencies
+ * @param registry The registry in which to search the dependencies
+ * @param upstreamRegistry The upstream registry in which to search as a backup
+ * @param name The name of the package
+ * @param version The version for which to search dependencies
+ * @param deep Whether to search for all dependencies
+ * @param client The client to use for communicating with the registries
+ */
 export const fetchPackageDependencies = async function (
   registry: Registry,
   upstreamRegistry: Registry,
   name: DomainName,
   version: SemanticVersion | "latest" | undefined,
-  deep?: boolean
+  deep: boolean,
+  client: NpmClient
 ): Promise<[Dependency[], Dependency[]]> {
   log.verbose(
     "dependency",
@@ -184,7 +199,8 @@ export const fetchPackageDependencies = async function (
         }
         // try fetching package info from the default registry
         if (packument === null) {
-          packument = (await fetchPackument(registry, entry.name)) ?? null;
+          packument =
+            (await fetchPackument(registry, entry.name, client)) ?? null;
           if (packument) {
             depObj.upstream = false;
             cachedPackageInfoDict[entry.name] = {
@@ -196,7 +212,8 @@ export const fetchPackageDependencies = async function (
         // try fetching package info from the upstream registry
         if (!packument) {
           packument =
-            (await fetchPackument(upstreamRegistry, entry.name)) ?? null;
+            (await fetchPackument(upstreamRegistry, entry.name, client)) ??
+            null;
           if (packument) {
             depObj.upstream = true;
             cachedPackageInfoDict[entry.name] = {

diff --git a/test/test-registry-client.ts b/test/test-registry-client.ts
@@ -1,7 +1,7 @@
 import "assert";
 import "should";
 import { parseEnv } from "../src/utils/env";
-import { fetchPackument } from "../src/registry-client";
+import { fetchPackument, getNpmClient } from "../src/registry-client";
 import {
   exampleRegistryUrl,
   registerMissingPackument,
@@ -17,6 +17,8 @@ const packageA = domainName("package-a");
 
 describe("registry-client", function () {
   describe("fetchPackageInfo", function () {
+    const client = getNpmClient();
+
     beforeEach(function () {
       startMockRegistry();
     });
@@ -31,7 +33,7 @@ describe("registry-client", function () {
       should(env).not.be.null();
       const packumentRemote = buildPackument(packageA);
       registerRemotePackument(packumentRemote);
-      const info = await fetchPackument(env!.registry, packageA);
+      const info = await fetchPackument(env!.registry, packageA, client);
       should(info).deepEqual(packumentRemote);
     });
     it("404", async function () {
@@ -41,7 +43,7 @@ describe("registry-client", function () {
       );
       should(env).not.be.null();
       registerMissingPackument(packageA);
-      const info = await fetchPackument(env!.registry, packageA);
+      const info = await fetchPackument(env!.registry, packageA, client);
       (info === undefined).should.be.ok();
     });
   });