fix: Remove dynamic imports, move myx to optional dependencies#8374
Open
fix: Remove dynamic imports, move myx to optional dependencies#8374
Conversation
Member
Author
|
@metamaskbot publish-previews |
Contributor
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
Open
7 tasks
abretonc7s
added a commit
that referenced
this pull request
Apr 8, 2026
Companion to the synced src/ changes in the previous commit. These files live outside the rsync path and must be maintained manually on the core side. - Move @myx-trade/sdk from dependencies to optionalDependencies in packages/perps-controller/package.json so consumers (extension, mobile) do not install it automatically. Combined with the public barrel removal in the previous commit, this prevents @myx-trade/sdk from entering the consumer's static webpack/metro import graph. MYXProvider continues to load it via dynamic import() when MM_PERPS_MYX_PROVIDER_ENABLED=true. - Regenerate yarn.lock so the @metamask/perps-controller workspace entry declares dependenciesMeta["@myx-trade/sdk"].optional = true. - Document the Unreleased changes in CHANGELOG.md (Changed and BREAKING Removed sections covering the public export removal, the webpackIgnore magic comment, the MetaMetrics action allow-list entry, and the dependency relocation). This is the core equivalent of #8374, re-created on top of the latest mobile sync.
This was referenced Apr 8, 2026
github-merge-queue bot
pushed a commit
to MetaMask/metamask-mobile
that referenced
this pull request
Apr 8, 2026
## **Description** Makes `@metamask/perps-controller` safe to consume from the MetaMask extension without pulling `@myx-trade/sdk` into the static webpack bundle or violating the LavaMoat policy. Mobile owns the source-of-truth for the perps controller package via `scripts/perps/validate-core-sync.sh`, so the fix is made in mobile and then rsync'd to core on a matching branch. Companion core PR: MetaMask/core#8398 — supersedes MetaMask/core#8374. ### What changed 1. **`app/controllers/perps/PerpsController.ts`** — add `/* webpackIgnore: true */` magic comment to the `MYXProvider` dynamic `import()` so webpack (extension) skips static resolution of the intentionally-unshipped module. Metro (mobile) ignores the magic comment, so runtime behavior on mobile is unchanged. 2. **`app/controllers/perps/index.ts`** — remove 8 MYX adapter functions from the public barrel (`adaptMarketFromMYX`, `adaptPriceFromMYX`, `adaptMarketDataFromMYX`, `filterMYXExclusiveMarkets`, `isOverlappingMarket`, `buildPoolSymbolMap`, `buildSymbolPoolsMap`, `extractSymbolFromPoolId`). These are still used internally by `MYXProvider`, which imports them via relative path `../utils/myxAdapter`. No runtime change in mobile — the functions are still available to MYXProvider. 3. **`app/controllers/perps/utils/index.ts`** — drop the `export * from './myxAdapter'` barrel re-export so the utils index no longer transitively re-exports MYX symbols. 4. **Drift fix — `perpsConnectionAttemptContext`** — moved from `app/util/perpsConnectionAttemptContext.ts` into `app/controllers/perps/utils/perpsConnectionAttemptContext.ts` and exported from `@metamask/perps-controller` so: - `HyperLiquidClientService` (inside the perps package) imports it via a relative path that stays inside the synced directory — required for the core sync to build. - `PerpsConnectionManager` (outside the perps package) imports it from `@metamask/perps-controller` — satisfies the `no-restricted-imports` rule for code outside `app/controllers/perps/`. 5. **`.eslintrc.js` — align mobile perps override with core's base rules.** Added `BinaryExpression[operator='in']`, `WithStatement`, and `SequenceExpression` selectors to the existing `no-restricted-syntax` rule in the `app/controllers/perps/**` override. Mobile's override was missing these, which meant `'x' in y` type-guards passed mobile lint silently and then landed in core as new `no-restricted-syntax` suppressions at sync time. The override now mirrors what `@metamask/eslint-config` enforces in core, catching the violations at source. 6. **Replace `'x' in y` with `hasProperty(y, 'x')` across 8 perps-controller files** — `HyperLiquidProvider.ts`, `HyperLiquidSubscriptionService.ts`, `TradingService.ts`, `marketDataTransform.ts`, `hyperLiquidAdapter.ts`, `types/index.ts`, `types/transactionTypes.ts`, `utils/errorUtils.ts`. Uses `hasProperty` from `@metamask/utils` (the idiomatic replacement documented in core's eslint config). 24 of the 28 occurrences were converted cleanly; the remaining 4 are kept as `'in'` behind `/* eslint-disable no-restricted-syntax */` blocks because `hasProperty` narrows the KEY but not the discriminated-union branch — losing access to `.filled`, `.resting`, `.oid`, `.totalSz` on the HyperLiquid SDK types and losing `keyof typeof HYPERLIQUID_ASSET_CONFIGS` narrowing. Each disable comment is documented with the narrowing rationale. 7. **`scripts/perps/validate-core-sync.sh` — hardened preflight + per-file suppression delta check.** - **Preflight freshness check** (`chore(perps-sync): hard-fail sync script when core main has drift`): the previous preflight only compared the core working tree against `.sync-state.json.lastSyncedCoreCommit`, so it could not detect that someone else had merged a competing perps-controller change into `origin/main` while this branch was in review. That gap let [#8398](MetaMask/core#8398) land in a conflicting state with [#8333](MetaMask/core#8333). The new check at the top of `step_conflict_check` fetches `origin/main`, runs `git rev-list --count HEAD..origin/main -- packages/perps-controller/`, and **hard-fails** (not warns) with the offending commit list + a merge suggestion if any such commits exist. Gracefully degrades to `WARN` if offline or if `origin/main` is missing. - **Per-file/per-rule suppression delta check** (`chore(perps-sync): hard-fail on per-file suppression delta increase`): step 6 now snapshots the per-file/per-rule suppression counts from `eslint-suppressions.json` BEFORE running `--fix` / `--suppress-all` / `--prune-suppressions`, then diffs against the post-run counts and hard-fails if any (file, rule) pair's count INCREASED. Reducing counts (mobile fixes removing previously-suppressed violations) is always allowed. Increases mean the current sync is introducing NEW violations that would land in core as fresh suppressions and must be fixed at source. The script prints every offending file+rule pair and points at `hasProperty()` from `@metamask/utils` as the canonical replacement. Replaces the old "WARN if count > 20" heuristic. This is the canonical local detection point for the problem that *"it should have been detected locally!"* — a violation now fails the sync script at step 6 BEFORE the core PR is opened. ### Not included (intentional) An earlier draft of this PR also declared `MetaMetricsController:trackEvent` in `PerpsControllerAllowedActions` to let the extension drop a type cast. That change was **reverted** because mobile has no `MetaMetricsController` — it uses a `MetaMetrics` singleton (`app/core/Analytics/MetaMetrics.ts`) — so adding the action to the allowed-actions union forced `@metamask/messenger`'s parent type to narrow to `never` and broke `app/core/Engine/messengers/perps-controller-messenger/index.ts`. The extension keeps its existing `as unknown as PackagePerpsControllerMessenger` cast workaround until both host apps share a real `MetaMetricsController`. ## **Changelog** CHANGELOG entry: null ## **Related issues** Fixes: [TAT-2863](https://consensyssoftware.atlassian.net/browse/TAT-2863) Related: MetaMask/core#8374 (superseded by companion core PR #8398) ## **Manual testing steps** ```gherkin Feature: Perps controller behavior unchanged after MYX export cleanup Scenario: User opens the Perps tab on testnet Given the wallet is unlocked and funded on HyperLiquid testnet When user navigates to the Perps tab Then the markets list loads And the HyperLiquid provider is selected by default Scenario: User opens and closes a BTC long position Given the wallet has testnet funds When user opens a $11 BTC long market order Then a position appears in the positions list When user closes the position at 100% Then the position is removed from the positions list ``` Automated verification (all passing locally): - `yarn lint` on all touched files — clean - `NODE_OPTIONS='--max-old-space-size=8192' npx tsc --noEmit` — clean (full, no incremental cache) - `yarn jest app/controllers/perps/PerpsController.test.ts app/controllers/perps/services/HyperLiquidSubscriptionService.test.ts app/controllers/perps/services/TradingService.test.ts` — 412 passing - `bash scripts/perps/validate-core-sync.sh --core-path …/core` — all 9 steps PASS. Suppression count drops from 30 → **6** after the lint cleanup. ## **Screenshots/Recordings** N/A — this PR is a bundler / import-graph + lint cleanup with zero UI impact. Mobile runtime behavior is unchanged because: - The `webpackIgnore` magic comment is extension/webpack-only; Metro ignores it. - The 8 removed exports are still used internally by `MYXProvider` via relative import. - `perpsConnectionAttemptContext` keeps its module-level state, just at a new file path. - `hasProperty(obj, key)` is runtime-equivalent to `key in obj` for plain objects (both delegate to `Object.prototype.hasOwnProperty` semantics via the prototype chain). ## **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 (existing tests untouched; new location for moved test file is covered by the same suite) - [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. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Medium risk because it changes perps controller module boundaries/exports and touches trading/provider code paths (type-guard refactors and a few scoped lint suppressions), plus alters sync script behavior that can block releases if misconfigured. > > **Overview** > Improves extension-safe consumption of `@metamask/perps-controller` by ensuring the optional `MYXProvider` stays excluded from webpack’s static bundle and by removing MYX adapter re-exports from the public perps barrels. > > Aligns mobile’s perps linting with core by restricting the `in` operator (and a couple of other syntaxes) and refactors multiple perps files to use `hasProperty()` instead of `'key' in obj`, keeping a few documented `in` usages behind targeted `no-restricted-syntax` disables where TypeScript narrowing is required. > > Moves/exports `perpsConnectionAttemptContext` under the perps utils surface so in-package code imports remain sync-safe and out-of-package callers use the package export, and hardens `scripts/perps/validate-core-sync.sh` with an `origin/main` freshness check plus a per-file/per-rule eslint suppression delta hard-fail (including prettier formatting of `eslint-suppressions.json`). > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 1f90a9b. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
github-merge-queue bot
pushed a commit
that referenced
this pull request
Apr 8, 2026
…-do of #8374) (#8398) ## Explanation `@metamask/perps-controller` currently cannot be consumed from the MetaMask extension because: 1. **LavaMoat policy violation** — the public barrel re-exports 8 MYX adapter functions, which pulls `@myx-trade/sdk` into the static import graph even though `MYXProvider` is loaded via dynamic `import()`. 2. **Webpack static analysis** — webpack eagerly resolves the argument of `import('./providers/MYXProvider')` at build time, which breaks because `MYXProvider` is intentionally not published (`files: [..., "!dist/providers/MYXProvider*"]`). 3. **Eager install** — `@myx-trade/sdk` is in `dependencies`, so extension `yarn install` still pulls it even though it is unused outside the dynamic path. ### Solution This PR is the functional equivalent of #8374, re-created on top of the latest mobile→core sync. The mobile repo owns the source-of-truth for `packages/perps-controller/src/**/*.ts` via `scripts/perps/validate-core-sync.sh`, so the `src/` changes arrive here via the sync script run from mobile branch [`feat/tat-2863-sync-controller-code-extension`](MetaMask/metamask-mobile#28509). The package metadata (`package.json`, `CHANGELOG.md`, `yarn.lock`) has to be maintained manually on the core side because it lives outside the rsync path. ### Commit layout **Commit 1 (`feat(perps): sync controller code from mobile`)** — initial sync output. The three core fixes (`webpackIgnore` magic comment, MYX barrel removal, `perpsConnectionAttemptContext` relocation) plus accumulated mobile drift from the previous sync baseline (withdrawal-timestamp state fields, HyperLiquid service improvements, etc.). **Commit 2 (`feat(perps): move @myx-trade/sdk to optionalDependencies`)** — package metadata only: - Move `@myx-trade/sdk` from `dependencies` to `optionalDependencies` so extension/mobile consumers don't install it automatically. - Regenerate `yarn.lock` with `dependenciesMeta["@myx-trade/sdk"].optional = true` on the workspace entry. - Document the Unreleased changes in `CHANGELOG.md` (Changed + BREAKING Removed sections). **Commit 3 (`Merge origin/main into feat/perps/another-sync`)** — merge commit that: - Pulls in #8333 (competing perps sync) and the 5 releases that landed on `main` while this PR was in review. Conflict resolution: take mobile as source of truth on `src/` (re-ran the sync script after merge to guarantee `packages/perps-controller/src/` matches mobile `HEAD` exactly), merged CHANGELOG entries, kept `optionalDependencies` block. - Reverts a draft addition of `MetaMetricsController:trackEvent` to `PerpsControllerAllowedActions`. An earlier draft added this to let the extension drop a type cast, but mobile has no `MetaMetricsController` (uses a `MetaMetrics` singleton), so `@metamask/messenger`'s parent type narrowed to `never` and broke mobile's `perps-controller-messenger` setup. Extension keeps its existing cast workaround until both host apps share a real `MetaMetricsController`. **Commit 4 (`chore(perps-controller): fix CI — changelog order + prettier`)** — small CI follow-up. **Commit 5 (`chore(perps-controller): sync lint cleanup from mobile`)** — output of running the hardened mobile sync script after a corresponding mobile fix to align mobile's `.eslintrc.js` perps-controller override with core's `@metamask/eslint-config` base rules. Mobile changes that produced this commit: 1. Added `BinaryExpression[operator='in']` (+ `WithStatement` and `SequenceExpression`) to the mobile `.eslintrc.js` perps-controller override so mobile lint mirrors what core enforces. This closes the gap where `'x' in y` violations passed mobile lint silently and then landed in core as new `no-restricted-syntax` suppressions at sync time. 2. Replaced 24 `'x' in y` uses with `hasProperty(y, 'x')` from `@metamask/utils` across 8 source files. Four cases were kept as `'in'` behind `/* eslint-disable no-restricted-syntax */` because `hasProperty` narrows the KEY but not the discriminated-union branch — losing access to `.filled`, `.resting`, `.oid`, `.totalSz` on the HyperLiquid SDK types and losing `keyof typeof HYPERLIQUID_ASSET_CONFIGS` narrowing. 3. Hardened `scripts/perps/validate-core-sync.sh` step 6 with a per-file/per-rule suppression delta check that hard-fails the sync if any (file, rule) pair's suppression count INCREASED compared to the baseline. This is the canonical local detection point for "a new lint violation is about to land in core." **Net effect on this PR**: perps-controller suppression count drops from 30 → **6**. `eslint-suppressions.json` loses 45 entries. No runtime change. Splitting the commits this way makes it easy for a reviewer to see the "PR 8374 intent" (commits 1–2) isolated from the merge (commit 3), CI fix (commit 4), and the lint cleanup (commit 5). ### Verification - `yarn install` — clean (only the expected optional-dep warning for `@myx-trade/sdk`). - `yarn workspace @metamask/perps-controller build` — PASS. - `yarn workspace @metamask/perps-controller changelog:validate` — PASS. - Full mobile→core sync pipeline (preflight → conflict check → rsync → yarn install → verify fixes → eslint auto-fix + suppressions → build → lint → sync state): **9/9 PASS** after the merge + lint cleanup. Suppression count: **6** (down from 30 before commit 5). - Mobile-side typecheck (`tsc --noEmit` with no incremental cache): PASS — validates that the package type changes round-trip cleanly into mobile's `RootExtendedMessenger`. - Mobile jest (`PerpsController.test.ts`, `HyperLiquidSubscriptionService.test.ts`, `TradingService.test.ts`): **412 passing**, 4 skipped. ## References - Supersedes #8374 - Depends on MetaMask/metamask-mobile#28509 (source-of-truth mobile changes — includes the lint alignment + `hasProperty` migration that produced commit 5) - Extension consumer: MetaMask/metamask-extension#41337 (this PR unblocks the LavaMoat/webpack concerns; the `controllerMessenger as PackagePerpsControllerMessenger` cast remains until MetaMetricsController becomes a shared concept) ## Checklist - [x] I've updated the test suite for new or updated code as appropriate - [x] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [x] I've communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/processes/updating-changelogs.md) - [x] I've introduced [breaking changes](https://github.com/MetaMask/core/tree/main/docs/processes/breaking-changes.md) in this PR and have prepared draft pull requests for clients and consumer packages to resolve them <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Medium risk because it introduces a breaking change to public exports and adjusts dynamic import/bundling behavior, plus modifies WebSocket/REST candle subscription lifecycles which could affect chart/market data reliability under rapid navigation. > > **Overview** > Makes `@metamask/perps-controller` safer for extension consumers by moving `@myx-trade/sdk` to `optionalDependencies`, removing MYX adapter utilities from the public barrel (breaking), and adding `/* webpackIgnore: true */` to the dynamic `MYXProvider` import to avoid webpack static resolution. > > Hardens HyperLiquid connectivity and live-data flows: adds `perpsConnectionAttemptContext` to optionally suppress noisy startup errors, adds candle subscription debounce and `AbortController` cancellation for in-flight REST candle fetches, and prevents WS subscription leaks/races by tracking pending subscription promises. > > Includes a broad lint cleanup replacing many `'x' in y` checks with `hasProperty` (keeping a few `in` checks with explicit eslint disables for type narrowing), and updates the changelog, lockfile, and sync-state metadata accordingly. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 5ec006a. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Explanation
Fixes two related issues that caused webpack build failures and LavaMoat policy violations in the MetaMask extension when consuming @metamask/perps-controller@2.0.0: @myx-trade/sdk was being pulled into the static bundle, and webpack was failing to statically resolve MYXProvider.mjs which was intentionally excluded from the published dist/.
Root Cause
Two separate problems stemmed from the same underlying issue — MYX-related code was leaking into the static import graph even though MYX is not yet ready to ship.
Problem 1 — @myx-trade/sdk in the static bundle (LavaMoat violation)
The static import chain was:
Because myxAdapter.ts was re-exported from the package's public barrel (utils/index.ts → index.ts), webpack included @myx-trade/sdk in the bundle at build time. LavaMoat then flagged it because there was no policy entry for it.
Problem 2 — webpack static analysis failure on missing MYXProvider.mjs
dist/PerpsController.mjscontainedimport('./providers/MYXProvider.mjs')without awebpackIgnorehint.MYXProvider.mjswas intentionally excluded from the published dist (via package.json files exclusions), but webpack's static analysis tried to resolve it at build time before the runtime handleMYXImportError catch handler could do anything.Changes
src/utils/index.ts — Remove export * from './myxAdapter' from the utils barrel. This severs myxAdapter.ts from the static import graph entirely.
src/index.ts — Remove the eight myxAdapter functions (adaptMarketFromMYX, adaptPriceFromMYX, adaptMarketDataFromMYX, filterMYXExclusiveMarkets, isOverlappingMarket, buildPoolSymbolMap, buildSymbolPoolsMap, extractSymbolFromPoolId) from the package's public exports. A codebase-wide search confirmed zero external consumers of these functions — they are used only internally by MYXProvider, which continues to import them directly via relative path and remains fully functional inside the dynamic-import boundary.
src/PerpsController.ts — Add /* webpackIgnore: true */ to the MYXProvider dynamic import. This instructs webpack to skip static resolution entirely. The existing handleMYXImportError catch handler already gracefully swallows the module-not-found error at runtime.
package.json — Move @myx-trade/sdk from dependencies to optionalDependencies. This prevents the extension (and other consumers) from installing it automatically, while keeping it available for developers working on MYX locally.
After These Changes
@myx-trade/sdk is only reachable through the dynamic-import boundary (MYXProvider), so webpack never processes it at build time and LavaMoat never needs a policy entry for it.
Breaking Change
The eight MYX adapter functions listed above are no longer public exports of @metamask/perps-controller. Since no external consumers were found, the practical impact is zero.
References
Checklist
Note
Medium Risk
Medium risk because it introduces a breaking API surface change (removing MYX adapter utilities from public exports) and alters bundling behavior via dynamic-import hints and optional dependency resolution, which could impact downstream builds.
Overview
Prevents MYX-related code (and
@myx-trade/sdk) from being pulled into consumers’ static bundles by making@myx-trade/sdkanoptionalDependenciesentry and removingmyxAdapterre-exports from the public barrels (src/utils/index.tsandsrc/index.ts).Updates the MYX provider dynamic import in
PerpsControllerto include/* webpackIgnore: true */so webpack doesn’t try to statically resolve the intentionally-unshippedMYXProvidermodule; documents the BREAKING removal of the MYX adapter exports in the changelog and marks the dependency optional inyarn.lock.Written by Cursor Bugbot for commit 3fd5560. This will update automatically on new commits. Configure here.