Skip to content

refactor(perps): align with @metamask/eslint-config for Core extraction#26064

Merged
abretonc7s merged 16 commits into
mainfrom
refactor/perps/remaining-core-steps
Feb 16, 2026
Merged

refactor(perps): align with @metamask/eslint-config for Core extraction#26064
abretonc7s merged 16 commits into
mainfrom
refactor/perps/remaining-core-steps

Conversation

@abretonc7s

@abretonc7s abretonc7s commented Feb 13, 2026

Copy link
Copy Markdown
Contributor

Description

Aligns app/controllers/perps/ with @metamask/eslint-config (Core's ESLint rules) and adds an automated sync script, preparing the perps controller for extraction into a standalone @metamask/perps-controller package in Core.

Why

The perps controller will move to Core, which enforces a strict ESLint config. Aligning now means the migration to Core is a straight file copy rather than mixed with hundreds of style fixes.

What changed

  • .eslintrc.js — Added a perps-specific override block enforcing Core rules: no-restricted-syntax, naming-convention, prefer-nullish-coalescing, explicit-function-return-type, import/order, jsdoc/*, and more. Test files are excluded.
  • 65 perps source/test files — Applied all lint fixes: type over interface, ES private fields (#field), ?? over ||, import ordering, explicit return types, JSDoc formatting, etc. Fixed the 59 TypeScript errors introduced by the auto-fixes.
  • package.json — Added eslint-plugin-promise (scoped to perps override only).
  • scripts/perps/validate-core-sync.sh — New 6-step script that copies source files (no tests) to Core, verifies build fixes, runs ESLint, builds, and lints. All steps pass with 0 suppressions. Tests remain in mobile as the source of truth for now.

Changelog

CHANGELOG entry: null

Related issues

Fixes: Perps controller Core extraction preparation

Manual testing steps

  1. npx eslint 'app/controllers/perps/**/*.ts' — 0 errors
  2. yarn lint:tsc 2>&1 | grep "app/controllers/perps/" — no output (clean)
  3. npx jest app/controllers/perps/ --no-coverage — 26 suites, 1273 tests pass
  4. ./scripts/perps/validate-core-sync.sh --core-path <core-path> — all 6 steps pass, 0 suppressions

Screenshots/Recordings

N/A — no UI changes, lint/style only.

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.

Note

Medium Risk
Broad refactor across perps controller and aggregation code (private fields, nullish coalescing, import/type changes) plus new lint enforcement; behavior should be equivalent but the volume and touchpoints make regressions possible.

Overview
Prepares app/controllers/perps/ for Core extraction by enforcing a Core-aligned ESLint override (including adding eslint-plugin-promise) and applying sweeping lint-driven refactors across the perps controller code.

Key code changes include migrating many private members to ES # fields, tightening types/return signatures and JSDoc, standardizing ??/Boolean(...) patterns, and updating provider reinitialization checks to use isCurrentlyReinitializing(); related tests were updated to avoid mutating internal flags directly. Also adds a scripts/perps/validate-core-sync.sh helper that rsyncs perps sources into a Core checkout and runs verification/build/lint steps to ensure a clean copy-over workflow.

Written by Cursor Bugbot for commit c803746. This will update automatically on new commits. Configure here.

Enforce Core's ESLint rules on perps source files so the codebase is
ready to move into @metamask/perps-controller without a wall of lint
errors at extraction time.

Key changes:
- Add perps-specific ESLint override matching @metamask/eslint-config
- Convert `in` operator usage to Object.hasOwn()
- Replace || with ?? where nullish coalescing is more correct
- Use ES private class fields (#field) instead of TS private keyword
- Enforce import ordering, explicit return types, JSDoc standards
- Test files excluded from strict enforcement via excludedFiles
@abretonc7s abretonc7s requested a review from a team as a code owner February 13, 2026 11:58
@github-actions

Copy link
Copy Markdown
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-perps Perps team label Feb 13, 2026
Comment thread app/controllers/perps/PerpsController.ts
The previous ESLint alignment commit (21b0d13) introduced 59 TS errors.
This fixes all of them by reverting changes that conflict with mobile's
tsconfig (lib: es2017) and established patterns:

- Revert Object.hasOwn() → 'in' operator (28 TS2550 + 5 TS2339 errors)
  Object.hasOwn() requires lib:es2022, and unlike `in`, doesn't provide
  TS type narrowing. Disabled the rule in .eslintrc.js for mobile.

- Revert !== null → != null for optional chaining (14 TS18048 errors)
  The != null pattern guards both null and undefined with ?. chains.
  Disabled no-eq-null in .eslintrc.js for mobile.

- Fix num → number variable references (3 TS2304 errors)
  ESLint id-denylist renamed vars but missed some references.

- Fix ?? → || for truthy checks on object refs (1 error)
- Fix isNaN() with possibly-undefined args (4 TS2345/TS18048 errors)
- Fix redundant condition in MYXClientService (1 TS2774 error)
- Fix test types: GetAvailableDexsParams, sl_price→slPrice, private
  field access (6 test TS errors)

Both deferred rules are tracked for enforcement in Core where lib will
be es2022+ and the patterns can be cleanly adopted.
Comment thread app/controllers/perps/PerpsController.ts
- Revert `current === null` back to `current == null` in
  setSelectedPaymentToken — the state field can be `undefined`
  (fresh state, migration), and `== null` correctly catches both
  null and undefined for the 'perps_balance' analytics fallback.

- Fix sl_price/tp_price → slPrice/tpPrice in PerpsController.test.ts
  and TradingService.test.ts — the method signature was renamed to
  camelCase but test assertions still used snake_case, silently passing
  due to `any` typing on the test helper.
…dow, promise rules)

- Enable `no-eq-null` in perps override and convert 17 `!= null` checks
  to explicit `=== null || === undefined` or `!== null && !== undefined`
- Add `@typescript-eslint/no-shadow` to perps override (with base
  `no-shadow` disabled)
- Install `eslint-plugin-promise` and enable 4 promise rules scoped to
  perps override only (`always-return`, `no-nesting`,
  `no-callback-in-promise`, `param-names`)
- Fix `import/order` grouping config to match Core's `import-x/order`
  (grouped `[builtin, external]` and `[internal, parent, sibling, index]`)
- Add explanatory comment block before perps override

Reduces Core's perps ESLint suppressions from ~50 to ~7. The only
remaining deferred rule is `BinaryExpression[operator='in']` (needs
lib:es2022 for Object.hasOwn).
…ing-core-steps

# Conflicts:
#	app/controllers/perps/providers/HyperLiquidProvider.ts
@socket-security

socket-security Bot commented Feb 14, 2026

Copy link
Copy Markdown

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Addednpm/​eslint-plugin-promise@​7.2.110010010082100

View full report

Comment thread .eslintrc.js
Replace indexed for-loop in removeCommas with str.replace() to satisfy
prefer-for-of, and replace || with ?? in subscription guard to satisfy
prefer-nullish-coalescing. Also add backup/restore for Core's
eslint-suppressions.json in the sync validation script.

@abretonc7s abretonc7s left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review: refactor(perps): align with @metamask/eslint-config for Core extraction

Overall: Well-structured PR with clear commit progression. The ESLint override approach (scoped to app/controllers/perps/) is the right strategy for pre-extraction alignment. Good documentation in the .eslintrc.js comments explaining Core differences.


Observations

1. setSelectedPaymentToken null handling — correctly addressed

File: PerpsController.ts:3432-3433

The code handles both cases correctly:

current === null ||
current === undefined ||

This is the proper expansion of == null for no-eq-null compliance.

2. sl_price/tp_priceslPrice/tpPrice rename is consistent

Internal interfaces use camelCase (slPrice/tpPrice), and DataLakeService.ts:156-157 correctly converts to snake_case (sl_price/tp_price) for the external API payload. All call sites match.

3. ES private fields (#field) migration looks correct

The private# conversion is comprehensive across PerpsController, TradingService, HyperLiquidProvider, etc. ES private fields are truly private at runtime (not just compile-time), which means tests can no longer access them via (instance as any).fieldName. The PR description acknowledges 85 pre-existing test failures from this pattern — that's expected and acceptable for a lint-alignment PR.

4. process.envglobalThis.process.env in isMYXProviderEnabled()

Good change for Core portability — globalThis.process.env works across Node.js and bundled environments.

5. New tooling: scripts/perps/validate-core-sync.sh

Well-written validation script with proper error handling (set -euo pipefail), fd 3 progress reporting, step timing, and suppression counting. Nice touch with the --skip-build flag for fast iteration.

6. orderCalculations.ts:448 — nested ?? precedence

grouping ?? ((takeProfitPrice ?? stopLossPrice) ? 'normalTpsl' : 'na');

The inner parens make the precedence explicit. Since all types are string | undefined, ?? is semantically equivalent to the original || here.


Minor Nits

  • TradingService.ts type parameter rename: <T><TResult> in withFeeDiscount — satisfies the typeParameter naming convention requiring 3+ chars.

  • Regex u flag additions (e.g., /[$,]/g/[$,]/gu): Correctly satisfies require-unicode-regexp. No behavioral change for these patterns.


Summary

Category Count
Issues to address 0
Observations (OK) 6
Nits 2

No issues found. The PR is solid.

@abretonc7s abretonc7s force-pushed the refactor/perps/remaining-core-steps branch from bbd6d6c to cc8287d Compare February 14, 2026 13:14
@abretonc7s abretonc7s added the DO-NOT-MERGE Pull requests that should not be merged label Feb 14, 2026
…ing-core-steps

# Conflicts:
#	app/controllers/perps/PerpsController.ts
#	app/controllers/perps/providers/HyperLiquidProvider.ts
#	app/controllers/perps/services/HyperLiquidSubscriptionService.ts
#	app/controllers/perps/utils/standaloneInfoClient.ts
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 85.66553% with 126 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.05%. Comparing base (5406044) to head (73450d5).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
...rollers/perps/services/HyperLiquidClientService.ts 72.15% 28 Missing and 16 partials ⚠️
app/controllers/perps/services/TradingService.ts 81.35% 12 Missing and 21 partials ⚠️
...rollers/perps/providers/AggregatedPerpsProvider.ts 76.38% 17 Missing ⚠️
app/controllers/perps/services/AccountService.ts 77.41% 0 Missing and 7 partials ⚠️
...llers/perps/aggregation/SubscriptionMultiplexer.ts 86.48% 4 Missing and 1 partial ⚠️
app/controllers/perps/utils/errorUtils.ts 0.00% 4 Missing ⚠️
app/controllers/perps/providers/MYXProvider.ts 94.00% 0 Missing and 3 partials ⚠️
app/controllers/perps/utils/myxAdapter.ts 60.00% 1 Missing and 1 partial ⚠️
app/controllers/perps/utils/sortMarkets.ts 84.61% 1 Missing and 1 partial ⚠️
app/controllers/perps/services/DataLakeService.ts 95.00% 1 Missing ⚠️
... and 8 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #26064      +/-   ##
==========================================
- Coverage   81.19%   81.05%   -0.15%     
==========================================
  Files        4396     4398       +2     
  Lines      113505   113872     +367     
  Branches    24373    24452      +79     
==========================================
+ Hits        92162    92295     +133     
- Misses      14976    15168     +192     
- Partials     6367     6409      +42     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

… rule

- Add missing AccountTreeControllerSelectedAccountGroupChangeEvent import
- Rename SCREAMING_SNAKE_CASE class properties to camelCase
  (#PRELOAD_WATCHED_PATHS, #PRELOAD_REFRESH_MS, #PRELOAD_GUARD_MS)
- Replace `void` fire-and-forget with `.catch()` (Core bans no-void)
- Expand @typescript-eslint/naming-convention from 2 to 10+ selectors
  to match Core's @metamask/eslint-config-typescript
- Add `no-void: error` to perps ESLint override
Rename _error to error in #withStreamPause catch blocks where the
variable is used by ensureError(). Replace 29 generic "@returns The
result of the operation." with specific descriptions matching each
method's actual return type.

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Comment thread app/controllers/perps/PerpsController.test.ts
…er pattern

The Record<string, never> type is a forward-compatibility placeholder
so the API signature stays stable when filters/pagination are added
later. Add a comment to prevent future confusion.
@abretonc7s abretonc7s removed the DO-NOT-MERGE Pull requests that should not be merged label Feb 16, 2026
@abretonc7s abretonc7s enabled auto-merge February 16, 2026 04:17
@github-actions

Copy link
Copy Markdown
Contributor

🔍 Smart E2E Test Selection

  • Selected E2E tags: SmokePerps, SmokeWalletPlatform, SmokeConfirmations
  • Selected Performance tags: @PerformancePreps
  • Risk Level: medium
  • AI Confidence: 85%
click to see 🤖 AI reasoning details

E2E Test Selection:
This PR is a comprehensive code style refactoring of the PerpsController and related perps infrastructure to align with MetaMask Core's ESLint rules. The changes include:

  1. ESLint Configuration: Added eslint-plugin-promise and extensive Core-alignment rules for perps files (import ordering, JSDoc requirements, naming conventions, private field syntax)

  2. Private Field Conversion: All private TypeScript keywords converted to ES private fields (#field syntax) across PerpsController, providers (HyperLiquid, MYX, Aggregated), and services

  3. Import Reorganization: Imports alphabetized and type imports separated using import type syntax

  4. JSDoc Additions: Added documentation comments to many methods

  5. No Functional Changes: The actual trading logic, state management, and public APIs remain unchanged

While this is primarily a refactoring with no intended functional changes, the PerpsController is a critical component for perpetuals trading functionality. The conversion of private fields and import reorganization could theoretically introduce subtle issues if any edge cases were missed.

Selected tags:

  • SmokePerps: Primary tag - directly tests the Perps functionality including Add Funds flow, balance verification, and trading operations that use PerpsController
  • SmokeWalletPlatform: Per tag description, Perps is a section inside the Trending tab, and changes to Perps views affect this tag
  • SmokeConfirmations: Per tag description, when selecting SmokePerps, also select SmokeConfirmations since Add Funds deposits are on-chain transactions

The risk is medium because while the changes are stylistic, they touch a critical trading controller with many private fields and methods being renamed.

Performance Test Selection:
The PerpsController changes, while primarily stylistic refactoring, could potentially impact performance due to the conversion of private fields to ES private class fields (#field syntax). ES private fields have slightly different runtime characteristics than TypeScript private keywords. Since this PR touches the core PerpsController and all its services/providers, running the @PerformancePreps tests will help verify that the refactoring hasn't introduced any performance regressions in the perps trading flows, market loading, position management, and add funds operations.

View GitHub Actions results

@sonarqubecloud

Copy link
Copy Markdown

@abretonc7s abretonc7s added this pull request to the merge queue Feb 16, 2026
Merged via the queue into main with commit 7783355 Feb 16, 2026
169 of 173 checks passed
@abretonc7s abretonc7s deleted the refactor/perps/remaining-core-steps branch February 16, 2026 07:47
@github-actions github-actions Bot locked and limited conversation to collaborators Feb 16, 2026
@metamaskbot metamaskbot added the release-7.67.0 Issue or pull request that will be included in release 7.67.0 label Feb 16, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

release-7.67.0 Issue or pull request that will be included in release 7.67.0 size-XL team-perps Perps team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants