Skip to content
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.

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

chore: Bump @metamask/providers to v18 #28757

Merged
merged 4 commits into from
Nov 27, 2024
Merged

chore: Bump @metamask/providers to v18 #28757

merged 4 commits into from
Nov 27, 2024

Conversation

Gudahtt
Copy link
Member

@Gudahtt Gudahtt commented Nov 27, 2024

Description

The @metamask/providers package has been updated from v14 to v18. The breaking changes do not require any changes to the extension. They include:

  • Removal of certain provider features, which were later restored
  • Updates to JSON-RPC packages that used to impact error serialization in a breaking way, but no longer do (as of rpc-errors v7.0.1)
  • webextension-polyfill added as a peer dependency
  • Sub-path exports no longer work due to addition of exports and ESM build.

Altogether this should result in no functional changes, aside from eliminating some redundant older copies of libraries (reducing bundle size).

This update was difficult because of the need for a subpath export in inpage.js. We want to import just the initializeInpageProvider module without the rest of the package, which means we need to use a subpath export (because browserify doesn't support tree shaking). But this was challenging because the export maps added in v17 prevent the use of any subpath exports not defined in the export map. There was no entry that worked correctly across webpack and browserify. This was resolved by a new entry added in MetaMask/providers#391 (as part of v18.2.0)

Changelog: https://github.com/MetaMask/providers/blob/main/CHANGELOG.md#1820

Open in GitHub Codespaces

Related issues

N/A

Manual testing steps

N/A

Screenshots/Recordings

N/A

Pre-merge author checklist

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

@Gudahtt
Copy link
Member Author

Gudahtt commented Nov 27, 2024

@metamaskbot update-policies

Copy link

socket-security bot commented Nov 27, 2024

New and removed dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/@metamask/providers@18.2.0 None 0 440 kB metamaskbot

🚮 Removed packages: npm/@metamask/providers@18.1.1, npm/json-rpc-middleware-stream@5.0.1

View full report↗︎

@metamaskbot
Copy link
Collaborator

Policy update failed. You can review the logs or retry the policy update here

@Gudahtt
Copy link
Member Author

Gudahtt commented Nov 27, 2024

@metamaskbot update-policies

@metamaskbot
Copy link
Collaborator

No policy changes

@Gudahtt Gudahtt force-pushed the bump-providers-v18 branch 2 times, most recently from 733faa0 to beab613 Compare November 27, 2024 20:46
The `@metamask/providers` package has been updated from v14 to v18. The
breaking changes do not require any changes to the extension. They
include:
* Removal of certain provider features, which were later restored
* Updates to JSON-RPC packages that used to impact error serialization
in a breaking way, but no longer do (as of rpc-errors v7.0.1)
* `webextension-polyfill` added as a peer dependency
* Sub-path exports no longer work due to addition of exports and ESM
build.

Altogether this should result in no functional changes, aside from
eliminating some redundant older copies of libraries (reducing bundle
size).

Changelog: https://github.com/MetaMask/providers/blob/main/CHANGELOG.md#1811
The subpath export keeps the inpage bundle small. The `.cjs` file
extension is required for `browserify` to find the file.
@Gudahtt Gudahtt marked this pull request as ready for review November 27, 2024 21:50
@Gudahtt Gudahtt requested a review from a team as a code owner November 27, 2024 21:50
Copy link
Contributor

@cryptodev-2s cryptodev-2s left a comment

Choose a reason for hiding this comment

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

LGTM!

@metamaskbot
Copy link
Collaborator

Builds ready [eb7c2d5]
Page Load Metrics (1825 ± 48 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint16912057182910249
domContentLoaded1649198017928842
load16832064182510048
domInteractive237241168
backgroundConnect1084322110
firstReactRender154922105
getState42107792110
initialActions00000
loadScripts1273153813828340
setupStore611711
uiStartup19182479206114971
Bundle size diffs
  • background: 0 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@Gudahtt Gudahtt added this pull request to the merge queue Nov 27, 2024
Copy link
Contributor

@mcmire mcmire left a comment

Choose a reason for hiding this comment

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

The only thing that sticks out to me is the bump to a newer version of @metamask/rpc-errors, particularly 7.0.0. It seems that we could see another change to error messages in dapps. Granted, 7.0.0 restores some behavior that was changed in 5.0.0, so it's probably for the better if there is a material effect. I also see that the extension was already upgraded to 7.0.0 in #24496. So perhaps this is nothing, but I thought I'd call it out anyway.

LGTM.

Merged via the queue into develop with commit 6aeb777 Nov 27, 2024
75 checks passed
@Gudahtt Gudahtt deleted the bump-providers-v18 branch November 27, 2024 22:26
@github-actions github-actions bot locked and limited conversation to collaborators Nov 27, 2024
@metamaskbot metamaskbot added the release-12.9.0 Issue or pull request that will be included in release 12.9.0 label Nov 27, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
release-12.9.0 Issue or pull request that will be included in release 12.9.0 team-wallet-framework
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants