refactor: Lazy-load MWP packages + eciesjs/KeyManager behind dynamic import()#244
refactor: Lazy-load MWP packages + eciesjs/KeyManager behind dynamic import()#244ffmcgee725 wants to merge 4 commits intomainfrom
import()#244Conversation
| return this.#transport instanceof MWPTransport | ||
| ? TransportType.MWP | ||
| : TransportType.Browser; | ||
| return this.#transportType ?? TransportType.Browser; |
There was a problem hiding this comment.
could we just invert the condition to this.#transport instanceof DefaultTransport instead? I'm not too keen on having manage the this.#transportType value if we don't have to
There was a problem hiding this comment.
The thing here is #transportType is used in too many places where instanceof won't work
Examples of this would be
- Reading the stored transport type before any instance exists, the public
transportTypegetter that feeds intoRequestRouter/analytics, - the
#setupMWPearly return (which would force an eager import and break lazy-loading - the disconnect logic where the transport may already be partially torn down
This isn't just tracking what the transport is, it's tracking what it should be in states where the instance doesn't exist yet. The one spot where it is redundant is the #setupDefaultTransport guard, but we removed this in b645322
| * | ||
| * @returns A ready-to-use key manager instance. | ||
| */ | ||
| export async function createKeyManager(): Promise<IKeyManager> { |
There was a problem hiding this comment.
i like how you were able to encapsulate the dynamic importing complexity entirely into this file
| await Promise.all([ | ||
| import('@metamask/mobile-wallet-protocol-core'), | ||
| import('@metamask/mobile-wallet-protocol-dapp-client'), | ||
| import('./transports/mwp/KeyManager'), |
There was a problem hiding this comment.
does KeyManager need to be lazy loaded?
There was a problem hiding this comment.
according to ticket spec, yes, it's intentional
| @@ -584,7 +609,10 @@ export class MetaMaskConnectMultichain extends MultichainCore { | |||
| resolve(); | |||
| }) | |||
| .catch(async (error) => { | |||
| if (error instanceof ProtocolError) { | |||
| if ( | |||
| this.#mwpModules && | |||
| error instanceof this.#mwpModules.ProtocolError | |||
| ) { | |||
There was a problem hiding this comment.
thoughts on just lazy import mwp-core again in here to get access to the error classes rather than setting the error classes on the #mwpModules property on the MultichainClient instance?
| const { MWPTransport: MWPTransportClass } = await import( | ||
| './transports/mwp' | ||
| ); | ||
| const apiTransport = new MWPTransportClass(dappClient, kvstore); |
There was a problem hiding this comment.
| const { MWPTransport: MWPTransportClass } = await import( | |
| './transports/mwp' | |
| ); | |
| const apiTransport = new MWPTransportClass(dappClient, kvstore); | |
| const { MWPTransport } = await import( | |
| './transports/mwp' | |
| ); | |
| const apiTransport = new MWPTransport(dappClient, kvstore); |
| this.#transportType === TransportType.Browser && | ||
| this.#transport instanceof DefaultTransport |
2 participants
Explanation
Converts static imports of
@metamask/mobile-wallet-protocol-core,@metamask/mobile-wallet-protocol-dapp-client, andeciesjsto dynamicimport()calls so the entire MWP + crypto dependency tree is only loaded when MWP transport (QR code / deeplink pairing with MetaMask Mobile) is actually used. Desktop browser extension users never hit this code path and will no longer pay the load-time cost.Transitive dependencies now deferred
mwp-corecentrifuge→protobufjs(12+ sub-packages incl.@types/node),async-mutex,eventseciesjs@ecies/ciphers,@noble/ciphers(3 unique packages;@noble/curves/@noble/hashesshared with mwp-core)What changed
packages/connect-multichain/src/multichain/index.tsErrorCode,ProtocolError,SessionStore,WebSocketTransport(mwp-core),DappClient(mwp-dapp-client),MWPTransport(local transport), andkeymanager(KeyManager).SessionRequestandDappClient(erased at compile time).#createDappClient()now dynamically imports all three MWP packages in parallel viaPromise.all, then calls the newcreateKeyManager()async factory.#mwpModulesclass field — cachesProtocolErrorandErrorCodefrom mwp-core after the first lazy load soerror instanceof ProtocolErrorandErrorCode.REQUEST_EXPIREDchecks in catch blocks continue to work correctly.#transportTypeclass field — replaces allinstanceof MWPTransportchecks with a trackedTransportTypeenum, eliminating the need for a static import of theMWPTransportclass.packages/connect-multichain/src/multichain/transports/mwp/KeyManager.tseciesjsimport + exported singleton to an async factory function (createKeyManager()) that dynamically importseciesjsand returns anIKeyManagervia closure. This prevents esbuild from hoisting theeciesjsexternal import to the bundle's top level.packages/connect-multichain/src/multichain/transports/mwp/index.tsimport { SessionStore }from mwp-core to a dynamicimport()insidegetActiveSession(). This was the last remaining static import that esbuild was hoisting to the top level.Bundle size analysis
The package's own bundle increases ~5–7 KB due to esbuild's async module init wrappers (
__esmblocks). This is expected because MWP packages are currently bundled (not external) withsplitting: false.The key improvement is for downstream consumers: the ESM output now contains zero top-level static imports of mwp-core, mwp-dapp-client, or eciesjs. A consumer's bundler (webpack, Vite, etc.) can code-split the entire MWP + crypto tree into a lazy chunk that is never loaded for extension-only users.
Test plan
error instanceof ProtocolErrorchecks still work correctly via cached#mwpModulesReferences
Checklist