Fix dependency-related constraints

packages/accounts-controller/CHANGELOG.md

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Fixed
+
+- `@metamask/network-controller` peer dependency is no longer also a direct dependency ([#5464](https://github.com/MetaMask/core/pull/5464)))
+
 ## [26.1.0]
 
 ### Changed

packages/accounts-controller/package.json

@@ -53,7 +53,6 @@
     "@metamask/keyring-api": "^17.2.0",
     "@metamask/keyring-internal-api": "^6.0.0",
     "@metamask/keyring-utils": "^3.0.0",
-    "@metamask/network-controller": "^22.2.1",
     "@metamask/snaps-sdk": "^6.17.1",
     "@metamask/snaps-utils": "^8.10.0",
     "@metamask/utils": "^11.2.0",
@@ -65,6 +64,7 @@
   "devDependencies": {
     "@metamask/auto-changelog": "^3.4.4",
     "@metamask/keyring-controller": "^21.0.0",
+    "@metamask/network-controller": "^22.2.1",
     "@metamask/providers": "^18.1.1",
     "@metamask/snaps-controllers": "^9.19.0",
     "@types/jest": "^27.4.1",

packages/bridge-status-controller/CHANGELOG.md

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Fixed
+
+- `@metamask/bridge-controller` dependency is no longer a peer dependency, just a direct dependency ([#5464](https://github.com/MetaMask/core/pull/5464)))
+
 ## [6.0.0]
 
 ### Changed

packages/bridge-status-controller/package.json

@@ -72,7 +72,6 @@
   },
   "peerDependencies": {
     "@metamask/accounts-controller": "^26.0.0",
-    "@metamask/bridge-controller": "^6.0.0",
     "@metamask/network-controller": "^22.0.0",
     "@metamask/transaction-controller": "^49.0.0"
   },

packages/multichain-network-controller/package.json

@@ -53,6 +53,7 @@
     "@solana/addresses": "^2.0.0"
   },
   "devDependencies": {
+    "@metamask/accounts-controller": "^26.1.0",
     "@metamask/auto-changelog": "^3.4.4",
     "@metamask/keyring-controller": "^21.0.0",
     "@metamask/network-controller": "^22.2.1",

packages/multichain-transactions-controller/CHANGELOG.md

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Fixed
+
+- `@metamask/snaps-controllers` peer dependency is no longer also a direct dependency ([#5464](https://github.com/MetaMask/core/pull/5464)))
+
 ## [0.7.1]
 
 ### Fixed

packages/multichain-transactions-controller/package.json

@@ -52,7 +52,6 @@
     "@metamask/keyring-internal-api": "^6.0.0",
     "@metamask/keyring-snap-client": "^4.0.1",
     "@metamask/polling-controller": "^12.0.3",
-    "@metamask/snaps-controllers": "^9.19.0",
     "@metamask/snaps-sdk": "^6.17.1",
     "@metamask/snaps-utils": "^8.10.0",
     "@metamask/utils": "^11.2.0",
@@ -64,6 +63,7 @@
     "@metamask/accounts-controller": "^26.1.0",
     "@metamask/auto-changelog": "^3.4.4",
     "@metamask/keyring-controller": "^21.0.0",
+    "@metamask/snaps-controllers": "^9.19.0",
     "@types/jest": "^27.4.1",
     "deepmerge": "^4.2.2",
     "jest": "^27.5.1",

packages/profile-sync-controller/CHANGELOG.md

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Fixed
+
+- Peer dependencies `@metamask/keyring-controller` and `@metamask/network-controller` are no longer also direct dependencies ([#5464](https://github.com/MetaMask/core/pull/5464)))
+
 ## [10.1.0]
 
 ### Added

packages/profile-sync-controller/package.json

@@ -102,8 +102,6 @@
   "dependencies": {
     "@metamask/base-controller": "^8.0.0",
     "@metamask/keyring-api": "^17.2.0",
-    "@metamask/keyring-controller": "^21.0.0",
-    "@metamask/network-controller": "^22.2.1",
     "@metamask/snaps-sdk": "^6.17.1",
     "@metamask/snaps-utils": "^8.10.0",
     "@noble/ciphers": "^0.5.2",
@@ -117,7 +115,9 @@
     "@lavamoat/preinstall-always-fail": "^2.1.0",
     "@metamask/accounts-controller": "^26.1.0",
     "@metamask/auto-changelog": "^3.4.4",
+    "@metamask/keyring-controller": "^21.0.0",
     "@metamask/keyring-internal-api": "^6.0.0",
+    "@metamask/network-controller": "^22.2.1",
     "@metamask/providers": "^18.1.1",
     "@metamask/snaps-controllers": "^9.19.0",
     "@types/jest": "^27.4.1",

yarn.config.cjs

@@ -193,25 +193,27 @@ module.exports = defineConfig({
       // If one workspace package lists another workspace package within
       // `dependencies` or `devDependencies`, the version used within the
       // dependency range must match the current version of the dependency.
-      expectUpToDateWorkspaceDependenciesAndDevDependencies(Yarn, workspace);
+      expectUpToDateWorkspaceDependenciesAndDevDependencies(
+        Yarn,
+        dependenciesByIdentAndType,
+      );
 
       // If one workspace package lists another workspace package within
       // `peerDependencies`, the dependency range must satisfy the current
       // version of that package.
       expectUpToDateWorkspacePeerDependencies(Yarn, workspace);
 
       // No dependency may be listed under both `dependencies` and
-      // `devDependencies`.
-      expectDependenciesNotInBothProdAndDev(
+      // `devDependencies`, or under both `dependencies` and `peerDependencies`.
+      expectDependenciesNotInBothProdAndDevOrPeer(
         workspace,
         dependenciesByIdentAndType,
       );
 
-      // If one workspace package (A) lists another workspace package (B) in its
-      // `dependencies`, and B is a controller package, then we need to ensure
-      // that B is also listed in A's `peerDependencies` and that the version
-      // range satisfies the current version of B.
-      expectControllerDependenciesListedAsPeerDependencies(
+      // If one package A lists another package B in its `peerDependencies`,
+      // then B must also be listed in A's `devDependencies`, and if B is a
+      // workspace package, the dev dependency must match B's version.
+      expectPeerDependenciesAlsoListedAsDevDependencies(
         Yarn,
         workspace,
         dependenciesByIdentAndType,
@@ -250,15 +252,14 @@ module.exports = defineConfig({
     }
 
     // All version ranges in `dependencies` and `devDependencies` for the same
-    // dependency across the monorepo must be the same.
+    // non-workspace dependency across the monorepo must be the same.
     expectConsistentDependenciesAndDevDependencies(Yarn);
   },
 });
 
 /**
- * Construct a nested map of dependencies. The inner layer categorizes
- * instances of the same dependency by its location in the manifest; the outer
- * layer categorizes the inner layer by the name of the dependency.
+ * Organizes the given dependencies by name and type (`dependencies`,
+ * `devDependencies`, or `peerDependencies`).
  *
  * @param {Dependency[]} dependencies - The list of dependencies to transform.
  * @returns {Map<string, Map<DependencyType, Dependency>>} The resulting map.
@@ -381,12 +382,15 @@ async function workspaceFileExists(workspace, path) {
 }
 
 /**
- * Expect that the workspace has the given field, and that it is a non-null
- * value. If the field is not present, or is null, this will log an error, and
- * cause the constraint to fail.
+ * This function does one of three things depending on the arguments given:
  *
- * If a value is provided, this will also verify that the field is equal to the
- * given value.
+ * - With no value provided, this will expect that the workspace has the given
+ * field and that it is a non-null value; if the field is not present or is
+ * null, this will log an error and cause the constraint to fail.
+ * - With a value is provided, and the value is non-null, this will verify that
+ * the field is equal to the given value.
+ * - With a value is provided, and the value is null, this will verify that the
+ * field is not present.
  *
  * @param {Workspace} workspace - The workspace to check.
  * @param {string} fieldName - The field to check.
@@ -592,25 +596,37 @@ function expectCorrectWorkspaceChangelogScripts(workspace) {
 
 /**
  * Expect that if the workspace package lists another workspace package within
- * `dependencies` or `devDependencies`, the version used within the dependency
- * range is exactly equal to the current version of the dependency (and the
- * range uses the `^` modifier).
+ * `devDependencies`, or lists another workspace package within `dependencies`
+ * (and does not already list it in `peerDependencies`), the version used within
+ * the dependency range is exactly equal to the current version of the
+ * dependency (and the range uses the `^` modifier).
  *
  * @param {Yarn} Yarn - The Yarn "global".
- * @param {Workspace} workspace - The workspace to check.
+ * @param {Map<string, Map<DependencyType, Dependency>>} dependenciesByIdentAndType -
+ * Map of dependency ident to dependency type and dependency.
  */
 function expectUpToDateWorkspaceDependenciesAndDevDependencies(
   Yarn,
-  workspace,
+  dependenciesByIdentAndType,
 ) {
-  for (const dependency of Yarn.dependencies({ workspace })) {
-    const dependencyWorkspace = Yarn.workspace({ ident: dependency.ident });
+  for (const [
+    dependencyIdent,
+    dependencyInstancesByType,
+  ] of dependenciesByIdentAndType.entries()) {
+    const dependencyWorkspace = Yarn.workspace({ ident: dependencyIdent });
 
-    if (
-      dependencyWorkspace !== null &&
-      dependency.type !== 'peerDependencies'
-    ) {
-      const ignoredRanges = ALLOWED_INCONSISTENT_DEPENDENCIES[dependency.ident];
+    if (!dependencyWorkspace) {
+      continue;
+    }
+
+    const devDependency = dependencyInstancesByType.get('devDependencies');
+    const prodDependency = dependencyInstancesByType.get('dependencies');
+    const peerDependency = dependencyInstancesByType.get('peerDependencies');
+
+    if (devDependency || (prodDependency && !peerDependency)) {
+      const dependency = devDependency ?? prodDependency;
+
+      const ignoredRanges = ALLOWED_INCONSISTENT_DEPENDENCIES[dependencyIdent];
       if (ignoredRanges?.includes(dependency.range)) {
         continue;
       }
@@ -645,63 +661,64 @@ function expectUpToDateWorkspacePeerDependencies(Yarn, workspace) {
           dependency.range,
         )
       ) {
-        expectWorkspaceField(
-          workspace,
-          `peerDependencies["${dependency.ident}"]`,
-          `^${dependencyWorkspaceVersion.major}.0.0`,
-        );
+        dependency.update(`^${dependencyWorkspaceVersion.major}.0.0`);
       }
     }
   }
 }
 
 /**
  * Expect that a workspace package does not list a dependency in both
- * `dependencies` and `devDependencies`.
+ * `dependencies` and `devDependencies`, or in both `dependencies` and
+ * `peerDependencies`.
  *
  * @param {Workspace} workspace - The workspace to check.
- * @param {Map<string, Map<DependencyType, Dependency>>} dependenciesByIdentAndType - Map of
- * dependency ident to dependency type and dependency.
+ * @param {Map<string, Map<DependencyType, Dependency>>} dependenciesByIdentAndType -
+ * Map of dependency ident to dependency type and dependency.
  */
-function expectDependenciesNotInBothProdAndDev(
+function expectDependenciesNotInBothProdAndDevOrPeer(
   workspace,
   dependenciesByIdentAndType,
 ) {
   for (const [
     dependencyIdent,
     dependencyInstancesByType,
   ] of dependenciesByIdentAndType.entries()) {
-    if (
-      dependencyInstancesByType.size > 1 &&
-      !dependencyInstancesByType.has('peerDependencies')
-    ) {
+    const dependency = dependencyInstancesByType.get('dependencies');
+    if (dependency === undefined) {
+      continue;
+    }
+    if (dependencyInstancesByType.has('devDependencies')) {
       workspace.error(
         `\`${dependencyIdent}\` cannot be listed in both \`dependencies\` and \`devDependencies\``,
       );
+    } else if (dependencyInstancesByType.has('peerDependencies')) {
+      expectWorkspaceField(
+        workspace,
+        `devDependencies["${dependencyIdent}"]`,
+        dependency.range,
+      );
+      expectWorkspaceField(
+        workspace,
+        `dependencies["${dependencyIdent}"]`,
+        null,
+      );
     }
   }
 }
 
 /**
- * Expect that if the workspace package lists another workspace package in its
- * dependencies, and it is a controller package, that the controller package is
- * listed in the workspace's `peerDependencies` and the version range satisfies
- * the current version of the controller package.
- *
- * The expectation in this case is that the client will instantiate B in order
- * to pass it into A. Therefore, it needs to list not only A as a dependency,
- * but also B. Additionally, the version of B that the client is using with A
- * needs to match the version that A itself is expecting internally.
- *
- * Note that this constraint does not apply for packages that seem to represent
- * instantiable controllers but actually represent abstract classes.
+ * Expect that if the workspace package lists another package in its
+ * `peerDependencies`, the package is also listed in the workspace's
+ * `devDependencies`. If the other package is a workspace package, also expect
+ * that the dev dependency matches the current version of the package.
  *
  * @param {Yarn} Yarn - The Yarn "global".
  * @param {Workspace} workspace - The workspace to check.
  * @param {Map<string, Map<DependencyType, Dependency>>} dependenciesByIdentAndType - Map of
  * dependency ident to dependency type and dependency.
  */
-function expectControllerDependenciesListedAsPeerDependencies(
+function expectPeerDependenciesAlsoListedAsDevDependencies(
   Yarn,
   workspace,
   dependenciesByIdentAndType,
@@ -710,27 +727,20 @@ function expectControllerDependenciesListedAsPeerDependencies(
     dependencyIdent,
     dependencyInstancesByType,
   ] of dependenciesByIdentAndType.entries()) {
-    if (!dependencyInstancesByType.has('dependencies')) {
+    if (!dependencyInstancesByType.has('peerDependencies')) {
       continue;
     }
 
     const dependencyWorkspace = Yarn.workspace({ ident: dependencyIdent });
 
-    if (
-      dependencyWorkspace !== null &&
-      dependencyIdent.endsWith('-controller') &&
-      dependencyIdent !== '@metamask/base-controller' &&
-      dependencyIdent !== '@metamask/polling-controller' &&
-      !dependencyInstancesByType.has('peerDependencies')
-    ) {
-      const dependencyWorkspaceVersion = new semver.SemVer(
-        dependencyWorkspace.manifest.version,
-      );
+    if (dependencyWorkspace) {
       expectWorkspaceField(
         workspace,
-        `peerDependencies["${dependencyIdent}"]`,
-        `^${dependencyWorkspaceVersion.major}.0.0`,
+        `devDependencies["${dependencyIdent}"]`,
+        `^${dependencyWorkspace.manifest.version}`,
       );
+    } else {
+      expectWorkspaceField(workspace, `devDependencies["${dependencyIdent}"]`);
     }
   }
 }
@@ -758,11 +768,10 @@ function getInconsistentDependenciesAndDevDependencies(
 }
 
 /**
- * Expect that all version ranges in `dependencies` and `devDependencies` for
- * the same dependency across the entire monorepo are the same. As it is
- * impossible to compare NPM version ranges, let the user decide if there are
- * conflicts. (`peerDependencies` is a special case, and we handle that
- * particularly for workspace packages elsewhere.)
+ * Expect that across the entire monorepo all version ranges in `dependencies`
+ * and `devDependencies` for the same dependency are the same (as long as it is
+ * not a dependency on a workspace package). As it is impossible to compare NPM
+ * version ranges, let the user decide if there are conflicts.
  *
  * @param {Yarn} Yarn - The Yarn "global".
  */
@@ -775,15 +784,19 @@ function expectConsistentDependenciesAndDevDependencies(Yarn) {
     dependencyIdent,
     dependenciesByRange,
   ] of nonPeerDependenciesByIdent.entries()) {
-    if (dependenciesByRange.size <= 1) {
+    const dependencyWorkspace = Yarn.workspace({ ident: dependencyIdent });
+
+    if (dependenciesByRange.size <= 1 || dependencyWorkspace) {
       continue;
     }
+
     const dependenciesToConsider =
       getInconsistentDependenciesAndDevDependencies(
         dependencyIdent,
         dependenciesByRange,
       );
     const dependencyRanges = [...dependenciesToConsider.keys()].sort();
+
     for (const dependencies of dependenciesToConsider.values()) {
       for (const dependency of dependencies) {
         dependency.error(