Skip to content

Commit cc436b1

Browse files
committed
Fix dependency-related constraints
This commit adjusts the constraints in `yarn.config.cjs`: - Remove `expectControllerDependenciesListedAsPeerDependencies`. As its name implies, this constraint ensured that dependencies which referred to controllers were always listed as peer dependencies. This is incorrect, because in some cases we may simply want to list them as "production" dependencies. - Correct `expectDependenciesNotInBothProdAndDev` (now `expectDependenciesNotInBothProdAndDevOrPeer`) to not only ensure that a dependency is not listed in both `dependencies` and `devDependencies` but also both `dependencies` and `peerDependencies` (only `devDependencies` + `peerDependencies` is allowed, as combinations go). - Add `expectPeerDependenciesAlsoListedAsDevDependencies` constraint to ensure that peer dependencies are also listed as dev dependencies. - Fix `expectUpToDateWorkspaceDependenciesAndDevDependencies` to skip validation of "production" dependencies that are also listed in peer dependencies (this avoids conflicting constraints). - Correct `expectConsistentDependenciesAndDevDependencies` to skip workspace dependencies, as that is already being handled by `expectUpToDateWorkspaceDependenciesAndDevDependencies` (this avoids conflicting constraints). Note that because of these changes, several controllers have also been corrected to fit.
1 parent b7c4f83 commit cc436b1

File tree

11 files changed

+109
-80
lines changed

11 files changed

+109
-80
lines changed

packages/accounts-controller/CHANGELOG.md

+4
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
### Fixed
11+
12+
- `@metamask/network-controller` peer dependency is no longer also a direct dependency
13+
1014
## [26.0.0]
1115

1216
### Changed

packages/accounts-controller/package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,6 @@
5353
"@metamask/keyring-api": "^17.2.0",
5454
"@metamask/keyring-internal-api": "^6.0.0",
5555
"@metamask/keyring-utils": "^3.0.0",
56-
"@metamask/network-controller": "^22.2.1",
5756
"@metamask/snaps-sdk": "^6.17.1",
5857
"@metamask/snaps-utils": "^8.10.0",
5958
"@metamask/utils": "^11.2.0",
@@ -65,6 +64,7 @@
6564
"devDependencies": {
6665
"@metamask/auto-changelog": "^3.4.4",
6766
"@metamask/keyring-controller": "^21.0.0",
67+
"@metamask/network-controller": "^22.2.1",
6868
"@metamask/providers": "^18.1.1",
6969
"@metamask/snaps-controllers": "^9.19.0",
7070
"@types/jest": "^27.4.1",

packages/bridge-status-controller/CHANGELOG.md

+4
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
### Fixed
11+
12+
- `@metamask/bridge-controller` dependency is no longer a peer dependency, just a direct dependency
13+
1014
## [5.0.0]
1115

1216
### Changed

packages/bridge-status-controller/package.json

-1
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,6 @@
7272
},
7373
"peerDependencies": {
7474
"@metamask/accounts-controller": "^26.0.0",
75-
"@metamask/bridge-controller": "^5.0.0",
7675
"@metamask/network-controller": "^22.0.0",
7776
"@metamask/transaction-controller": "^48.0.0"
7877
},

packages/multichain-network-controller/package.json

+1
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@
5353
"@solana/addresses": "^2.0.0"
5454
},
5555
"devDependencies": {
56+
"@metamask/accounts-controller": "^26.0.0",
5657
"@metamask/auto-changelog": "^3.4.4",
5758
"@metamask/keyring-controller": "^21.0.0",
5859
"@metamask/network-controller": "^22.2.1",

packages/multichain-transactions-controller/CHANGELOG.md

+4
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
### Fixed
11+
12+
- `@metamask/snaps-controllers` peer dependency is no longer also a direct dependency
13+
1014
## [0.7.0]
1115

1216
### Changed

packages/multichain-transactions-controller/package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,6 @@
5252
"@metamask/keyring-internal-api": "^6.0.0",
5353
"@metamask/keyring-snap-client": "^4.0.1",
5454
"@metamask/polling-controller": "^12.0.3",
55-
"@metamask/snaps-controllers": "^9.19.0",
5655
"@metamask/snaps-sdk": "^6.17.1",
5756
"@metamask/snaps-utils": "^8.10.0",
5857
"@metamask/utils": "^11.2.0",
@@ -64,6 +63,7 @@
6463
"@metamask/accounts-controller": "^26.0.0",
6564
"@metamask/auto-changelog": "^3.4.4",
6665
"@metamask/keyring-controller": "^21.0.0",
66+
"@metamask/snaps-controllers": "^9.19.0",
6767
"@types/jest": "^27.4.1",
6868
"deepmerge": "^4.2.2",
6969
"jest": "^27.5.1",

packages/profile-sync-controller/CHANGELOG.md

+4
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
### Fixed
11+
12+
- Peer dependencies `@metamask/keyring-controller` and `@metamask/network-controller` are no longer also direct dependencies
13+
1014
## [10.0.0]
1115

1216
### Changed

packages/profile-sync-controller/package.json

+2-2
Original file line numberDiff line numberDiff line change
@@ -102,8 +102,6 @@
102102
"dependencies": {
103103
"@metamask/base-controller": "^8.0.0",
104104
"@metamask/keyring-api": "^17.2.0",
105-
"@metamask/keyring-controller": "^21.0.0",
106-
"@metamask/network-controller": "^22.2.1",
107105
"@metamask/snaps-sdk": "^6.17.1",
108106
"@metamask/snaps-utils": "^8.10.0",
109107
"@noble/ciphers": "^0.5.2",
@@ -117,7 +115,9 @@
117115
"@lavamoat/preinstall-always-fail": "^2.1.0",
118116
"@metamask/accounts-controller": "^26.0.0",
119117
"@metamask/auto-changelog": "^3.4.4",
118+
"@metamask/keyring-controller": "^21.0.0",
120119
"@metamask/keyring-internal-api": "^6.0.0",
120+
"@metamask/network-controller": "^22.2.1",
121121
"@metamask/providers": "^18.1.1",
122122
"@metamask/snaps-controllers": "^9.19.0",
123123
"@types/jest": "^27.4.1",

yarn.config.cjs

+87-74
Original file line numberDiff line numberDiff line change
@@ -193,25 +193,27 @@ module.exports = defineConfig({
193193
// If one workspace package lists another workspace package within
194194
// `dependencies` or `devDependencies`, the version used within the
195195
// dependency range must match the current version of the dependency.
196-
expectUpToDateWorkspaceDependenciesAndDevDependencies(Yarn, workspace);
196+
expectUpToDateWorkspaceDependenciesAndDevDependencies(
197+
Yarn,
198+
dependenciesByIdentAndType,
199+
);
197200

198201
// If one workspace package lists another workspace package within
199202
// `peerDependencies`, the dependency range must satisfy the current
200203
// version of that package.
201204
expectUpToDateWorkspacePeerDependencies(Yarn, workspace);
202205

203206
// No dependency may be listed under both `dependencies` and
204-
// `devDependencies`.
205-
expectDependenciesNotInBothProdAndDev(
207+
// `devDependencies`, or under both `dependencies` and `peerDependencies`.
208+
expectDependenciesNotInBothProdAndDevOrPeer(
206209
workspace,
207210
dependenciesByIdentAndType,
208211
);
209212

210-
// If one workspace package (A) lists another workspace package (B) in its
211-
// `dependencies`, and B is a controller package, then we need to ensure
212-
// that B is also listed in A's `peerDependencies` and that the version
213-
// range satisfies the current version of B.
214-
expectControllerDependenciesListedAsPeerDependencies(
213+
// If one package A lists another package B in its `peerDependencies`,
214+
// then B must also be listed in A's `devDependencies`, and if B is a
215+
// workspace package, the dev dependency must match B's version.
216+
expectPeerDependenciesAlsoListedAsDevDependencies(
215217
Yarn,
216218
workspace,
217219
dependenciesByIdentAndType,
@@ -250,15 +252,14 @@ module.exports = defineConfig({
250252
}
251253

252254
// All version ranges in `dependencies` and `devDependencies` for the same
253-
// dependency across the monorepo must be the same.
255+
// non-workspace dependency across the monorepo must be the same.
254256
expectConsistentDependenciesAndDevDependencies(Yarn);
255257
},
256258
});
257259

258260
/**
259-
* Construct a nested map of dependencies. The inner layer categorizes
260-
* instances of the same dependency by its location in the manifest; the outer
261-
* layer categorizes the inner layer by the name of the dependency.
261+
* Organizes the given dependencies by name and type (`dependencies`,
262+
* `devDependencies`, or `peerDependencies`).
262263
*
263264
* @param {Dependency[]} dependencies - The list of dependencies to transform.
264265
* @returns {Map<string, Map<DependencyType, Dependency>>} The resulting map.
@@ -381,12 +382,15 @@ async function workspaceFileExists(workspace, path) {
381382
}
382383

383384
/**
384-
* Expect that the workspace has the given field, and that it is a non-null
385-
* value. If the field is not present, or is null, this will log an error, and
386-
* cause the constraint to fail.
385+
* This function does one of three things depending on the arguments given:
387386
*
388-
* If a value is provided, this will also verify that the field is equal to the
389-
* given value.
387+
* - With no value provided, this will expect that the workspace has the given
388+
* field and that it is a non-null value; if the field is not present or is
389+
* null, this will log an error and cause the constraint to fail.
390+
* - With a value is provided, and the value is non-null, this will verify that
391+
* the field is equal to the given value.
392+
* - With a value is provided, and the value is null, this will verify that the
393+
* field is not present.
390394
*
391395
* @param {Workspace} workspace - The workspace to check.
392396
* @param {string} fieldName - The field to check.
@@ -592,25 +596,37 @@ function expectCorrectWorkspaceChangelogScripts(workspace) {
592596

593597
/**
594598
* Expect that if the workspace package lists another workspace package within
595-
* `dependencies` or `devDependencies`, the version used within the dependency
596-
* range is exactly equal to the current version of the dependency (and the
597-
* range uses the `^` modifier).
599+
* `devDependencies`, or lists another workspace package within `dependencies`
600+
* (and does not already list it in `peerDependencies`), the version used within
601+
* the dependency range is exactly equal to the current version of the
602+
* dependency (and the range uses the `^` modifier).
598603
*
599604
* @param {Yarn} Yarn - The Yarn "global".
600-
* @param {Workspace} workspace - The workspace to check.
605+
* @param {Map<string, Map<DependencyType, Dependency>>} dependenciesByIdentAndType -
606+
* Map of dependency ident to dependency type and dependency.
601607
*/
602608
function expectUpToDateWorkspaceDependenciesAndDevDependencies(
603609
Yarn,
604-
workspace,
610+
dependenciesByIdentAndType,
605611
) {
606-
for (const dependency of Yarn.dependencies({ workspace })) {
607-
const dependencyWorkspace = Yarn.workspace({ ident: dependency.ident });
612+
for (const [
613+
dependencyIdent,
614+
dependencyInstancesByType,
615+
] of dependenciesByIdentAndType.entries()) {
616+
const dependencyWorkspace = Yarn.workspace({ ident: dependencyIdent });
608617

609-
if (
610-
dependencyWorkspace !== null &&
611-
dependency.type !== 'peerDependencies'
612-
) {
613-
const ignoredRanges = ALLOWED_INCONSISTENT_DEPENDENCIES[dependency.ident];
618+
if (!dependencyWorkspace) {
619+
continue;
620+
}
621+
622+
const devDependency = dependencyInstancesByType.get('devDependencies');
623+
const prodDependency = dependencyInstancesByType.get('dependencies');
624+
const peerDependency = dependencyInstancesByType.get('peerDependencies');
625+
626+
if (devDependency || (prodDependency && !peerDependency)) {
627+
const dependency = devDependency ?? prodDependency;
628+
629+
const ignoredRanges = ALLOWED_INCONSISTENT_DEPENDENCIES[dependencyIdent];
614630
if (ignoredRanges?.includes(dependency.range)) {
615631
continue;
616632
}
@@ -645,63 +661,64 @@ function expectUpToDateWorkspacePeerDependencies(Yarn, workspace) {
645661
dependency.range,
646662
)
647663
) {
648-
expectWorkspaceField(
649-
workspace,
650-
`peerDependencies["${dependency.ident}"]`,
651-
`^${dependencyWorkspaceVersion.major}.0.0`,
652-
);
664+
dependency.update(`^${dependencyWorkspaceVersion.major}.0.0`);
653665
}
654666
}
655667
}
656668
}
657669

658670
/**
659671
* Expect that a workspace package does not list a dependency in both
660-
* `dependencies` and `devDependencies`.
672+
* `dependencies` and `devDependencies`, or in both `dependencies` and
673+
* `peerDependencies`.
661674
*
662675
* @param {Workspace} workspace - The workspace to check.
663-
* @param {Map<string, Map<DependencyType, Dependency>>} dependenciesByIdentAndType - Map of
664-
* dependency ident to dependency type and dependency.
676+
* @param {Map<string, Map<DependencyType, Dependency>>} dependenciesByIdentAndType -
677+
* Map of dependency ident to dependency type and dependency.
665678
*/
666-
function expectDependenciesNotInBothProdAndDev(
679+
function expectDependenciesNotInBothProdAndDevOrPeer(
667680
workspace,
668681
dependenciesByIdentAndType,
669682
) {
670683
for (const [
671684
dependencyIdent,
672685
dependencyInstancesByType,
673686
] of dependenciesByIdentAndType.entries()) {
674-
if (
675-
dependencyInstancesByType.size > 1 &&
676-
!dependencyInstancesByType.has('peerDependencies')
677-
) {
687+
const dependency = dependencyInstancesByType.get('dependencies');
688+
if (dependency === undefined) {
689+
continue;
690+
}
691+
if (dependencyInstancesByType.has('devDependencies')) {
678692
workspace.error(
679693
`\`${dependencyIdent}\` cannot be listed in both \`dependencies\` and \`devDependencies\``,
680694
);
695+
} else if (dependencyInstancesByType.has('peerDependencies')) {
696+
expectWorkspaceField(
697+
workspace,
698+
`devDependencies["${dependencyIdent}"]`,
699+
dependency.range,
700+
);
701+
expectWorkspaceField(
702+
workspace,
703+
`dependencies["${dependencyIdent}"]`,
704+
null,
705+
);
681706
}
682707
}
683708
}
684709

685710
/**
686-
* Expect that if the workspace package lists another workspace package in its
687-
* dependencies, and it is a controller package, that the controller package is
688-
* listed in the workspace's `peerDependencies` and the version range satisfies
689-
* the current version of the controller package.
690-
*
691-
* The expectation in this case is that the client will instantiate B in order
692-
* to pass it into A. Therefore, it needs to list not only A as a dependency,
693-
* but also B. Additionally, the version of B that the client is using with A
694-
* needs to match the version that A itself is expecting internally.
695-
*
696-
* Note that this constraint does not apply for packages that seem to represent
697-
* instantiable controllers but actually represent abstract classes.
711+
* Expect that if the workspace package lists another package in its
712+
* `peerDependencies`, the package is also listed in the workspace's
713+
* `devDependencies`. If the other package is a workspace package, also expect
714+
* that the dev dependency matches the current version of the package.
698715
*
699716
* @param {Yarn} Yarn - The Yarn "global".
700717
* @param {Workspace} workspace - The workspace to check.
701718
* @param {Map<string, Map<DependencyType, Dependency>>} dependenciesByIdentAndType - Map of
702719
* dependency ident to dependency type and dependency.
703720
*/
704-
function expectControllerDependenciesListedAsPeerDependencies(
721+
function expectPeerDependenciesAlsoListedAsDevDependencies(
705722
Yarn,
706723
workspace,
707724
dependenciesByIdentAndType,
@@ -710,27 +727,20 @@ function expectControllerDependenciesListedAsPeerDependencies(
710727
dependencyIdent,
711728
dependencyInstancesByType,
712729
] of dependenciesByIdentAndType.entries()) {
713-
if (!dependencyInstancesByType.has('dependencies')) {
730+
if (!dependencyInstancesByType.has('peerDependencies')) {
714731
continue;
715732
}
716733

717734
const dependencyWorkspace = Yarn.workspace({ ident: dependencyIdent });
718735

719-
if (
720-
dependencyWorkspace !== null &&
721-
dependencyIdent.endsWith('-controller') &&
722-
dependencyIdent !== '@metamask/base-controller' &&
723-
dependencyIdent !== '@metamask/polling-controller' &&
724-
!dependencyInstancesByType.has('peerDependencies')
725-
) {
726-
const dependencyWorkspaceVersion = new semver.SemVer(
727-
dependencyWorkspace.manifest.version,
728-
);
736+
if (dependencyWorkspace) {
729737
expectWorkspaceField(
730738
workspace,
731-
`peerDependencies["${dependencyIdent}"]`,
732-
`^${dependencyWorkspaceVersion.major}.0.0`,
739+
`devDependencies["${dependencyIdent}"]`,
740+
`^${dependencyWorkspace.manifest.version}`,
733741
);
742+
} else {
743+
expectWorkspaceField(workspace, `devDependencies["${dependencyIdent}"]`);
734744
}
735745
}
736746
}
@@ -758,11 +768,10 @@ function getInconsistentDependenciesAndDevDependencies(
758768
}
759769

760770
/**
761-
* Expect that all version ranges in `dependencies` and `devDependencies` for
762-
* the same dependency across the entire monorepo are the same. As it is
763-
* impossible to compare NPM version ranges, let the user decide if there are
764-
* conflicts. (`peerDependencies` is a special case, and we handle that
765-
* particularly for workspace packages elsewhere.)
771+
* Expect that across the entire monorepo all version ranges in `dependencies`
772+
* and `devDependencies` for the same dependency are the same (as long as it is
773+
* not a dependency on a workspace package). As it is impossible to compare NPM
774+
* version ranges, let the user decide if there are conflicts.
766775
*
767776
* @param {Yarn} Yarn - The Yarn "global".
768777
*/
@@ -775,15 +784,19 @@ function expectConsistentDependenciesAndDevDependencies(Yarn) {
775784
dependencyIdent,
776785
dependenciesByRange,
777786
] of nonPeerDependenciesByIdent.entries()) {
778-
if (dependenciesByRange.size <= 1) {
787+
const dependencyWorkspace = Yarn.workspace({ ident: dependencyIdent });
788+
789+
if (dependenciesByRange.size <= 1 || dependencyWorkspace) {
779790
continue;
780791
}
792+
781793
const dependenciesToConsider =
782794
getInconsistentDependenciesAndDevDependencies(
783795
dependencyIdent,
784796
dependenciesByRange,
785797
);
786798
const dependencyRanges = [...dependenciesToConsider.keys()].sort();
799+
787800
for (const dependencies of dependenciesToConsider.values()) {
788801
for (const dependency of dependencies) {
789802
dependency.error(

0 commit comments

Comments
 (0)