Update controller packages to match core v42 #6125
Merged
+81
−134
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This comment was marked as resolved.
This comment was marked as resolved.
Gudahtt
changed the title
Gudahtt
force-pushed
the
update-controllers-v40
branch
2 times, most recently
from
7143e1d to
125aa70
Compare
Gudahtt
force-pushed
the
update-controllers-v42
branch
from
5b9530f to
77c6815
Compare
Gudahtt
force-pushed
the
update-controllers-v40
branch
2 times, most recently
from
47ca3b1 to
381bb42
Compare
Gudahtt
force-pushed
the
update-controllers-v42
branch
from
77c6815 to
65fc728
Compare
Gudahtt
force-pushed
the
update-controllers-v40
branch
4 times, most recently
from
a124353 to
120d8a7
Compare
Gudahtt
force-pushed
the
update-controllers-v42
branch
from
65fc728 to
5ff5eac
Compare
Gudahtt
commented
May 8, 2023
Gudahtt
commented
May 8, 2023
Gudahtt
force-pushed
the
update-controllers-v42
branch
from
5ff5eac to
2d44482
Compare
Gudahtt
commented
May 8, 2023
Gudahtt
force-pushed
the
update-controllers-v42
branch
4 times, most recently
from
5553673 to
fdade64
Compare
Most controller packages have been updated to the versions included in the core monorepo v42 release. The keyring controller was the only package held back, due to the BigInt incompatibility. The breaking change for most of these updates was related to the removal of the `isomorphic-fetch` package. That package would polyfill the `fetch` API if it wasn't present. It's not a breaking change for mobile because mobile already includes its own polyfill for the `fetch` API. The only other breaking change is to the gas fee controller, which has made a constructor parameter required rather than optional. This does not affect mobile because mobile already sets that parameter.. See here for the release notes for each updated controller: https://github.com/MetaMask/core/releases/tag/v42.0.0 This progresses MetaMask/mobile-planning#798
Gudahtt
force-pushed
the
update-controllers-v42
branch
from
fdade64 to
33bb454
Compare
Gudahtt
commented
May 8, 2023
Gudahtt
commented
May 8, 2023
Gudahtt
commented
May 8, 2023
Gudahtt
added
needs-dev-review
|
The Bitrise iOS e2e tests succeeded: https://app.bitrise.io/build/3abfec4a-1077-45c6-89c1-0a3872a319e9 |
|
@Gudahtt I sent over a few comments. Once you tracked down the results on the Android side let me know and I can review again. |
|
Got a successful Android e2e test run here (passing all tests except 2 that are failing on |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Development & PR Process
release-xxlabel to identify the PR slated for a upcoming release (will be used in release discussion)needs-dev-reviewlabel when work is completedneeds-qalabel when dev review is completedQA Passedlabel when QA has signed offDescription
Most controller packages have been updated to the versions included in the core monorepo v42 release. The keyring controller was the only package held back, due to the BigInt incompatibility.
The breaking change for most of these updates was related to the removal of the
isomorphic-fetchpackage. That package would polyfill thefetchAPI if it wasn't present. It's not a breaking change for mobile because mobile already includes its own polyfill for thefetchAPI.The only other breaking change is to the gas fee controller, which has made a constructor parameter required rather than optional. This does not affect mobile because mobile already sets that parameter..
See here for the release notes for each updated controller: https://github.com/MetaMask/core/releases/tag/v42.0.0
A number of packages had their ranges reduced from
^to~or were pinned so that the scope of this PR could be kept reasonably small. Two required resolutions to avoid bringing in unwanted updates. Here are the details:@metamask/controller-utilsrange set to~3.0.0with a resolutionLater versions of this package use the @metamask/utils package, which has introduced BigInt-related problems for mobile recently. We can remove this resolution in a future PR after carefully considering the impact, or after the React Native upgrade.
@metamask/approval-controllerrange set to~2.0.0with a resolutionThis was necessary to avoid dependencies upon later
@metamask/controller-utilsversions"@metamask/assets-controllerspinned to5.0.0Avoids dependency upon
@metamask/[email protected], which is required by@metamask/[email protected]@metamask/gas-fee-controllerpinned to4.0.0Avoids dependency upon
@metamask/[email protected], which is required by@metamask/[email protected]@metamask/permission-controllerrange reduced to~3.0.0Avoids dependency upon later versions of
@metamask/approval-controller@metamask/transaction-controllerpinned to4.0.0Avoids dependency upon
@metamask/[email protected], which is required by@metamask//[email protected]Issue
Progresses https://github.com/MetaMask/mobile-planning/issues/798
Checklist