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

refactor: Unflatten tests #28776

Draft
wants to merge 86 commits into
base: jongsun/perf/redux/241204-unflatten-metamask-slice
Choose a base branch
from

Conversation

MajorLift
Copy link
Contributor

@MajorLift MajorLift commented Nov 27, 2024

Description

Remaining downstream references to metamask slice, post-unflattening of selectors, hooks, mock objects:

  • Search term: state\.metamask\.(?!(\w+Controller|AccountTracker|SnapsRegistry))
    • Include: *.test.*
    • Exclude: ''
    • Results: 80 results - 11 files

Open in GitHub Codespaces

Related issues

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.

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.

@MajorLift MajorLift changed the base branch from develop to jongsun/perf/redux/241008-unflatten-MetamaskController-stores November 27, 2024 16:03
@gauthierpetetin gauthierpetetin added the team-tiger Tiger team (for tech debt reduction + performance improvements) label Nov 27, 2024
@MajorLift MajorLift force-pushed the jongsun/perf/redux/241008-unflatten-MetamaskController-stores branch 5 times, most recently from a7fc45e to 907d672 Compare December 2, 2024 10:11
@MajorLift MajorLift force-pushed the jongsun/perf/redux/241127-unflatten-downstream-references-to-metamask-slice branch from a1fba1d to b75f215 Compare December 2, 2024 10:13
@MajorLift MajorLift force-pushed the jongsun/perf/redux/241008-unflatten-MetamaskController-stores branch from 907d672 to fddacbc Compare December 2, 2024 19:22
@MajorLift MajorLift force-pushed the jongsun/perf/redux/241127-unflatten-downstream-references-to-metamask-slice branch from b75f215 to 5317fec Compare December 2, 2024 19:25
@MajorLift MajorLift force-pushed the jongsun/perf/redux/241008-unflatten-MetamaskController-stores branch from fddacbc to f5b6067 Compare December 4, 2024 14:48
@MajorLift MajorLift force-pushed the jongsun/perf/redux/241127-unflatten-downstream-references-to-metamask-slice branch 2 times, most recently from 817dfc4 to 0565217 Compare December 4, 2024 15:34
@MajorLift MajorLift force-pushed the jongsun/perf/redux/241204-unflatten-metamask-slice branch 8 times, most recently from cf9e3ae to f854428 Compare December 11, 2024 09:35
@MajorLift MajorLift changed the title perf: Unflatten downstream references to metamask Redux slice (5/5) refactor: Unflatten downstream references to metamask Redux slice (5/5) Dec 11, 2024
@MajorLift MajorLift force-pushed the jongsun/perf/redux/241204-unflatten-metamask-slice branch 6 times, most recently from 56716cd to 62df3ae Compare December 12, 2024 17:09
@MajorLift MajorLift self-assigned this Dec 12, 2024
@MajorLift MajorLift force-pushed the jongsun/perf/redux/241204-unflatten-metamask-slice branch 2 times, most recently from b5ddb61 to 62d01dd Compare December 18, 2024 03:30
@MajorLift MajorLift force-pushed the jongsun/perf/redux/241204-unflatten-metamask-slice branch 3 times, most recently from 1add77c to c47ed91 Compare January 6, 2025 19:39
@MajorLift MajorLift added DO-NOT-MERGE Pull requests that should not be merged team-extension-platform Extension Platform team and removed team-wallet-framework labels Jan 6, 2025
@MajorLift MajorLift changed the title refactor: Unflatten downstream references to metamask Redux slice (5/5) refactor: Unflatten test and downstream references to metamask Redux slice (6/6) Jan 7, 2025
@MajorLift MajorLift changed the title refactor: Unflatten test and downstream references to metamask Redux slice (6/6) refactor: Unflatten tests and downstream references to metamask Redux slice (6/6) Jan 7, 2025
@MajorLift MajorLift changed the title refactor: Unflatten tests and downstream references to metamask Redux slice (6/6) refactor: Unflatten tests Jan 9, 2025
github-merge-queue bot pushed a commit that referenced this pull request Jan 13, 2025
…ate` slice (#28703)

## **Description**

1. Moves properties in the `metamask` Redux slice that are not part of
background (i.e. controller-derived) state into the `appState` slice.

The affected state properties are as follows:

```
  customNonceValue: string;
  isAccountMenuOpen: boolean;
  isNetworkMenuOpen: boolean;
  nextNonce: string | null;
  pendingTokens: {
    [address: string]: Token & { isCustom?: boolean; unlisted?: boolean };
  };
  welcomeScreenSeen: boolean;
  confirmationExchangeRates: ContractExchangeRates;
```

- Foreground properties that are a part of `AppStateController` state
have not been moved into the `appState` slice, and will remain in the
unflattened `metamask` slice.
- It's not immediately clear why the properties listed above were
excluded from `AppStateController` state, but since all of them appear
to neither require persistence beyond a given session, nor anonymization
of PII, I'm opting to maintain the status quo and keep them out of
controller state.
- The `isInitialized` property is not included, because it's derived
from background state before the Redux store is instantiated, and is
updated by the `PatchStore` which supplies it directly to the `metamask`
slice.

2. Groups `appState` slice selectors in `ui/selectors/selectors.js`
together.

[![Open in GitHub
Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/28703?quickstart=1)

## **Related issues**

- Closes #29601
- Closes MetaMask/MetaMask-planning#2895
- 0 of 7 PRs that will close
#29600
- Blocking MetaMask/MetaMask-planning#2894
  - #28723
  - #28929
  - #28776

## **Manual testing steps**

## **Screenshots/Recordings**

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

- [x] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask
Extension Coding
Standards](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/CODING_GUIDELINES.md).
- [x] I've completed the PR template to the best of my ability
- [x] I’ve included tests if applicable
- [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [x] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-extension/blob/develop/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **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.
@MajorLift MajorLift force-pushed the jongsun/perf/redux/241204-unflatten-metamask-slice branch from c47ed91 to 7af2f6a Compare January 14, 2025 11:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DO-NOT-MERGE Pull requests that should not be merged team-extension-platform Extension Platform team team-tiger Tiger team (for tech debt reduction + performance improvements)
Projects
Status: Needs more work from the author
Development

Successfully merging this pull request may close these issues.

3 participants