refactor(perps): align with @metamask/eslint-config for Core extraction#26064
Conversation
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
|
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. |
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.
- 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
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
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.
There was a problem hiding this comment.
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_price → slPrice/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.env → globalThis.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.tstype parameter rename:<T>→<TResult>inwithFeeDiscount— satisfies thetypeParameternaming convention requiring 3+ chars. -
Regex
uflag additions (e.g.,/[$,]/g→/[$,]/gu): Correctly satisfiesrequire-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.
bbd6d6c to
cc8287d
Compare
…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 Report❌ Patch coverage is 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. 🚀 New features to boost your workflow:
|
… 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.
…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.
🔍 Smart E2E Test Selection
click to see 🤖 AI reasoning detailsE2E Test Selection:
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:
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: |
|



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-controllerpackage 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.typeoverinterface, 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— Addedeslint-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
npx eslint 'app/controllers/perps/**/*.ts'— 0 errorsyarn lint:tsc 2>&1 | grep "app/controllers/perps/"— no output (clean)npx jest app/controllers/perps/ --no-coverage— 26 suites, 1273 tests pass./scripts/perps/validate-core-sync.sh --core-path <core-path>— all 6 steps pass, 0 suppressionsScreenshots/Recordings
N/A — no UI changes, lint/style only.
Pre-merge author checklist
Pre-merge reviewer checklist
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 addingeslint-plugin-promise) and applying sweeping lint-driven refactors across the perps controller code.Key code changes include migrating many
privatemembers to ES#fields, tightening types/return signatures and JSDoc, standardizing??/Boolean(...)patterns, and updating provider reinitialization checks to useisCurrentlyReinitializing(); related tests were updated to avoid mutating internal flags directly. Also adds ascripts/perps/validate-core-sync.shhelper 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.