Improvements and fixes for npm client resolution

smapiot · Dec 9, 2024 · 4d4edf9 · 4d4edf9
1 parent 06f4f2d
commit 4d4edf9
Show file tree

Hide file tree

Showing 5 changed files with 138 additions and 61 deletions.
diff --git a/src/tooling/piral-cli/src/common/npm.ts b/src/tooling/piral-cli/src/common/npm.ts
@@ -5,10 +5,10 @@ import { config } from './config';
 import { legacyCoreExternals, frameworkLibs, defaultRegistry, packageJson } from './constants';
 import { inspectPackage } from './inspect';
 import { readJson, checkExists } from './io';
-import { clients, detectClients, isWrapperClient } from '../npm-clients';
+import { clients, detectDirectClients, detectWrapperClients, isDirectClient, isWrapperClient } from '../npm-clients';
 import { clientTypeKeys } from '../helpers';
 import { getModulePath } from '../external';
-import { PackageType, NpmClientType } from '../types';
+import { PackageType, NpmClientType, NpmClient, NpmDirectClientType, NpmWapperClientType } from '../types';
 
 const gitPrefix = 'git+';
 const filePrefix = 'file:';
@@ -63,56 +63,88 @@ async function detectMonorepoRoot(root: string): Promise<[] | [string, NpmClient
   return [];
 }
 
-/**
- * For details about how this works consult issue
- * https://github.com/smapiot/piral/issues/203
- * @param root The project's root directory.
- */
-export async function determineNpmClient(root: string, selected?: NpmClientType): Promise<NpmClientType> {
-  if (!selected || !clientTypeKeys.includes(selected)) {
-    log('generalDebug_0003', 'No npm client selected. Checking for lock or config files ...');
+async function determineWrapperClient(root: string): Promise<NpmWapperClientType | undefined> {
+  const searchedClients = await detectWrapperClients(root);
+  const foundClients = searchedClients.filter((m) => m.result).map((m) => m.client);
 
-    const searchedClients = await detectClients(root);
-    const foundClients = searchedClients.filter((m) => m.result);
-
-    log(
-      'generalDebug_0003',
-      `Results of the lock file check: ${searchedClients.map((m) => `${m.client}=${m.result}`).join(', ')}`,
-    );
+  if (foundClients.length > 0) {
+    const [client] = foundClients;
 
     if (foundClients.length > 1) {
-      const wrapperClient = foundClients.find((m) => isWrapperClient(m.client));
-
-      if (wrapperClient) {
-        const { client } = wrapperClient;
-        log('generalDebug_0003', `Found valid wrapper client via lock or config file: "${client}".`);
-      }
+      log(
+        'generalWarning_0001',
+        `Found multiple clients via their lock or config files: "${foundClients.join('", "')}".`,
+      );
     }
 
-    if (foundClients.length > 0) {
-      const { client } = foundClients[0];
+    log('generalDebug_0003', `Found valid direct client via lock or config file: "${client}".`);
+    return client;
+  }
 
-      if (foundClients.length > 1) {
-        const clientStr = `"${foundClients.map((m) => m.client).join('", "')}"`;
-        log('generalWarning_0001', `Found multiple clients via their lock or config files: ${clientStr}.`);
-      }
+  const defaultClient = config.npmClient;
 
-      log('generalDebug_0003', `Found valid direct client via lock or config file: "${client}".`);
-      return client;
-    }
+  if (isWrapperClient(defaultClient)) {
+    log('generalDebug_0003', `Using the default client: "${defaultClient}".`);
+    return defaultClient;
+  }
+
+  return undefined;
+}
 
-    const defaultClient = config.npmClient;
+async function determineDirectClient(root: string): Promise<NpmDirectClientType> {
+  const searchedClients = await detectDirectClients(root);
+  const foundClients = searchedClients.filter((m) => m.result).map((m) => m.client);
 
-    if (clientTypeKeys.includes(defaultClient)) {
-      log('generalDebug_0003', `Using the default client: "${defaultClient}".`);
-      return defaultClient;
+  if (foundClients.length > 0) {
+    const [client] = foundClients;
+
+    if (foundClients.length > 1) {
+      log(
+        'generalWarning_0001',
+        `Found multiple clients via their lock or config files: "${foundClients.join('", "')}".`,
+      );
     }
 
-    log('generalDebug_0003', 'Using the fallback "npm" client.');
-    return 'npm';
+    log('generalDebug_0003', `Found valid direct client via lock or config file: "${client}".`);
+    return client;
+  }
+
+  const defaultClient = config.npmClient;
+
+  if (isDirectClient(defaultClient)) {
+    log('generalDebug_0003', `Using the default client: "${defaultClient}".`);
+    return defaultClient;
   }
 
-  return selected;
+  log('generalDebug_0003', 'Using the fallback "npm" client.');
+  return 'npm';
+}
+
+/**
+ * For details about how this works consult issue
+ * https://github.com/smapiot/piral/issues/203
+ * @param root The project's root directory.
+ */
+export async function determineNpmClient(root: string, selected?: NpmClientType): Promise<NpmClient> {
+  if (!selected || !clientTypeKeys.includes(selected)) {
+    log('generalDebug_0003', 'No npm client selected. Checking for lock or config files ...');
+    const [direct, wrapper] = await Promise.all([determineDirectClient(root), determineWrapperClient(root)]);
+    return {
+      direct,
+      wrapper,
+    };
+  } else if (isDirectClient(selected)) {
+    return {
+      proposed: selected,
+      direct: selected,
+    };
+  } else {
+    return {
+      proposed: selected,
+      direct: await determineDirectClient(root),
+      wrapper: selected,
+    };
+  }
 }
 
 export async function isMonorepoPackageRef(refName: string, root: string): Promise<boolean> {
@@ -126,35 +158,37 @@ export async function isMonorepoPackageRef(refName: string, root: string): Promi
   return false;
 }
 
-export function installNpmDependencies(client: NpmClientType, target = '.'): Promise<string> {
-  const { installDependencies } = clients[client];
+export function installNpmDependencies(client: NpmClient, target = '.'): Promise<string> {
+  const { installDependencies } = clients[client.direct];
   return installDependencies(target);
 }
 
 export async function installNpmPackageFromOptionalRegistry(
   packageRef: string,
-  target = '.',
+  target: string,
   registry: string,
 ): Promise<void> {
+  const client = await determineNpmClient(target, 'npm');
+
   try {
-    await installNpmPackage('npm', packageRef, target, '--registry', registry);
+    await installNpmPackage(client, packageRef, target, '--registry', registry);
   } catch (e) {
     if (registry === defaultRegistry) {
       throw e;
     }
 
-    await installNpmPackage('npm', packageRef, target, '--registry', defaultRegistry);
+    await installNpmPackage(client, packageRef, target, '--registry', defaultRegistry);
   }
 }
 
 export async function uninstallNpmPackage(
-  client: NpmClientType,
+  client: NpmClient,
   packageRef: string,
   target = '.',
   ...flags: Array<string>
 ): Promise<string> {
   try {
-    const { uninstallPackage } = clients[client];
+    const { uninstallPackage } = clients[client.direct];
     return await uninstallPackage(packageRef, target, ...flags);
   } catch (ex) {
     log(
@@ -166,13 +200,13 @@ export async function uninstallNpmPackage(
 }
 
 export async function installNpmPackage(
-  client: NpmClientType,
+  client: NpmClient,
   packageRef: string,
   target = '.',
   ...flags: Array<string>
 ): Promise<string> {
   try {
-    const { installPackage } = clients[client];
+    const { installPackage } = clients[client.direct];
     return await installPackage(packageRef, target, ...flags);
   } catch (ex) {
     log(
@@ -183,8 +217,8 @@ export async function installNpmPackage(
   }
 }
 
-export function initNpmProject(client: NpmClientType, projectName: string, target: string) {
-  const { initProject } = clients[client];
+export function initNpmProject(client: NpmClient, projectName: string, target: string) {
+  const { initProject } = clients[client.wrapper];
   return initProject(projectName, target);
 }
 

diff --git a/src/tooling/piral-cli/src/common/shell.ts b/src/tooling/piral-cli/src/common/shell.ts
@@ -5,7 +5,7 @@ import { readJson, updateExistingJson, writeJson } from './io';
 import { scaffoldFromEmulatorWebsite } from './website';
 import { combinePackageRef, getPackageName, getPackageVersion } from './npm';
 import { dissectPackageName, installNpmPackage, isLinkedPackage } from './npm';
-import { NpmClientType, PackageType, PiralInstanceDetails } from '../types';
+import { NpmClient, PackageType, PiralInstanceDetails } from '../types';
 
 async function updatePiletJson(target: string, appName: string, appDetails: PiralInstanceDetails) {
   const oldContent = await readJson(target, piletJson);
@@ -30,7 +30,7 @@ async function setupPiralInstance(
   hadVersion: boolean,
   rootDir: string,
   sourceVersion: string,
-  npmClient: NpmClientType,
+  npmClient: NpmClient,
 ) {
   if (!isLinkedPackage(sourceName, type, hadVersion, rootDir)) {
     const packageRef = combinePackageRef(sourceName, sourceVersion, type);
@@ -55,7 +55,7 @@ export async function installPiralInstance(
   usedSource: string,
   baseDir: string,
   rootDir: string,
-  npmClient: NpmClientType,
+  npmClient: NpmClient,
   agent: Agent,
   selected?: boolean,
 ): Promise<string> {

diff --git a/src/tooling/piral-cli/src/npm-clients/index.ts b/src/tooling/piral-cli/src/npm-clients/index.ts
@@ -1,5 +1,6 @@
 import { dirname, resolve } from 'path';
 import { findFile } from '../common/io';
+import type { NpmClientType, NpmDirectClientType, NpmWapperClientType } from '../types';
 
 import * as lerna from './lerna';
 import * as npm from './npm';
@@ -19,20 +20,26 @@ export const clients = {
   bun,
 };
 
-type ClientName = keyof typeof clients;
+const directClients: Array<NpmDirectClientType> = ['npm', 'pnp', 'yarn', 'pnpm', 'bun'];
+const wrapperClients: Array<NpmWapperClientType> = ['lerna', 'rush'];
 
-const directClients = ['npm', 'pnp', 'yarn', 'pnpm', 'bun'];
+export function isWrapperClient(client: NpmClientType): client is NpmWapperClientType {
+  return wrapperClients.includes(client as any);
+}
 
-export function isWrapperClient(client: ClientName) {
-  return !directClients.includes(client);
+export function isDirectClient(client: NpmClientType): client is NpmDirectClientType {
+  return directClients.includes(client as any);
 }
 
-export async function detectClients(root: string) {
+async function detectClients<T extends NpmClientType>(
+  root: string,
+  clientNames: Array<T>,
+): Promise<Array<{ client: T; result: boolean }>> {
   const packageJson = await findFile(resolve(root, '..'), 'package.json');
   const stopDir = packageJson ? dirname(packageJson) : undefined;
 
   return await Promise.all(
-    Object.keys(clients).map(async (client: ClientName) => {
+    clientNames.map(async (client) => {
       const result = await clients[client].detectClient(root, stopDir);
       return {
         client,
@@ -41,3 +48,11 @@ export async function detectClients(root: string) {
     }),
   );
 }
+
+export function detectDirectClients(root: string) {
+  return detectClients<NpmDirectClientType>(root, directClients);
+}
+
+export function detectWrapperClients(root: string) {
+  return detectClients<NpmWapperClientType>(root, wrapperClients);
+}
diff --git a/src/tooling/piral-cli/src/types/internal.ts b/src/tooling/piral-cli/src/types/internal.ts
@@ -1,5 +1,11 @@
 import type { LogLevels } from './common';
-import type { ImportmapVersions, PiletSchemaVersion } from './public';
+import type {
+  ImportmapVersions,
+  NpmClientType,
+  NpmDirectClientType,
+  NpmWapperClientType,
+  PiletSchemaVersion,
+} from './public';
 
 export interface PiralInstanceDetails {
   selected?: boolean;
@@ -34,3 +40,21 @@ export interface FileInfo {
  * 2. The (short) error message
  */
 export type QuickMessage = [LogLevels, string, string];
+
+/**
+ * Result of identifying an npm client in a project.
+ */
+export interface NpmClient {
+  /**
+   * The proposed client - can be anything.
+   */
+  proposed?: NpmClientType;
+  /**
+   * The direct npm client.
+   */
+  direct: NpmDirectClientType;
+  /**
+   * The wrapper npm client, if any.
+   */
+  wrapper?: NpmWapperClientType;
+}
diff --git a/src/tooling/piral-cli/src/types/public.ts b/src/tooling/piral-cli/src/types/public.ts
@@ -261,7 +261,11 @@ export type PiletBuildType = 'default' | 'standalone' | 'manifest';
 
 export type PackageType = 'registry' | 'file' | 'git' | 'remote';
 
-export type NpmClientType = 'npm' | 'yarn' | 'pnp' | 'pnpm' | 'lerna' | 'rush' | 'bun';
+export type NpmDirectClientType = 'npm' | 'yarn' | 'pnp' | 'pnpm' | 'bun';
+
+export type NpmWapperClientType = 'lerna' | 'rush';
+
+export type NpmClientType = NpmDirectClientType | NpmWapperClientType;
 
 export type Framework = 'piral' | 'piral-core' | 'piral-base';