fix(align-deps): --write should add capabilities when used with `--…

…no-unmanaged` (#3029) * fix(align-deps): `--write` should add capabilities when used with `--no-unmanaged` * fixup! fix(align-deps): `--write` should add capabilities when used with `--no-unmanaged` * fixup! fix(align-deps): `--write` should add capabilities when used with `--no-unmanaged`
microsoft · Mar 25, 2024 · e7ad713 · e7ad713
1 parent cbb5f4f
commit e7ad713
Show file tree

Hide file tree

Showing 6 changed files with 286 additions and 107 deletions.
diff --git a/.changeset/popular-beers-shop.md b/.changeset/popular-beers-shop.md
@@ -0,0 +1,5 @@
+---
+"@rnx-kit/align-deps": patch
+---
+
+`--write` should add capabilities when used with `--no-unmanaged`
diff --git a/packages/align-deps/src/commands/vigilant.ts b/packages/align-deps/src/commands/vigilant.ts
@@ -1,5 +1,5 @@
-import type { Capability } from "@rnx-kit/config";
-import { error, info, warn } from "@rnx-kit/console";
+import type { Capability, KitConfig } from "@rnx-kit/config";
+import { error, warn } from "@rnx-kit/console";
 import { keysOf } from "@rnx-kit/tools-language/properties";
 import type { PackageManifest } from "@rnx-kit/tools-node/package";
 import * as path from "path";
@@ -10,7 +10,7 @@ import {
   resolveCapabilitiesUnchecked,
 } from "../capabilities";
 import { stringify } from "../diff";
-import { modifyManifest } from "../helpers";
+import { dependencySections, modifyManifest } from "../helpers";
 import { updateDependencies } from "../manifest";
 import { ensurePreset, filterPreset, mergePresets } from "../preset";
 import type {
@@ -24,9 +24,9 @@ import type {
 } from "../types";
 
 type Report = {
-  changes: Changes;
-  changesCount: number;
-  unmanagedDependencies: [string, string][];
+  errors: Changes;
+  errorCount: number;
+  warnings: Changes["capabilities"];
 };
 
 function getAllCapabilities(preset: Preset): Capability[] {
@@ -190,24 +190,25 @@ const buildManifestProfileCached = ((): typeof buildManifestProfile => {
  *
  * @param manifest The package manifest
  * @param profile The desired profile to compare against
- * @param write Whether misaligned dependencies should be updated
+ * @param options Whether misaligned dependencies should be updated
  * @returns A list of misaligned dependencies
  */
 export function inspect(
   manifest: PackageManifest,
   profile: ManifestProfile,
-  write: boolean
+  { noUnmanaged, write }: Pick<Options, "noUnmanaged" | "write">
 ): Report {
-  const allChanges: Report["changes"] = {
+  const errors: Report["errors"] = {
     dependencies: [],
     peerDependencies: [],
     devDependencies: [],
+    capabilities: [],
   };
-  const capabilities: Record<string, string> = {};
+  const extraCapabilities: Record<string, string> = {};
 
   const { unmanagedCapabilities } = profile;
 
-  const changesCount = keysOf(allChanges).reduce((count, section) => {
+  const errorCount = dependencySections.reduce((count, section) => {
     const dependencies = manifest[section];
     if (!dependencies) {
       return count;
@@ -216,7 +217,7 @@ export function inspect(
     const isMisaligned =
       section === "peerDependencies" ? isMisalignedPeer : isMisalignedDirect;
     const desiredDependencies = profile[section];
-    const changes = allChanges[section];
+    const changes = errors[section];
 
     for (const name of Object.keys(dependencies)) {
       if (name in desiredDependencies) {
@@ -236,18 +237,40 @@ export function inspect(
 
         const capability = unmanagedCapabilities[name];
         if (capability) {
-          capabilities[name] = capability;
+          extraCapabilities[name] = capability;
         }
       }
     }
 
     return count + changes.length;
   }, 0);
 
+  // If `--no-unmanaged`, treat unmanaged dependencies as errors if the package
+  // also declares `rnx-kit.alignDeps.capabilities`.
+  const warnings: Changes["capabilities"] = [];
+  const config = manifest["rnx-kit"]?.["alignDeps"] as KitConfig["alignDeps"];
+  if (Array.isArray(config?.capabilities)) {
+    const entries = noUnmanaged ? errors.capabilities : warnings;
+    entries.push(
+      ...Object.entries(extraCapabilities).map(([dependency, capability]) => ({
+        type: "unmanaged" as const,
+        dependency,
+        capability,
+      }))
+    );
+
+    if (write) {
+      const capabilities = config.capabilities;
+      for (const { capability } of errors.capabilities) {
+        capabilities.push(capability as Capability);
+      }
+    }
+  }
+
   return {
-    changes: allChanges,
-    changesCount,
-    unmanagedDependencies: Object.entries(capabilities),
+    errors,
+    errorCount: errorCount + errors.capabilities.length,
+    warnings,
   };
 }
 
@@ -266,55 +289,41 @@ export function inspect(
  */
 export function checkPackageManifestUnconfigured(
   manifestPath: string,
-  { excludePackages, noUnmanaged, write }: Options,
+  options: Options,
   config: AlignDepsConfig,
   logError = error
 ): ErrorCode {
+  const { excludePackages, write } = options;
   if (excludePackages?.includes(config.manifest.name)) {
     return "success";
   }
 
   const manifestProfile = buildManifestProfileCached(manifestPath, config);
   const { manifest } = config;
-  const { changes, changesCount, unmanagedDependencies } = inspect(
+  const { errors, errorCount, warnings } = inspect(
     manifest,
     manifestProfile,
-    write
+    options
   );
-  const hasUnmanagedDeps =
-    config.alignDeps.capabilities.length > 0 &&
-    unmanagedDependencies.length > 0;
-
-  if (hasUnmanagedDeps) {
-    const log = noUnmanaged ? logError : warn;
-    const dependencies = unmanagedDependencies
-      .map(([name, capability]) => {
-        return `\t  - ${name} can be managed by '${capability}'`;
-      })
-      .join("\n");
-    log(
-      `${manifestPath}: Found dependencies that are currently missing from capabilities:\n${dependencies}`
-    );
-    info(
-      "Note: Capabilities will never be added automatically, even with '--write'."
-    );
+
+  if (warnings.length > 0) {
+    const violations = stringify({ capabilities: warnings }, [
+      `${manifestPath}: Found dependencies that are currently missing from capabilities:`,
+    ]);
+    warn(violations);
   }
 
-  if (changesCount > 0) {
+  if (errorCount > 0) {
     if (write) {
       modifyManifest(manifestPath, manifest);
     } else {
-      const violations = stringify(changes, [
-        `${manifestPath}: Found ${changesCount} violation(s) outside of capabilities.`,
+      const violations = stringify(errors, [
+        `${manifestPath}: Found ${errorCount} violation(s) outside of capabilities.`,
       ]);
       logError(violations);
       return "unsatisfied";
     }
   }
 
-  if (noUnmanaged && hasUnmanagedDeps) {
-    return "unmanaged-capabilities";
-  }
-
   return "success";
 }
diff --git a/packages/align-deps/src/diff.ts b/packages/align-deps/src/diff.ts
@@ -1,5 +1,5 @@
-import { keysOf } from "@rnx-kit/tools-language";
 import type { PackageManifest } from "@rnx-kit/tools-node/package";
+import { dependencySections } from "./helpers";
 import type { Changes } from "./types";
 
 export function diff(
@@ -10,9 +10,10 @@ export function diff(
     dependencies: [],
     peerDependencies: [],
     devDependencies: [],
+    capabilities: [],
   };
 
-  const numChanges = keysOf(allChanges).reduce((count, section) => {
+  const numChanges = dependencySections.reduce((count, section) => {
     const changes = allChanges[section];
     const currentDeps = manifest[section] ?? {};
     const updatedDeps = updatedManifest[section] ?? {};
@@ -38,7 +39,10 @@ export function diff(
   return numChanges > 0 ? allChanges : undefined;
 }
 
-export function stringify(allChanges: Changes, output: string[] = []): string {
+export function stringify(
+  allChanges: Partial<Changes>,
+  output: string[] = []
+): string {
   const prefix = "\t  - ";
 
   for (const [section, changes] of Object.entries(allChanges)) {
@@ -58,6 +62,11 @@ export function stringify(allChanges: Changes, output: string[] = []): string {
           case "removed":
             output.push(`${prefix}${dependency} should be removed`);
             break;
+          case "unmanaged":
+            output.push(
+              `${prefix}${dependency} can be managed by '${change.capability}'`
+            );
+            break;
         }
       }
     }

diff --git a/packages/align-deps/src/helpers.ts b/packages/align-deps/src/helpers.ts
@@ -4,6 +4,12 @@ import detectIndent from "detect-indent";
 import fs from "fs";
 import semverValidRange from "semver/ranges/valid";
 
+export const dependencySections = [
+  "dependencies",
+  "peerDependencies",
+  "devDependencies",
+] as const;
+
 export function compare<T>(lhs: T, rhs: T): -1 | 0 | 1 {
   if (lhs === rhs) {
     return 0;
@@ -59,8 +65,7 @@ export function modifyManifest(
 }
 
 export function omitEmptySections(manifest: PackageManifest): PackageManifest {
-  const sections = ["dependencies", "peerDependencies", "devDependencies"];
-  for (const sectionName of sections) {
+  for (const sectionName of dependencySections) {
     const section = manifest[sectionName];
     if (typeof section === "object" && Object.keys(section).length === 0) {
       delete manifest[sectionName];

diff --git a/packages/align-deps/src/types.ts b/packages/align-deps/src/types.ts
@@ -16,6 +16,7 @@ export type Changes = {
   dependencies: Change[];
   peerDependencies: Change[];
   devDependencies: Change[];
+  capabilities: { type: "unmanaged"; dependency: string; capability: string }[];
 };
 
 export type Options = {
@@ -50,7 +51,6 @@ export type ErrorCode =
   | "invalid-manifest"
   | "missing-react-native"
   | "not-configured"
-  | "unmanaged-capabilities"
   | "unsatisfied";
 
 export type Command = (manifest: string) => ErrorCode;