Skip to content

test: test accounts-controller bump #31393

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

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

test: test accounts-controller bump #31393

wants to merge 2 commits into from
+8 −7

Conversation

ccharly
Copy link
Contributor

@ccharly ccharly commented Mar 28, 2025 •
edited by github-actions bot
Loading

Description

Testing.

Open in GitHub Codespaces

Related issues

Fixes:

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.

@github-actions GitHub Actions
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.

@metamaskbot metamaskbot added the team-accounts-framework Accounts Framework team label Mar 28, 2025
@socket-security Socket Security
Copy link

socket-security bot commented Mar 28, 2025 •
edited
Loading

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

Package New capabilities Transitives Size Publisher
npm/@metamask/[email protected] ➜ 26.1.0 Transitive: environment, network +13 7.82 MB danfinlay, gudahtt, kumavis, ...6 more
npm/@metamask/[email protected] None 0 194 kB mcmire
npm/@metamask/[email protected] environment +1 1.37 MB metamaskbot

View full report↗︎

@metamaskbot
Copy link
Collaborator

Builds ready [ff50c0b]
UI Startup Metrics (1185 ± 60 ms)
PlatformBuildTypePageMetricMean (ms)Min (ms)Max (ms)Std Dev (ms)P 75 (ms)P 95 (ms)
ChromeBrowserifyHomeuiStartup1185105414496012151298
load10339381274531149973
domContentLoaded1027933127054933987
domInteractive16133341528
firstPaint750831150392245957
backgroundConnect96273910
firstReactRender19155061934
getState11431767
initialActions001001
loadScripts815721105354842901
setupStore7515278
WebpackHomeuiStartup985695122682984992
load83954295666865918
domContentLoaded83353995365858906
domInteractive16124781435
firstPaint53055956347855880
backgroundConnect16115071539
firstReactRender14123131424
getState7416288
initialActions001001
loadScripts83053795165857904
setupStore7521289
FirefoxBrowserifyHomeuiStartup13881172194915014361760
load12461032180814312921612
domContentLoaded12461032180714312911612
domInteractive10344242289099
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect2416100112436
firstReactRender23185952430
getState7449578
initialActions001001
loadScripts12231016178614212701591
setupStore6429367
WebpackHomeuiStartup9768431482153887942
load8547411334137798947
domContentLoaded8547411333137797946
domInteractive11234217283497
firstPaintNaNNaNNaNNaNNaNNaN
backgroundConnect211382132447
firstReactRender19163131925
getState8463879
initialActions001001
loadScripts8377281310133783951
setupStore7536478
Bundle size diffs [🚀 Bundle size reduced!]
  • background: -659 Bytes (-0.01%)
  • ui: 0 Bytes (0%)
  • common: -25 Bytes (0%)

@ccharly ccharly mentioned this pull request Mar 28, 2025
Merged
4 tasks
montelaidev added a commit to MetaMask/core that referenced this pull request Apr 1, 2025
@ccharly @montelaidev
fix(accounts-controller): do not fire events during update blocks (#…
87ebc4b 
…5555)

## Explanation

Events should be fired after making a state update. Otherwise we could
end up with race conditions like some other component react to
`AccountsController:accountAdded` and try to call
`getAccount(account.id)`, it might not the same value compared to the
`account` being passed during the event (since the state update has not
been persisted yet).

This PR now moves all events firing after the state update.

It also includes a small refactor of the `KeyringController:stateChange`
handler (which made the new events firing logic, a bit easier).

Test E2E PR:
- MetaMask/metamask-extension#31393 (CI is
passing)

## References

N/A

## Changelog

TODO

## 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

---------

Co-authored-by: Monte Lai <[email protected]>
@ccharly ccharly closed this Apr 7, 2025
@github-actions github-actions bot locked and limited conversation to collaborators Apr 7, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

team-accounts-framework Accounts Framework team

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ccharly @metamaskbot