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

Jl/caip multichain/cleanup middleware destroy #28751

Conversation

jiexi
Copy link
Contributor

@jiexi jiexi commented Nov 27, 2024

Description

Open in GitHub Codespaces

Related issues

Core: MetaMask/core#4984

Manual testing steps

  1. Go to this page...

Screenshots/Recordings

Before

After

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.

@jiexi
Copy link
Contributor Author

jiexi commented Nov 27, 2024

@metamaskbot update-policies

Copy link
Contributor

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

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-previews/[email protected] Transitive: filesystem +8 1.01 MB mcmire, metamaskbot

🚮 Removed packages: npm/@json-schema-tools/[email protected], npm/@metamask-previews/[email protected]

View full report↗︎

@metamaskbot
Copy link
Collaborator

Policies updated.
👀 Please review the diff for suspicious new powers.

🧠 Learn how: https://lavamoat.github.io/guides/policy-diff/#what-to-look-for-when-reviewing-a-policy-diff

@metamaskbot
Copy link
Collaborator

Builds ready [81867db]
Page Load Metrics (1783 ± 67 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint15722036178113665
domContentLoaded15081961173712962
load15752056178313967
domInteractive24180655526
backgroundConnect11101522713
firstReactRender1675362010
getState45216126
initialActions01000
loadScripts10571464127510651
setupStore65914147
uiStartup17592352198015474
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 30.81 KiB (0.53%)
  • ui: 626 Bytes (0.01%)
  • common: 466.93 KiB (5.74%)

@metamaskbot
Copy link
Collaborator

Builds ready [0f0bb96]
Page Load Metrics (1709 ± 63 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint15692075171712560
domContentLoaded15222065168013464
load15312077170913263
domInteractive207745168
backgroundConnect1472402110
firstReactRender1679422411
getState581202110
initialActions01000
loadScripts10961526123710852
setupStore611811
uiStartup17062323188414771
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 30.81 KiB (0.53%)
  • ui: 626 Bytes (0.01%)
  • common: 466.93 KiB (5.74%)

@metamaskbot
Copy link
Collaborator

Builds ready [3005665]
Page Load Metrics (1659 ± 87 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint25021801579351169
domContentLoaded14352102164317082
load14432184165918287
domInteractive22563094
backgroundConnect883192010
firstReactRender1493362713
getState20107432512
initialActions01000
loadScripts10471682123315273
setupStore589182311
uiStartup161825351917251120
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 30.8 KiB (0.57%)
  • ui: 626 Bytes (0.01%)
  • common: 466.84 KiB (5.68%)

jiexi added a commit to MetaMask/core that referenced this pull request Dec 2, 2024
…try (#4984)

## Explanation

Ensures that the subscription entry for a subscription is removed when
the middleware is destroyed

## References

Related: MetaMask/metamask-extension#28751

## Changelog

<!--
If you're making any consumer-facing changes, list those changes here as
if you were updating a changelog, using the template below as a guide.

(CATEGORY is one of BREAKING, ADDED, CHANGED, DEPRECATED, REMOVED, or
FIXED. For security-related issues, follow the Security Advisory
process.)

Please take care to name the exact pieces of the API you've added or
changed (e.g. types, interfaces, functions, or methods).

If there are any breaking changes, make sure to offer a solution for
consumers to follow once they upgrade to the changes.

Finally, if you're only making changes to development scripts or tests,
you may replace the template below with "None".
-->

### `@metamask/package-a`

- **<CATEGORY>**: Your change here
- **<CATEGORY>**: Your change here

### `@metamask/package-b`

- **<CATEGORY>**: Your change here
- **<CATEGORY>**: Your change here

## Checklist

- [ ] I've updated the test suite for new or updated code as appropriate
- [ ] I've updated documentation (JSDoc, Markdown, etc.) for new or
updated code as appropriate
- [ ] I've highlighted breaking changes using the "BREAKING" category
above as appropriate
- [ ] I've prepared draft pull requests for clients and consumer
packages to resolve any breaking changes
@jiexi jiexi marked this pull request as ready for review December 2, 2024 20:30
@jiexi jiexi requested a review from a team as a code owner December 2, 2024 20:30
@jiexi jiexi merged commit 21b2181 into jl/caip-multichain-migrate-core Dec 2, 2024
7 of 11 checks passed
@jiexi jiexi deleted the jl/caip-multichain/cleanup-middleware-destroy branch December 2, 2024 20:30
@github-actions github-actions bot locked and limited conversation to collaborators Dec 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants