MetaMask / metamask-sdk

The simplest yet most secure way to connect your blockchain-based applications to millions of MetaMask Wallet users.
https://metamask.io/sdk/
Other
189 stars 117 forks source link

Global type is modified importing into node.js projects #491

Open BrettCleary opened 1 year ago

BrettCleary commented 1 year ago

In HyperPlay, which is an electron app, we import metamask sdk (v0.12.0) in the main (node.js) process. This imports the following from node_modules\@metamask\sdk\dist\node\es\src\index.d.ts

import { CommunicationLayerPreference, ConnectionStatus, DEFAULT_SERVER_URL, EventType, ServiceStatus, MessageType, PlatformType } from '@metamask/sdk-communication-layer';
import WebView from 'react-native-webview';
import { SDKProvider } from './provider/SDKProvider';
import { MetaMaskSDK, MetaMaskSDKOptions } from './sdk';
import { RPC_URLS_MAP } from './services/MetaMaskSDK/InitializerManager/setupReadOnlyRPCProviders';
import { PROVIDER_UPDATE_TYPE } from './types/ProviderUpdateType';
import type { SDKLoggingOptions } from './types/SDKLoggingOptions';
declare global {
    interface Window {
        ReactNativeWebView?: WebView;
        sdkProvider: SDKProvider;
        ethereum?: SDKProvider;
        extension: unknown;
        MSStream: unknown;
    }
}
export { CommunicationLayerPreference, ConnectionStatus, DEFAULT_SERVER_URL, EventType, MessageType, RPC_URLS_MAP, MetaMaskSDK, PROVIDER_UPDATE_TYPE, PlatformType, SDKProvider, };
export type { MetaMaskSDKOptions, SDKLoggingOptions, ServiceStatus };
export default MetaMaskSDK;
//# sourceMappingURL=index.d.ts.map

This type modifies the global type which conflicts with the frontend definition for window.ethereum of

declare global {
  interface Window {
    ethereum: {
      /*eslint-disable-next-line @typescript-eslint/no-explicit-any */
      request: (args: any) => any
      /*eslint-disable-next-line @typescript-eslint/no-explicit-any */
      send: (...args: any) => any
      /*eslint-disable-next-line @typescript-eslint/no-explicit-any */
      sendAsync: (...args: any) => any
      /*eslint-disable-next-line @typescript-eslint/no-explicit-any */
      on: (topic: string, handler: (...args: any) => void) => void
      isConnected: () => boolean
    }
  }
}

I don't see the purpose of modifying the global window.etheruem object for nodejs projects since the provider will be obtained by following the steps defined here https://docs.metamask.io/wallet/how-to/connect/set-up-sdk/javascript/nodejs/ and the sdk object will be called directly.

Would it be possible to remove the following from node_modules\@metamask\sdk\dist\node\es\src\index.d.ts?

declare global {
    interface Window {
        ReactNativeWebView?: WebView;
        sdkProvider: SDKProvider;
        ethereum?: SDKProvider;
        extension: unknown;
        MSStream: unknown;
    }
}
christopherferreira9 commented 2 weeks ago

Hi @BrettCleary ! Is this still relevant? We've made significant improvements to the SDK even though the suggestion you gave hasn't been addressed yet. Please do let us know so that we can include these changes in an upcoming release.