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

Initialize composable observable store after update #20468

Merged

Conversation

Gudahtt
Copy link
Member

@Gudahtt Gudahtt commented Aug 16, 2023

Explanation

The composable observable store now updates state immediately when the structure is updated. Previously each store would only be updated after the first state change. This ensures that the composable observable store state is always complete.

Manual Testing Steps

This PR should ensure that the background state and the UI state contains an entry for every single controller immediately upon construction, without needing to wait for each controller to update. This is most obvious by looking at the background state snapshot captured by the errors.spec.js test suite. You can manually test this by looking at the background state in the background console shortly after the extension starts (e.g. using breakpoints or one of the state hooks).

Pre-merge author checklist

  • I've clearly explained:
    • What problem this PR is solving
    • How this problem was solved
    • How reviewers can test my changes
  • Sufficient automated test coverage has been added

Pre-merge reviewer checklist

  • Manual testing (e.g. pull and build branch, run in browser, test code being changed)
  • PR is linked to the appropriate GitHub issue
  • IF this PR fixes a bug in the release milestone, add this PR to the release milestone

If further QA is required (e.g. new feature, complex testing steps, large refactor), add the Extension QA Board label.

In this case, a QA Engineer approval will be be required.

@Gudahtt
Copy link
Member Author

Gudahtt commented Aug 16, 2023

This depends upon #20465 and #20457

@metamaskbot
Copy link
Collaborator

Builds ready [10f5431]
Page Load Metrics (1615 ± 78 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint1132571453115
domContentLoaded13712044161316077
load13712044161516378
domInteractive13712044161316077
Bundle size diffs [🚀 Bundle size reduced!]
  • background: -35 Bytes (-0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@Gudahtt Gudahtt force-pushed the rename-backup-module branch from f475859 to 31de96a Compare August 16, 2023 12:35
@Gudahtt Gudahtt force-pushed the initialize-composable-observable-store-after-structure-update branch from 10f5431 to 3b4b416 Compare August 16, 2023 12:41
@metamaskbot
Copy link
Collaborator

Builds ready [3b4b416]
Page Load Metrics (1506 ± 34 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint106167128157
domContentLoaded1382163115057235
load1382163115067234
domInteractive1382163115057235
Bundle size diffs [🚀 Bundle size reduced!]
  • background: -35 Bytes (-0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

@codecov
Copy link

codecov bot commented Aug 16, 2023

Codecov Report

Merging #20468 (9459765) into develop (8f178bc) will increase coverage by 0.00%.
Report is 1 commits behind head on develop.
The diff coverage is 100.00%.

@@           Coverage Diff            @@
##           develop   #20468   +/-   ##
========================================
  Coverage    68.68%   68.68%           
========================================
  Files          991      991           
  Lines        38440    38444    +4     
  Branches     10327    10329    +2     
========================================
+ Hits         26402    26405    +3     
- Misses       12038    12039    +1     
Files Changed Coverage Δ
app/scripts/lib/ComposableObservableStore.js 95.65% <100.00%> (-1.27%) ⬇️

Base automatically changed from rename-backup-module to develop August 16, 2023 19:29
@Gudahtt Gudahtt force-pushed the initialize-composable-observable-store-after-structure-update branch 2 times, most recently from 4d784de to d294f12 Compare August 17, 2023 13:24
@Gudahtt Gudahtt marked this pull request as ready for review August 17, 2023 13:27
@Gudahtt Gudahtt requested a review from a team as a code owner August 17, 2023 13:27
The composable observable store now updates state immediately when the
structure is updated. Previously each store would only be updated after
the first state change. This ensures that the composable observable
store state is always complete.
@Gudahtt Gudahtt force-pushed the initialize-composable-observable-store-after-structure-update branch from d294f12 to 861e7db Compare August 17, 2023 13:43
Gudahtt and others added 3 commits August 17, 2023 11:25
We now use the nullish coalescing operator when checkint store.state, so that we don't accidentally ignore falsy state.

Co-authored-by: Frederik Bolding <[email protected]>
A change on `develop` required another state update.
@metamaskbot
Copy link
Collaborator

Builds ready [9459765]
Page Load Metrics (1770 ± 69 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint1101911402311
domContentLoaded15902106176714268
load15902106177014469
domInteractive15902106176714268
Bundle size diffs [🚨 Warning! Bundle size has increased!]
  • background: 103 Bytes (0.00%)
  • ui: 0 Bytes (0.00%)
  • common: 0 Bytes (0.00%)

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.

Makes sense!

@Gudahtt Gudahtt merged commit 4cf886f into develop Aug 17, 2023
@Gudahtt Gudahtt deleted the initialize-composable-observable-store-after-structure-update branch August 17, 2023 14:52
@github-actions github-actions bot locked and limited conversation to collaborators Aug 17, 2023
@metamaskbot metamaskbot added the release-10.36.0 Issue or pull request that will be included in release 10.36.0 label Aug 17, 2023
@Gudahtt Gudahtt added release-10.34.5 Issue or pull request that will be included in release 10.34.5 and removed release-10.36.0 Issue or pull request that will be included in release 10.36.0 labels Sep 19, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
release-10.34.5 Issue or pull request that will be included in release 10.34.5 team-extension-platform
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants