Address ESlint warnings #11100
Assignees
Labels
area-performance
Issues relating to slowness of app, cpu usage, and/or blank screens.
good first issue
Good for newcomers
release-7.35.0
Issue or pull request that will be included in release 7.35.0
team-mobile-platform
Comments
tommasini
added
area-performance
tommasini
added a commit
that referenced
this issue
Sep 9, 2024
## **Description** This PR aims to update eslint to match extension version and to support the typescript version on mobile and remove this warning: ``` WARNING: You are currently running a version of TypeScript which is not officially supported by @typescript-eslint/typescript-estree. You may find that it works just fine, or you may not. SUPPORTED TYPESCRIPT VERSIONS: >=3.3.1 <5.2.0 YOUR TYPESCRIPT VERSION: 5.4.5 ``` There is some things that we need to do to finish this PR: - [x] Solve new linter issues - [x] Consider keep or not the `@babel/preset-react` to the babel parser config to javascript files (not needed, deepcheck flagged it as not used) - [x] Discover why app/selectors/accountTrackerController.test.tsx was triggering an issue on eslint - [x] Discover why we still have eslint issues here: e2e/pages/Settings/AesCryptoTestForm.ts (maybe we just need to convert it to JS since every e2e file is on JS ) - [x] Investigate if we should disable no-unused-vars for JS files since our efforts should be to transition those files to TypeScript (we didn't flag this until now, so let's keep this unused vars unchecked by the linter) - [x] Investigate if we should we enable empty functions on test files (we shouldn't) - [x] Remove unused dev package @metamask/eslint-config Next steps after this PR: - [ ] Remove the patch to @metamask/typescript-eslint-config (to remove it we need to bump @metamask/typescript-eslint-config and fix the eslint issues that came with it, this will need to be addressed on other PR) #11103 - [ ] Add @metamask/eslint-config and fix the issues that came by doing it around (10248 problems (10219 errors, 29 warnings) - #11102 5203 errors and 0 warnings potentially fixable with the --fix option.) - [ ] Address warnings that are shown when running `yarn lint` like `react/no-unstable-nested-components`, `jest/valid-expect` and `jest/no-disabled-tests` - #11100 ## **Related issues** Fixes: ## **Manual testing steps** 1. Go to this page... 2. 3. ## **Screenshots/Recordings** https://github.com/user-attachments/assets/d195a93e-bccc-4b7b-b4c6-c55dcfa15565 <!-- 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. --------- Co-authored-by: LeoTM <1881059+leotm@users.noreply.github.com>
devin-ai-integration bot
pushed a commit
that referenced
this issue
Sep 9, 2024
## **Description** This PR aims to update eslint to match extension version and to support the typescript version on mobile and remove this warning: ``` WARNING: You are currently running a version of TypeScript which is not officially supported by @typescript-eslint/typescript-estree. You may find that it works just fine, or you may not. SUPPORTED TYPESCRIPT VERSIONS: >=3.3.1 <5.2.0 YOUR TYPESCRIPT VERSION: 5.4.5 ``` There is some things that we need to do to finish this PR: - [x] Solve new linter issues - [x] Consider keep or not the `@babel/preset-react` to the babel parser config to javascript files (not needed, deepcheck flagged it as not used) - [x] Discover why app/selectors/accountTrackerController.test.tsx was triggering an issue on eslint - [x] Discover why we still have eslint issues here: e2e/pages/Settings/AesCryptoTestForm.ts (maybe we just need to convert it to JS since every e2e file is on JS ) - [x] Investigate if we should disable no-unused-vars for JS files since our efforts should be to transition those files to TypeScript (we didn't flag this until now, so let's keep this unused vars unchecked by the linter) - [x] Investigate if we should we enable empty functions on test files (we shouldn't) - [x] Remove unused dev package @metamask/eslint-config Next steps after this PR: - [ ] Remove the patch to @metamask/typescript-eslint-config (to remove it we need to bump @metamask/typescript-eslint-config and fix the eslint issues that came with it, this will need to be addressed on other PR) #11103 - [ ] Add @metamask/eslint-config and fix the issues that came by doing it around (10248 problems (10219 errors, 29 warnings) - #11102 5203 errors and 0 warnings potentially fixable with the --fix option.) - [ ] Address warnings that are shown when running `yarn lint` like `react/no-unstable-nested-components`, `jest/valid-expect` and `jest/no-disabled-tests` - #11100 ## **Related issues** Fixes: ## **Manual testing steps** 1. Go to this page... 2. 3. ## **Screenshots/Recordings** https://github.com/user-attachments/assets/d195a93e-bccc-4b7b-b4c6-c55dcfa15565 <!-- 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. --------- Co-authored-by: LeoTM <1881059+leotm@users.noreply.github.com>
7 tasks
metamaskbot
added
the
release-7.35.0
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
What is this about?
When running
yarn linton the terminal we can see this 3 warnings:The goal of this issue is address all the warnings.
Scenario
No response
Design
No response
Technical Details
No response
Threat Modeling Framework
No response
Acceptance Criteria
No response
Stakeholder review needed before the work gets merged
References
No response
The text was updated successfully, but these errors were encountered: