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(js-ts): Convert app/core/Engine.test.js to TypeScript #11799

Closed

Conversation

devin-ai-integration[bot]
Copy link
Contributor

@devin-ai-integration devin-ai-integration bot commented Oct 15, 2024

TypeScript Conversion: app/core/Engine.test.js (Work in Progress)

Description

This PR is a work in progress to convert the file 'app/core/Engine.test.js' from JavaScript to TypeScript. The conversion has not been completed yet, but this PR serves as a placeholder for the upcoming changes.

Current Status

  • The file 'Engine.test.js' has not been converted to TypeScript yet.
  • No changes have been made to the original JavaScript file.

Next Steps

  • Rename the file from 'Engine.test.js' to 'Engine.test.ts'
  • Add type annotations for variables, functions, and test cases
  • Resolve any TypeScript errors that arise during the conversion
  • Ensure all tests pass after the conversion
  • Verify linter compliance

Notes

  • The actual conversion work will be done in subsequent commits.
  • No semantic changes will be made to the code logic during the conversion process.
  • PropTypes will be removed as they are not needed in TypeScript.

Testing (To be completed)

  • All unit tests in 'Engine.test.ts' should pass after conversion
  • TypeScript linter (yarn lint:tsc) should show no errors
  • General linter (yarn lint) should show only acceptable warnings

Link to Devin run: https://preview.devin.ai/devin/48f3b260585d4beda93f99698235dcda

Please review this placeholder PR and provide any feedback or suggestions for the upcoming conversion process.

@devin-ai-integration devin-ai-integration bot added needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) team-mobile-platform Run Smoke E2E Triggers smoke e2e on Bitrise skip-sonar-cloud Only used for bypassing sonar cloud when failures are not relevant to the changes. labels Oct 15, 2024
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.

@github-actions github-actions bot added the team-ai AI team (for the Devin AI bot) label Oct 15, 2024
Copy link
Contributor

github-actions bot commented Oct 15, 2024

https://bitrise.io/ Bitrise

✅✅✅ pr_smoke_e2e_pipeline passed on Bitrise! ✅✅✅

Commit hash: ff535d1
Build link: https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/1c74a104-b955-4110-a3be-da6c0e3e2321

Note

  • You can kick off another pr_smoke_e2e_pipeline on Bitrise by removing and re-applying the Run Smoke E2E label on the pull request

gambinish and others added 9 commits October 15, 2024 17:13
## **Description**

Resurrecting an older PR of mine to provide support for custom network
images: #10448

## **Related issues**

Fixes: Support custom network icons on mobile.

## **Manual testing steps**

1. Add custom network (In this case Flare Mainnet or Songbird Testnet)
2. Icon should appear wherever Network Icon should appear

## **Screenshots/Recordings**


https://github.com/user-attachments/assets/6ea5b310-4b6a-4b6c-9656-d892bae83fc3

## **Pre-merge author checklist**

- [x] I’ve followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.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-mobile/blob/main/.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.
## **Description**

This PR reduces the sampling rate by 50% Sentry Performance. We are
currently using too much of our allocation.

## **Related issues**

Fixes:

## **Manual testing steps**

1. Go to this page...
2.
3.

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

- [ ] I’ve followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've completed the PR template to the best of my ability
- [ ] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.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.
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->
<img width="1052" alt="Screenshot 2024-10-11 at 20 29 04"
src="https://github.com/user-attachments/assets/a5680804-7bc8-45c4-a27a-d790bd5f9a5d">
<img width="766" alt="Screenshot 2024-10-16 at 00 01 02"
src="https://github.com/user-attachments/assets/aa4689f2-9ce0-4dc7-9b99-f24fd4db1b78">
<img width="699" alt="Screenshot 2024-10-15 at 17 57 12"
src="https://github.com/user-attachments/assets/dc31684a-4dea-4f26-8b18-c5497679a0ce">

This task is for adding custom spans to track activities that happen
between app start and wallet UI load. The screenshot below is an example
of a trace for Wallet UI load that takes about a minute to load. During
that time, we can see a large gap between app start spans and the
initial http requests.

The goal here is to isolate these areas and track them with custom
spans. Once implemented, we can expect to see the custom spans appearing
within the gap, which would inform us of the areas to optimize

Issue: MetaMask/mobile-planning#1940

Technical Details
* Added custom span for when the Login screen is mounted to when the
login button is tapped
* Added span for when the login button is tapped to when the wallet view
is mounted
* Added custom span for Engine initialization process
* Added custom span for Store creation
* Added custom span Storage rehydration
* Added custom span fro Create New Wallet to Choose Password
* Added custom span for Biometrics authentication

## **Related issues**

Fixes:

## **Manual testing steps**

1. Go to this page...
2.
3.

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

- [ ] I’ve followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've completed the PR template to the best of my ability
- [ ] I’ve included tests if applicable
- [ ] I’ve documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] I’ve applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.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.
## **Description**
Persisting back again token list and phishing list.

From 11KBs to 8MB to an wallet with:
* 2 accounts
* 7 Networks added plus Ethereum and LInea Mainnet
Tokens imported by network
* - Ethereum: 16
* - Linea: 6
* - Avalanche: 9 
* - Binance: 10
* - Base: 1
* - Optimism: 3
* - Polygon: 3
* - Palm:0
* - ZkSync: 1
* - Arbitrum: 5

<img
src="https://github.com/user-attachments/assets/a61cc6a3-44ec-4c93-b2d0-5fdc22157c64"
width:200 />

App launch times e2e: 
1-
https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/b1ae0ba0-cee8-472d-978e-17882f132740
2-
https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/d8fda877-0c9a-4448-a75a-cc4921551e16
3-
https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/38dc5140-e18e-494c-a86a-22492554e837

------After fixing e2e------
1-
https://app.bitrise.io/app/be69d4368ee7e86d/pipelines/797b8317-6116-4bb5-8a12-e60a8e1b7aeb
## **Related issues**

Fixes:

## **Manual testing steps**

1. Go to this page...
2.
3.

## **Screenshots/Recordings**


Exploring app (tokens):

https://github.com/user-attachments/assets/638e42de-2219-423f-a9ee-6b18ada57751


Phishing detector in app browser:

https://github.com/user-attachments/assets/62f63451-fa7e-445f-b875-21155b13a8bc


### **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 Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.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-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

## **Pre-merge reviewer checklist**

- [x] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [x] 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.

---------

Co-authored-by: Aslau Mario-Daniel <[email protected]>
<!--
Please submit this PR as a draft initially.
Do not mark it as "Ready for review" until the template has been
completely filled out, and PR status checks have passed at least once.
-->

## **Description**

<!--
Write a short description of the changes included in this pull request,
also include relevant motivation and context. Have in mind the following
questions:
1. What is the reason for the change?
2. What is the improvement/solution?
-->

This PR adds the staking confirmation screen with some mock data being
used temporarily.

### Change List
- Add staking confirmation screen.
- Connect existing `<StakeInputView />` to staking confirmation screen
when user enters valid amount to stake.


## **Related issues**
Ticket: [FE] Build staking input confirmation screen -
([link](https://consensyssoftware.atlassian.net/browse/STAKE-824))
Figma Designs -
[link](https://www.figma.com/design/1c0Y9jDJe6p0j82jydJDcs/Mobile-Staking?node-id=2979-22435&m=dev)

## **Manual testing steps**

1. Add `export MM_POOLED_STAKING_UI_ENABLED=true` to your `.js.env`
file.
2. Click on Ethereum In the token list page
3. Scroll down a bit and click "Stake more". This should open the stake
input view (not related to this PR)
4. Enter a valid amount to stake and click "Confirm"
5. You should be redirected to a staking confirmation screen. The screen
should display the amount to stake in `wETH` and Fiat.

## **Screenshots/Recordings**

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

### **Before**

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

Nothing would happen after clicking "Confirm" on the stake input view.
This screen is new.

### **After**

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


https://github.com/user-attachments/assets/84ea4c52-50c5-48c3-8077-2c2e8a92bf21


## **Pre-merge author checklist**

- [x] I’ve followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.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
- [ ] 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-mobile/blob/main/.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.
Copy link
Contributor Author

Closing this PR as requested.

@github-actions github-actions bot locked and limited conversation to collaborators Oct 16, 2024
@github-actions github-actions bot removed the needs-dev-review PR needs reviews from other engineers (in order to receive required approvals) label Oct 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Run Smoke E2E Triggers smoke e2e on Bitrise skip-sonar-cloud Only used for bypassing sonar cloud when failures are not relevant to the changes. team-ai AI team (for the Devin AI bot) team-mobile-platform
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants