Skip to content

Remove recent peer dependency bumps #4338

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
mcmire wants to merge 1 commit into from
Closed

Remove recent peer dependency bumps #4338

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/assets-controllers/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Fixed

- Relax peer dependency `@metamask/network-controller` from `^18.1.2` to `^18.0.0` ([#4338](https://github.com/MetaMask/core/pull/4338))

## [30.0.0]

### Added
Expand Down
2 changes: 1 addition & 1 deletion packages/assets-controllers/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@
"@metamask/accounts-controller": "^14.0.0",
"@metamask/approval-controller": "^6.0.0",
"@metamask/keyring-controller": "^16.0.0",
"@metamask/network-controller": "^18.1.2",
"@metamask/network-controller": "^18.0.0",
"@metamask/preferences-controller": "^11.0.0"
},
"engines": {
Expand Down
4 changes: 4 additions & 0 deletions packages/ens-controller/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Fixed

- Relax peer dependency `@metamask/network-controller` from `^18.1.2` to `^18.0.0` ([#4338](https://github.com/MetaMask/core/pull/4338))

## [10.0.1]

### Fixed
Expand Down
2 changes: 1 addition & 1 deletion packages/ens-controller/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@
"typescript": "~4.9.5"
},
"peerDependencies": {
"@metamask/network-controller": "^18.1.2"
"@metamask/network-controller": "^18.0.0"
},
"engines": {
"node": ">=16.0.0"
Expand Down
4 changes: 4 additions & 0 deletions packages/gas-fee-controller/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Fixed

- Relax peer dependency `@metamask/network-controller` from `^18.1.2` to `^18.0.0` ([#4338](https://github.com/MetaMask/core/pull/4338))

## [15.1.2]

### Fixed
Expand Down
2 changes: 1 addition & 1 deletion packages/gas-fee-controller/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@
"typescript": "~4.9.5"
},
"peerDependencies": {
"@metamask/network-controller": "^18.1.2"
"@metamask/network-controller": "^18.0.0"
Copy link
Contributor

@legobeat legobeat May 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this one should be aligned with the dependencies field? So if dropping here, then we'd also like to drop to dependencies["@metamask/network-controller"] = "^18.1.0" ?

Copy link
Contributor Author

@mcmire mcmire May 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, are you saying that if a version of a package within the monorepo is bumped we should no longer sync all dependencies on that package?

Copy link
Contributor

@legobeat legobeat May 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, are you saying that if a version of a package within the monorepo is bumped we should no longer sync all dependencies on that package?

Just considering this case: Using @metamask/gas-fee-controller, it will itself use @metamask/network-controller@^18.1.2 due to its dependency. It will accept any @metamask/network-controller@^18.0.0.
I have trouble coming up with why we want to enforce the direct dependency to be latest but still allow supplying a different, older versions as constructor parameters or what it may be. I think the developer assumption is generally that when using e.g. a controller instance as a construction parameter, it's easy to assume to be working with a single version of the package and not multiple at the same time depending on where instantiation was made.

For the general case for packages that have the same package in both dependencies and peerDependencies, I'm thinking it's a case-by-case-basis: Either both fields are bumped, or neither. I can see both situations and can't really give you a general heuristic.

Conversely, why do you think we want to have these disjoint?

Copy link
Contributor

@legobeat legobeat May 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Kind of separate, some of the messiness involved here goes away if properly utilizing worskpace cross-references, I think)

Copy link
Contributor Author

@mcmire mcmire May 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Originally, we linked dependencies and peerDependencies together. So if a developer updated the dependency of a package and it was also a peer dependency, then the versions needed to match. However, since bumping a peer dependency requires a major version bump, this resulted in many major version bumps. We decided to unlink them here to reduce the number of major versions: #3881.

I do see your point about allowing older versions of constructor parameters but not allowing older versions of "messages" between controllers. It seems highly likely for a change in API to affect direct usage (instantiating or calling methods on the controller) as much as indirect usage (using the controller through the messenger) and in the same way. Perhaps the change I mentioned above needs to be readdressed?

As for why we don't use workspace:^, this prevents developers from being able to test changes to core packages in extension and mobile locally (e.g. using yarn link). We replaced workspace:^ references with static versions and added a Yarn constraint which ensured that if one workspace package has a dependency on another, the version of the dependency matches the current version of the package: #1623. Admittedly, this goes further than workspace:^ (and acts more like workspace:*).

},
"engines": {
"node": ">=16.0.0"
Expand Down
4 changes: 4 additions & 0 deletions packages/polling-controller/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Fixed

- Relax peer dependency `@metamask/network-controller` from `^18.1.2` to `^18.0.0` ([#4338](https://github.com/MetaMask/core/pull/4338))

## [6.0.2]

### Changed
Expand Down
2 changes: 1 addition & 1 deletion packages/polling-controller/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@
"typescript": "~4.9.5"
},
"peerDependencies": {
"@metamask/network-controller": "^18.1.2"
"@metamask/network-controller": "^18.0.0"
Copy link
Contributor

@legobeat legobeat May 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this one should be aligned with the dependencies field? So if dropping here, then we'd also like to drop to dependencies["@metamask/network-controller"] = "^18.1.0" ?

},
"engines": {
"node": ">=16.0.0"
Expand Down
4 changes: 4 additions & 0 deletions packages/queued-request-controller/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Fixed

- Relax peer dependency `@metamask/network-controller` from `^18.1.2` to `^18.0.0` ([#4338](https://github.com/MetaMask/core/pull/4338))

## [0.10.0]

### Changed
Expand Down
2 changes: 1 addition & 1 deletion packages/queued-request-controller/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@
"typescript": "~4.9.5"
},
"peerDependencies": {
"@metamask/network-controller": "^18.1.2",
"@metamask/network-controller": "^18.0.0",
"@metamask/selected-network-controller": "^13.0.0"
},
"engines": {
Expand Down
4 changes: 4 additions & 0 deletions packages/selected-network-controller/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Fixed

- Relax peer dependency `@metamask/network-controller` from `^18.1.2` to `^18.0.0` ([#4338](https://github.com/MetaMask/core/pull/4338))

## [13.0.0]

### Changed
Expand Down
2 changes: 1 addition & 1 deletion packages/selected-network-controller/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@
"typescript": "~4.9.5"
},
"peerDependencies": {
"@metamask/network-controller": "^18.1.2",
"@metamask/network-controller": "^18.0.0",
"@metamask/permission-controller": "^9.0.0"
},
"engines": {
Expand Down
4 changes: 4 additions & 0 deletions packages/transaction-controller/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Fixed

- Relax peer dependency `@metamask/network-controller` from `^18.1.2` to `^18.0.0` ([#4338](https://github.com/MetaMask/core/pull/4338))

## [30.0.0]

### Fixed
Expand Down
2 changes: 1 addition & 1 deletion packages/transaction-controller/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@
"@babel/runtime": "^7.23.9",
"@metamask/approval-controller": "^6.0.0",
"@metamask/gas-fee-controller": "^15.0.0",
"@metamask/network-controller": "^18.1.2"
"@metamask/network-controller": "^18.0.0"
Copy link
Contributor

@legobeat legobeat May 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think for transaction-controller, we do want to ensure we use the same version of @metamask/eth-block-tracker? So I think for this specific package, we do want the bump in peerDependencies entry.

Copy link
Contributor Author

@mcmire mcmire May 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. The way we've been using the peer dependency is that it dictates which version of this package the project needs to use in order to be able to use this controller in all situations without breaking.

So are you saying that in order to use transaction-controller without breaking, a project needs to effectively rely on the version of network-controller that uses @metamask/eth-block-tracker?

Copy link
Contributor

@legobeat legobeat May 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So are you saying that in order to use transaction-controller without breaking, a project needs to effectively rely on the version of network-controller that uses @metamask/eth-block-tracker?

Yes, due to (or the other way around, depending on how you want to see the causality) the switch from nonce-tracker to @metamask/nonce-tracker and its associated switch from eth-block-tracker to @metamask/eth-block-tracker.

},
"engines": {
"node": ">=16.0.0"
Expand Down
4 changes: 4 additions & 0 deletions packages/user-operation-controller/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Fixed

- Relax peer dependency `@metamask/network-controller` from `^18.1.2` to `^18.0.0` ([#4338](https://github.com/MetaMask/core/pull/4338))

## [10.0.0]

### Changed
Expand Down
2 changes: 1 addition & 1 deletion packages/user-operation-controller/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@
"@metamask/approval-controller": "^6.0.0",
"@metamask/gas-fee-controller": "^15.0.0",
"@metamask/keyring-controller": "^16.0.0",
"@metamask/network-controller": "^18.1.2",
"@metamask/network-controller": "^18.0.0",
Copy link
Contributor

@legobeat legobeat May 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think maybe this should still be ^18.1.2 do to the dependency+peerDependency on transactions-controller? Or if we can drop to peerDependencies["@metamask/transaction-controller"] = "^29.1.0 | ^30.0.0.

"@metamask/transaction-controller": "^30.0.0"
},
"engines": {
Expand Down
16 changes: 8 additions & 8 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1756,7 +1756,7 @@ __metadata:
"@metamask/accounts-controller": ^14.0.0
"@metamask/approval-controller": ^6.0.0
"@metamask/keyring-controller": ^16.0.0
"@metamask/network-controller": ^18.1.2
"@metamask/network-controller": ^18.0.0
"@metamask/preferences-controller": ^11.0.0
languageName: unknown
linkType: soft
Expand Down Expand Up @@ -2011,7 +2011,7 @@ __metadata:
typedoc-plugin-missing-exports: ^2.0.0
typescript: ~4.9.5
peerDependencies:
"@metamask/network-controller": ^18.1.2
"@metamask/network-controller": ^18.0.0
languageName: unknown
linkType: soft

Expand Down Expand Up @@ -2331,7 +2331,7 @@ __metadata:
typescript: ~4.9.5
uuid: ^8.3.2
peerDependencies:
"@metamask/network-controller": ^18.1.2
"@metamask/network-controller": ^18.0.0
languageName: unknown
linkType: soft

Expand Down Expand Up @@ -2706,7 +2706,7 @@ __metadata:
typescript: ~4.9.5
uuid: ^8.3.2
peerDependencies:
"@metamask/network-controller": ^18.1.2
"@metamask/network-controller": ^18.0.0
languageName: unknown
linkType: soft

Expand Down Expand Up @@ -2807,7 +2807,7 @@ __metadata:
typedoc-plugin-missing-exports: ^2.0.0
typescript: ~4.9.5
peerDependencies:
"@metamask/network-controller": ^18.1.2
"@metamask/network-controller": ^18.0.0
"@metamask/selected-network-controller": ^13.0.0
languageName: unknown
linkType: soft
Expand Down Expand Up @@ -2880,7 +2880,7 @@ __metadata:
typedoc-plugin-missing-exports: ^2.0.0
typescript: ~4.9.5
peerDependencies:
"@metamask/network-controller": ^18.1.2
"@metamask/network-controller": ^18.0.0
"@metamask/permission-controller": ^9.0.0
languageName: unknown
linkType: soft
Expand Down Expand Up @@ -3081,7 +3081,7 @@ __metadata:
"@babel/runtime": ^7.23.9
"@metamask/approval-controller": ^6.0.0
"@metamask/gas-fee-controller": ^15.0.0
"@metamask/network-controller": ^18.1.2
"@metamask/network-controller": ^18.0.0
languageName: unknown
linkType: soft

Expand Down Expand Up @@ -3117,7 +3117,7 @@ __metadata:
"@metamask/approval-controller": ^6.0.0
"@metamask/gas-fee-controller": ^15.0.0
"@metamask/keyring-controller": ^16.0.0
"@metamask/network-controller": ^18.1.2
"@metamask/network-controller": ^18.0.0
"@metamask/transaction-controller": ^30.0.0
languageName: unknown
linkType: soft
Expand Down