feat: allow networkVersion to be set to null. fire connection events based on new isConnected property value

jest.config.js

@@ -45,10 +45,10 @@ const baseConfig = {
   // An object that configures minimum threshold enforcement for coverage results
   coverageThreshold: {
     global: {
-      branches: 66.79,
+      branches: 68.23,
       functions: 68.69,
-      lines: 68.35,
-      statements: 68.38,
+      lines: 68.68,
+      statements: 68.71,
     },
   },
 

src/BaseProvider.ts

@@ -238,6 +238,7 @@ export abstract class BaseProvider extends SafeEventEmitter {
    * @param initialState.chainId - The chain ID.
    * @param initialState.isUnlocked - Whether the user has unlocked MetaMask.
    * @param initialState.networkVersion - The network version.
+   * @param initialState.isConnected - Whether the network is available.
    * @fires BaseProvider#_initialized - If `initialState` is defined.
    * @fires BaseProvider#connect - If `initialState` is defined.
    */
@@ -246,17 +247,19 @@ export abstract class BaseProvider extends SafeEventEmitter {
     chainId: string;
     isUnlocked: boolean;
     networkVersion?: string;
+    isConnected?: boolean;
   }) {
     if (this._state.initialized) {
       throw new Error('Provider already initialized.');
     }
 
     if (initialState) {
-      const { accounts, chainId, isUnlocked, networkVersion } = initialState;
+      const { accounts, chainId, isUnlocked, networkVersion, isConnected } =
+        initialState;
 
       // EIP-1193 connect
-      this._handleConnect(chainId);
-      this._handleChainChanged({ chainId, networkVersion });
+      this._handleConnect({ chainId, isConnected });
+      this._handleChainChanged({ chainId, networkVersion, isConnected });
       this._handleUnlockStateChanged({ accounts, isUnlocked });
       this._handleAccountsChanged(accounts);
     }
@@ -311,11 +314,19 @@ export abstract class BaseProvider extends SafeEventEmitter {
    * When the provider becomes connected, updates internal state and emits
    * required events. Idempotent.
    *
-   * @param chainId - The ID of the newly connected chain.
+   * @param networkInfo - The options object.
+   * @param networkInfo.chainId - The ID of the newly connected chain.
+   * @param networkInfo.isConnected - Whether the network is available.
    * @fires MetaMaskInpageProvider#connect
    */
-  protected _handleConnect(chainId: string) {
-    if (!this._state.isConnected) {
+  protected _handleConnect({
+    chainId,
+    isConnected,
+  }: {
+    chainId: string;
+    isConnected?: boolean | undefined;
+  }) {
+    if (!this._state.isConnected && isConnected) {
       this._state.isConnected = true;
       this.emit('connect', { chainId });
       this._log.debug(messages.info.connected(chainId));
@@ -375,18 +386,24 @@ export abstract class BaseProvider extends SafeEventEmitter {
    * @fires BaseProvider#chainChanged
    * @param networkInfo - An object with network info.
    * @param networkInfo.chainId - The latest chain ID.
+   * @param networkInfo.isConnected - Whether the network is available.
    */
   protected _handleChainChanged({
     chainId,
+    isConnected,
   }:
-    | { chainId?: string | undefined; networkVersion?: string | undefined }
+    | {
+        chainId?: string;
+        networkVersion?: string | undefined;
+        isConnected?: boolean | undefined;
+      }
     | undefined = {}) {
     if (!isValidChainId(chainId)) {
       this._log.error(messages.errors.invalidNetworkParams(), { chainId });
       return;
     }
 
-    this._handleConnect(chainId);
+    this._handleConnect({ chainId, isConnected });
 
     if (chainId !== this.#chainId) {
       this.#chainId = chainId;

src/MetaMaskInpageProvider.test.ts

@@ -1,4 +1,5 @@
 import ObjectMultiplex from '@metamask/object-multiplex';
+import { JsonRpcError } from '@metamask/rpc-errors';
 import type { JsonRpcRequest } from '@metamask/utils';
 import { pipeline } from 'readable-stream';
 
@@ -51,6 +52,7 @@ async function getInitializedProvider({
     chainId = '0x0',
     isUnlocked = true,
     networkVersion = '0',
+    isConnected = true,
   } = {},
   onMethodCalled = [],
 }: {
@@ -81,6 +83,7 @@ async function getInitializedProvider({
             chainId,
             isUnlocked,
             networkVersion,
+            isConnected,
           },
         }),
       );
@@ -796,42 +799,119 @@ describe('MetaMaskInpageProvider: RPC', () => {
         connectionStream.notify(MetaMaskInpageProviderStreamName, {
           jsonrpc: '2.0',
           method: 'metamask_chainChanged',
-          // A "loading" networkVersion indicates the network is changing.
-          // Although the chainId is different, chainChanged should not be
-          // emitted in this case.
-          params: { chainId: '0x1', networkVersion: 'loading' },
+          params: { chainId: '0x1', networkVersion: '1', isConnected: false },
         });
       });
 
-      // Only once, for "disconnect".
-      expect(emitSpy).toHaveBeenCalledTimes(1);
+      expect(emitSpy).toHaveBeenCalledTimes(3);
+      expect(emitSpy).toHaveBeenCalledWith('chainChanged', '0x1');
+      expect(emitSpy).toHaveBeenCalledWith('networkChanged', '1');
+      expect(emitSpy).toHaveBeenCalledWith(
+        'disconnect',
+        new JsonRpcError(1013, messages.errors.disconnected()),
+      );
       emitSpy.mockClear(); // Clear the mock to avoid keeping a count.
 
       expect(provider.isConnected()).toBe(false);
-      // These should be unchanged.
+      expect(provider.chainId).toBe('0x1');
+      expect(provider.networkVersion).toBe('1');
+
+      await new Promise<void>((resolve) => {
+        provider.once('chainChanged', (newChainId) => {
+          expect(newChainId).toBe('0x2');
+          resolve();
+        });
+
+        connectionStream.notify(MetaMaskInpageProviderStreamName, {
+          jsonrpc: '2.0',
+          method: 'metamask_chainChanged',
+          params: { chainId: '0x2', networkVersion: '2', isConnected: false },
+        });
+      });
+
+      expect(emitSpy).toHaveBeenCalledTimes(2);
+      expect(emitSpy).toHaveBeenCalledWith('chainChanged', '0x2');
+      expect(emitSpy).toHaveBeenCalledWith('networkChanged', '2');
+      emitSpy.mockClear(); // Clear the mock to avoid keeping a count.
+
+      expect(provider.isConnected()).toBe(false);
+      expect(provider.chainId).toBe('0x2');
+      expect(provider.networkVersion).toBe('2');
+
+      await new Promise<void>((resolve) => {
+        provider.once('connect', (message) => {
+          expect(message).toStrictEqual({ chainId: '0x2' });
+          resolve();
+        });
+
+        connectionStream.notify(MetaMaskInpageProviderStreamName, {
+          jsonrpc: '2.0',
+          method: 'metamask_chainChanged',
+          params: { chainId: '0x2', networkVersion: '2', isConnected: true },
+        });
+      });
+
+      expect(emitSpy).toHaveBeenCalledTimes(1);
+      expect(emitSpy).toHaveBeenCalledWith('connect', { chainId: '0x2' });
+
+      expect(provider.isConnected()).toBe(true);
+      expect(provider.chainId).toBe('0x2');
+      expect(provider.networkVersion).toBe('2');
+    });
+
+    it('handles chain changes when networkVersion is "loading" by interpreting it as null', async () => {
+      const { provider, connectionStream } = await getInitializedProvider();
+
+      // We check this mostly for the readability of this test.
+      expect(provider.isConnected()).toBe(true);
       expect(provider.chainId).toBe('0x0');
       expect(provider.networkVersion).toBe('0');
 
+      const emitSpy = jest.spyOn(provider, 'emit');
+
       await new Promise<void>((resolve) => {
-        provider.once('chainChanged', (newChainId) => {
-          expect(newChainId).toBe('0x1');
+        provider.once('networkChanged', (newNetworkId) => {
+          expect(newNetworkId).toBeNull();
           resolve();
         });
 
         connectionStream.notify(MetaMaskInpageProviderStreamName, {
           jsonrpc: '2.0',
           method: 'metamask_chainChanged',
-          params: { chainId: '0x1', networkVersion: '1' },
+          params: {
+            chainId: '0x0',
+            networkVersion: 'loading',
+            isConnected: true,
+          },
         });
       });
 
-      expect(emitSpy).toHaveBeenCalledTimes(3);
-      expect(emitSpy).toHaveBeenNthCalledWith(1, 'connect', { chainId: '0x1' });
-      expect(emitSpy).toHaveBeenCalledWith('chainChanged', '0x1');
+      expect(emitSpy).toHaveBeenCalledTimes(1);
+      expect(emitSpy).toHaveBeenCalledWith('networkChanged', null);
+      emitSpy.mockClear(); // Clear the mock to avoid keeping a count.
+
+      expect(provider.isConnected()).toBe(true);
+      expect(provider.chainId).toBe('0x0');
+      expect(provider.networkVersion).toBeNull();
+
+      await new Promise<void>((resolve) => {
+        provider.once('networkChanged', (newNetworkId) => {
+          expect(newNetworkId).toBe('1');
+          resolve();
+        });
+
+        connectionStream.notify(MetaMaskInpageProviderStreamName, {
+          jsonrpc: '2.0',
+          method: 'metamask_chainChanged',
+          params: { chainId: '0x0', networkVersion: '1', isConnected: true },
+        });
+      });
+
+      expect(emitSpy).toHaveBeenCalledTimes(1);
       expect(emitSpy).toHaveBeenCalledWith('networkChanged', '1');
 
       expect(provider.isConnected()).toBe(true);
-      expect(provider.chainId).toBe('0x1');
+      expect(provider.chainId).toBe('0x0');
       expect(provider.networkVersion).toBe('1');
     });
   });
@@ -1036,6 +1116,7 @@ describe('MetaMaskInpageProvider: Miscellanea', () => {
             chainId: '0x0',
             isUnlocked: true,
             networkVersion: '0',
+            isConnected: true,
           };
         });
 

src/MetaMaskInpageProvider.ts

@@ -450,25 +450,41 @@ export class MetaMaskInpageProvider extends AbstractStreamProvider {
   }
 
   /**
-   * Upon receipt of a new chainId and networkVersion, emits corresponding
-   * events and sets relevant public state. Does nothing if neither the chainId
-   * nor the networkVersion are different from existing values.
+   * Upon receipt of a new chainId, networkVersion, and isConnected value
+   * emits corresponding events and sets relevant public state. We interpret
+   * a `networkVersion` with the value of `loading` to be null. The `isConnected`
+   * value determines if a `connect` or recoverable `disconnect` has occurred.
+   * Child classes that use the `networkVersion` for other purposes must implement
+   * additional handling therefore.
    *
    * @fires MetamaskInpageProvider#networkChanged
    * @param networkInfo - An object with network info.
    * @param networkInfo.chainId - The latest chain ID.
    * @param networkInfo.networkVersion - The latest network ID.
+   * @param networkInfo.isConnected - Whether the network is available.
    */
   protected _handleChainChanged({
     chainId,
     networkVersion,
-  }: { chainId?: string; networkVersion?: string } = {}) {
-    // This will validate the params and disconnect the provider if the
-    // networkVersion is 'loading'.
-    super._handleChainChanged({ chainId, networkVersion });
-
-    if (this._state.isConnected && networkVersion !== this.#networkVersion) {
-      this.#networkVersion = networkVersion as string;
+    isConnected,
+  }: {
+    chainId?: string;
+    networkVersion?: string;
+    isConnected?: boolean;
+  } = {}) {
+    super._handleChainChanged({ chainId, networkVersion, isConnected });
+
+    // The wallet will send a value of `loading` for `networkVersion` when it intends
+    // to communicate that this value cannot be resolved and should be intepreted as null.
+    // The wallet cannot directly send a null value for `networkVersion` because this
+    // would be a breaking change for existing dapps that use their own embedded MetaMask provider
+    // that expect this value to always be a integer string or the value 'loading'.
+
+    const targetNetworkVersion =
+      networkVersion === 'loading' ? null : networkVersion;
+
+    if (targetNetworkVersion !== this.#networkVersion) {
+      this.#networkVersion = targetNetworkVersion as string;
       if (this._state.initialized) {
         this.emit('networkChanged', this.#networkVersion);
       }