Make chainId, networkVersion, selectedAddress readonly and log …

…deprecation warning (#280)

* Make chainId, networkVersion, selectedAddress readonly and log warning

* Fix Proxy getter

* Loosen jest coverage

* Add link to deprecation messages

jest.config.js

@@ -45,10 +45,10 @@ const baseConfig = {
   // An object that configures minimum threshold enforcement for coverage results
   coverageThreshold: {
     global: {
-      branches: 61.29,
-      functions: 60.24,
-      lines: 63.82,
-      statements: 63.97,
+      branches: 61.9,
+      functions: 61.79,
+      lines: 65.18,
+      statements: 65.3,
     },
   },
 

src/BaseProvider.ts

@@ -89,14 +89,14 @@ export abstract class BaseProvider extends SafeEventEmitter {
    * The chain ID of the currently connected Ethereum chain.
    * See [chainId.network]{@link https://chainid.network} for more information.
    */
-  public chainId: string | null;
+  #chainId: string | null;
 
   /**
    * The user's currently selected Ethereum address.
    * If null, MetaMask is either locked or the user has not permitted any
    * addresses to be viewed.
    */
-  public selectedAddress: string | null;
+  #selectedAddress: string | null;
 
   /**
    * Create a new instance of the provider.
@@ -124,8 +124,8 @@ export abstract class BaseProvider extends SafeEventEmitter {
     };
 
     // Public state
-    this.selectedAddress = null;
-    this.chainId = null;
+    this.#selectedAddress = null;
+    this.#chainId = null;
 
     // Bind functions to prevent consumers from making unbound calls
     this._handleAccountsChanged = this._handleAccountsChanged.bind(this);
@@ -145,6 +145,18 @@ export abstract class BaseProvider extends SafeEventEmitter {
     this._rpcEngine = rpcEngine;
   }
 
+  //====================
+  // Public Properties
+  //====================
+
+  get chainId(): string | null {
+    return this.#chainId;
+  }
+
+  get selectedAddress(): string | null {
+    return this.#selectedAddress;
+  }
+
   //====================
   // Public Methods
   //====================
@@ -337,9 +349,9 @@ export abstract class BaseProvider extends SafeEventEmitter {
           errorMessage ?? messages.errors.permanentlyDisconnected(),
         );
         this._log.error(error);
-        this.chainId = null;
+        this.#chainId = null;
         this._state.accounts = null;
-        this.selectedAddress = null;
+        this.#selectedAddress = null;
         this._state.isUnlocked = false;
         this._state.isPermanentlyDisconnected = true;
       }
@@ -372,10 +384,10 @@ export abstract class BaseProvider extends SafeEventEmitter {
 
     this._handleConnect(chainId);
 
-    if (chainId !== this.chainId) {
-      this.chainId = chainId;
+    if (chainId !== this.#chainId) {
+      this.#chainId = chainId;
       if (this._state.initialized) {
-        this.emit('chainChanged', this.chainId);
+        this.emit('chainChanged', this.#chainId);
       }
     }
   }
@@ -428,8 +440,8 @@ export abstract class BaseProvider extends SafeEventEmitter {
       this._state.accounts = _accounts as string[];
 
       // handle selectedAddress
-      if (this.selectedAddress !== _accounts[0]) {
-        this.selectedAddress = (_accounts[0] as string) || null;
+      if (this.#selectedAddress !== _accounts[0]) {
+        this.#selectedAddress = (_accounts[0] as string) || null;
       }
 
       // finally, after all state has been updated, emit the event

src/MetaMaskInpageProvider.test.ts

@@ -1070,4 +1070,97 @@ describe('MetaMaskInpageProvider: Miscellanea', () => {
       expect(provider.isMetaMask).toBe(true);
     });
   });
+
+  describe('chainId', () => {
+    let provider: any | MetaMaskInpageProvider;
+
+    beforeEach(async () => {
+      provider = (
+        await getInitializedProvider({
+          initialState: {
+            chainId: '0x5',
+          },
+        })
+      ).provider;
+    });
+
+    it('should warn the first time chainId is accessed', async () => {
+      const consoleWarnSpy = jest.spyOn(globalThis.console, 'warn');
+
+      expect(provider.chainId).toBe('0x5');
+      expect(consoleWarnSpy).toHaveBeenCalledWith(
+        messages.warnings.chainIdDeprecation,
+      );
+      expect(consoleWarnSpy).toHaveBeenCalledTimes(1);
+    });
+
+    it('should not allow chainId to be modified', () => {
+      expect(() => (provider.chainId = '0x539')).toThrow(
+        'Cannot set property chainId',
+      );
+      expect(provider.chainId).toBe('0x5');
+    });
+  });
+
+  describe('networkVersion', () => {
+    let provider: any | MetaMaskInpageProvider;
+
+    beforeEach(async () => {
+      provider = (
+        await getInitializedProvider({
+          initialState: {
+            networkVersion: '5',
+          },
+        })
+      ).provider;
+    });
+
+    it('should warn the first time networkVersion is accessed', async () => {
+      const consoleWarnSpy = jest.spyOn(globalThis.console, 'warn');
+
+      expect(provider.networkVersion).toBe('5');
+      expect(consoleWarnSpy).toHaveBeenCalledWith(
+        messages.warnings.networkVersionDeprecation,
+      );
+      expect(consoleWarnSpy).toHaveBeenCalledTimes(1);
+    });
+
+    it('should not allow networkVersion to be modified', () => {
+      expect(() => (provider.networkVersion = '1337')).toThrow(
+        'Cannot set property networkVersion',
+      );
+      expect(provider.networkVersion).toBe('5');
+    });
+  });
+
+  describe('selectedAddress', () => {
+    let provider: any | MetaMaskInpageProvider;
+
+    beforeEach(async () => {
+      provider = (
+        await getInitializedProvider({
+          initialState: {
+            accounts: ['0xdeadbeef'],
+          },
+        })
+      ).provider;
+    });
+
+    it('should warn the first time selectedAddress is accessed', async () => {
+      const consoleWarnSpy = jest.spyOn(globalThis.console, 'warn');
+
+      expect(provider.selectedAddress).toBe('0xdeadbeef');
+      expect(consoleWarnSpy).toHaveBeenCalledWith(
+        messages.warnings.selectedAddressDeprecation,
+      );
+      expect(consoleWarnSpy).toHaveBeenCalledTimes(1);
+    });
+
+    it('should not allow selectedAddress to be modified', () => {
+      expect(() => (provider.selectedAddress = '0x12345678')).toThrow(
+        'Cannot set property selectedAddress',
+      );
+      expect(provider.selectedAddress).toBe('0xdeadbeef');
+    });
+  });
 });

src/MetaMaskInpageProvider.ts

@@ -36,6 +36,10 @@ export type MetaMaskInpageProviderOptions = {
 } & Partial<Omit<StreamProviderOptions, 'rpcMiddleware'>>;
 
 type SentWarningsState = {
+  // properties
+  chainId: boolean;
+  networkVersion: boolean;
+  selectedAddress: boolean;
   // methods
   enable: boolean;
   experimentalMethods: boolean;
@@ -56,6 +60,10 @@ export const MetaMaskInpageProviderStreamName = 'metamask-provider';
 
 export class MetaMaskInpageProvider extends AbstractStreamProvider {
   protected _sentWarnings: SentWarningsState = {
+    // properties
+    chainId: false,
+    networkVersion: false,
+    selectedAddress: false,
     // methods
     enable: false,
     experimentalMethods: false,
@@ -76,7 +84,7 @@ export class MetaMaskInpageProvider extends AbstractStreamProvider {
     MetaMaskInpageProvider['_getExperimentalApi']
   >;
 
-  public networkVersion: string | null;
+  #networkVersion: string | null;
 
   /**
    * Indicating that this provider is a MetaMask provider.
@@ -118,7 +126,7 @@ export class MetaMaskInpageProvider extends AbstractStreamProvider {
     // eslint-disable-next-line @typescript-eslint/no-floating-promises
     this._initializeStateAsync();
 
-    this.networkVersion = null;
+    this.#networkVersion = null;
     this.isMetaMask = true;
 
     this._sendSync = this._sendSync.bind(this);
@@ -160,6 +168,34 @@ export class MetaMaskInpageProvider extends AbstractStreamProvider {
     }
   }
 
+  //====================
+  // Deprecated Properties
+  //====================
+
+  get chainId(): string | null {
+    if (!this._sentWarnings.chainId) {
+      this._log.warn(messages.warnings.chainIdDeprecation);
+      this._sentWarnings.chainId = true;
+    }
+    return super.chainId;
+  }
+
+  get networkVersion(): string | null {
+    if (!this._sentWarnings.networkVersion) {
+      this._log.warn(messages.warnings.networkVersionDeprecation);
+      this._sentWarnings.networkVersion = true;
+    }
+    return this.#networkVersion;
+  }
+
+  get selectedAddress(): string | null {
+    if (!this._sentWarnings.selectedAddress) {
+      this._log.warn(messages.warnings.selectedAddressDeprecation);
+      this._sentWarnings.selectedAddress = true;
+    }
+    return super.selectedAddress;
+  }
+
   //====================
   // Public Methods
   //====================
@@ -228,8 +264,8 @@ export class MetaMaskInpageProvider extends AbstractStreamProvider {
    */
   protected _handleDisconnect(isRecoverable: boolean, errorMessage?: string) {
     super._handleDisconnect(isRecoverable, errorMessage);
-    if (this.networkVersion && !isRecoverable) {
-      this.networkVersion = null;
+    if (this.#networkVersion && !isRecoverable) {
+      this.#networkVersion = null;
     }
   }
 
@@ -369,7 +405,7 @@ export class MetaMaskInpageProvider extends AbstractStreamProvider {
         break;
 
       case 'net_version':
-        result = this.networkVersion ?? null;
+        result = this.#networkVersion ?? null;
         break;
 
       default:
@@ -457,10 +493,10 @@ export class MetaMaskInpageProvider extends AbstractStreamProvider {
     // networkVersion is 'loading'.
     super._handleChainChanged({ chainId, networkVersion });
 
-    if (this._state.isConnected && networkVersion !== this.networkVersion) {
-      this.networkVersion = networkVersion as string;
+    if (this._state.isConnected && networkVersion !== this.#networkVersion) {
+      this.#networkVersion = networkVersion as string;
       if (this._state.initialized) {
-        this.emit('networkChanged', this.networkVersion);
+        this.emit('networkChanged', this.#networkVersion);
       }
     }
   }

src/initializeInpageProvider.ts

@@ -55,6 +55,11 @@ export function initializeProvider({
   const proxiedProvider = new Proxy(provider, {
     // some common libraries, e.g. web3@1.x, mess with our API
     deleteProperty: () => true,
+    // fix issue with Proxy unable to access private variables from getters
+    // https://stackoverflow.com/a/73051482
+    get(target, propName: 'chainId' | 'networkVersion' | 'selectedAddress') {
+      return target[propName];
+    },
   });
 
   if (shouldSetOnWindow) {

src/messages.ts

@@ -24,6 +24,10 @@ const messages = {
       `MetaMask: Connected to chain with ID "${chainId}".`,
   },
   warnings: {
+    // deprecated properties
+    chainIdDeprecation: `MetaMask: 'ethereum.chainId' is deprecated and may be removed in the future. Please use the 'eth_chainId' RPC method instead.\nFor more information, see: https://github.com/MetaMask/metamask-improvement-proposals/discussions/23`,
+    networkVersionDeprecation: `MetaMask: 'ethereum.networkVersion' is deprecated and may be removed in the future. Please use the 'net_version' RPC method instead.\nFor more information, see: https://github.com/MetaMask/metamask-improvement-proposals/discussions/23`,
+    selectedAddressDeprecation: `MetaMask: 'ethereum.selectedAddress' is deprecated and may be removed in the future. Please use the 'eth_accounts' RPC method instead.\nFor more information, see: https://github.com/MetaMask/metamask-improvement-proposals/discussions/23`,
     // deprecated methods
     enableDeprecation: `MetaMask: 'ethereum.enable()' is deprecated and may be removed in the future. Please use the 'eth_requestAccounts' RPC method instead.\nFor more information, see: https://eips.ethereum.org/EIPS/eip-1102`,
     sendDeprecation: `MetaMask: 'ethereum.send(...)' is deprecated and may be removed in the future. Please use 'ethereum.sendAsync(...)' or 'ethereum.request(...)' instead.\nFor more information, see: https://eips.ethereum.org/EIPS/eip-1193`,