fix: make MultichainCore's dapp name and url properties mandatory (#56)

* mark dapp's name and url property as non-optional

* update changelog

* simplify getDappId to use url first then fallback to name

* Make dapp.url optional. Overwrite value if browser

* throw if dapp name missing

* fix type

* changelog

* changelog validation

* changelog format fix

---------

Co-authored-by: Jiexi Luan <jiexiluan@gmail.com>
Co-authored-by: Alex Donesky <adonesky@gmail.com>

Author: 3 people

packages/connect-evm/CHANGELOG.md

@@ -19,6 +19,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 
+- **BREAKING** Make the `dapp.name` and `dapp.url` properties required in `createMetamaskConnectEVM()` ([#56](https://github.com/MetaMask/connect-monorepo/pull/56))
+  - The `dapp.url` property is now always overwritten with the value of the page's url when MetaMask Connect is running in a browser context ([#56](https://github.com/MetaMask/connect-monorepo/pull/56))
 - `connect()` now always returns `chainId`, previously it could undefined ([#53](https://github.com/MetaMask/connect-monorepo/pull/53))
 
 ### Fixed

packages/connect-multichain/CHANGELOG.md

@@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Changed
+
+- **BREAKING** Make the `dapp.name` and `dapp.url` properties required in `createMetamaskConnect()` ([#56](https://github.com/MetaMask/connect-monorepo/pull/56))
+  - The `dapp.url` property is now always overwritten with the value of the page's url when MetaMask Connect is running in a browser context ([#56](https://github.com/MetaMask/connect-monorepo/pull/56))
+
 ### Fixed
 
 - Fixed mobile deeplink bug that occurred when `MultichainSDK.connect()` was called and the transport was already connected ([#57](https://github.com/MetaMask/connect-monorepo/pull/57))

packages/connect-multichain/src/domain/multichain/types.ts

@@ -22,7 +22,7 @@ export type { SessionData } from '@metamask/multichain-api-client';
  * - Using a base64-encoded icon
  */
 export type DappSettings = {
-  name?: string;
+  name: string;
   url?: string;
 } & ({ iconUrl?: string } | { base64Icon?: string });
 

packages/connect-multichain/src/multichain/utils/index.test.ts

@@ -31,41 +31,21 @@ t.describe('Utils', () => {
 	});
 
 	t.describe('getDappId', () => {
-		const mockHostname = 'mockdapp.com';
 		const mockDappName = 'Mock DApp Name';
 		const mockDappUrl = 'http://mockdapp.com';
-		const originalWindow = global.window;
 
-		t.afterEach(() => {
-			global.window = originalWindow;
-		});
-
-		t.it('should return window.location.hostname if window and window.location are defined', () => {
-			global.window = {
-				location: {
-					hostname: mockHostname,
-				},
-			} as any;
-			t.expect(utils.getDappId()).toBe(mockHostname);
-		});
-
-		t.it('should return dappMetadata.name if window is undefined and name is available', () => {
+		t.it('should return dappMetadata.name if defined and url is not', () => {
 			global.window = undefined as any;
-			const dappSettings = { name: mockDappName, url: mockDappUrl };
+			const dappSettings = { name: mockDappName };
 			t.expect(utils.getDappId(dappSettings)).toBe(mockDappName);
 		});
 
-		t.it('should return dappMetadata.url if window is undefined and name is not available but url is', () => {
+		t.it('should return dappMetadata.url if defined', () => {
 			global.window = undefined as any;
-			const dappSettings = { url: mockDappUrl };
+			const dappSettings = { url: mockDappUrl, name: mockDappName };
 			t.expect(utils.getDappId(dappSettings)).toBe(mockDappUrl);
 		});
 
-		t.it('should return "N/A" if window is undefined and neither name nor url is available', () => {
-			global.window = undefined as any;
-			const dappSettings = {};
-			t.expect(utils.getDappId(dappSettings)).toBe('N/A');
-		});
 	});
 
 	t.describe('getSDKVersion', () => {
@@ -161,7 +141,7 @@ t.describe('Utils', () => {
 			t.expect(consoleWarnSpy).toHaveBeenCalledWith('Invalid dappMetadata.iconUrl: URL must start with http:// or https://');
 		});
 
-		t.it('should set url to undenied if it does not start with http:// or https:// and favicon is undefined', async () => {
+		t.it('should set url to undefined if it does not start with http:// or https:// and favicon is undefined', async () => {
 			options.dapp.url = 'wrong';
 			const consoleWarnSpy = t.vi.spyOn(console, 'warn');
 
@@ -171,12 +151,17 @@ t.describe('Utils', () => {
 			t.expect(consoleWarnSpy).toHaveBeenCalledWith('Invalid dappMetadata.url: URL must start with http:// or https://');
 		});
 
-		t.it('should prove that dapp is mandatory is platform is not browser', async () => {
-			(options.dapp as any) = undefined;
+		t.it('throw if platform is not browser and dapp url is missing', async () => {
+			(options.dapp as any) = { name: 'test' };
 			await t.expect(() => utils.setupDappMetadata(options)).toThrow('You must provide dapp url');
 		});
 
-		t.it('should prove that dapp is optional is platform is browser', async () => {
+		t.it('throw if platform dapp name is missing', async () => {
+			(options.dapp as any) = { url: 'https://example.com' };
+			await t.expect(() => utils.setupDappMetadata(options)).toThrow('You must provide dapp name');
+		});
+
+		t.it('should set the dapp url if not provided and platform is browser', async () => {
 			const mockGetPlatformType = t.vi.mocked(getPlatformType);
 			mockGetPlatformType.mockReturnValue(PlatformType.DesktopWeb);
 			t.vi.stubGlobal('window', {
@@ -185,7 +170,9 @@ t.describe('Utils', () => {
 					host: 'example.com',
 				},
 			});
-			(options.dapp as any) = undefined;
+			(options.dapp as any) = {
+				name: 'test',
+			};
 			utils.setupDappMetadata(options);
 			t.expect(options.dapp.url).toBe('https://example.com');
 		});
@@ -195,6 +182,7 @@ t.describe('Utils', () => {
 			const consoleWarnSpy = t.vi.spyOn(console, 'warn');
 
 			options.dapp = {
+				name: 'test',
 				iconUrl: 'https://example.com/favicon.ico',
 				url: 'https://example.com',
 				base64Icon: longString,
@@ -208,6 +196,7 @@ t.describe('Utils', () => {
 
 		t.it('should set iconUrl to the extracted favicon if iconUrl and base64Icon are not provided', async () => {
 			options.dapp = {
+				name: 'test',
 				url: 'https://example.com',
 			};
 

packages/connect-multichain/src/multichain/utils/index.ts

@@ -43,12 +43,8 @@ export function compressString(str: string): string {
   return base64Encode(binaryString);
 }
 
-export function getDappId(dapp?: DappSettings) {
-  if (typeof window === 'undefined' || typeof window.location === 'undefined') {
-    return dapp?.name ?? dapp?.url ?? 'N/A';
-  }
-
-  return window.location.hostname;
+export function getDappId(dapp: DappSettings) {
+  return dapp.url ?? dapp.name;
 }
 
 export function openDeeplink(
@@ -126,16 +122,17 @@ export function setupDappMetadata(
     platform === PlatformType.MobileWeb ||
     platform === PlatformType.MetaMaskMobileWebview;
 
+  if (!options.dapp?.name) {
+    throw new Error('You must provide dapp name');
+  }
+  if (isBrowser) {
+    options.dapp = {
+      ...options.dapp,
+      url: `${window.location.protocol}//${window.location.host}`,
+    };
+  }
   if (!options.dapp?.url) {
-    // Automatically set dappMetadata on web env if not defined
-    if (isBrowser) {
-      options.dapp = {
-        ...options.dapp,
-        url: `${window.location.protocol}//${window.location.host}`,
-      };
-    } else {
-      throw new Error('You must provide dapp url');
-    }
+    throw new Error('You must provide dapp url');
   }
   const BASE_64_ICON_MAX_LENGTH = 163400;
   // Check if iconUrl and url are valid