Code review fixes

LedgerHQ · Mar 15, 2023 · 389b08f · 389b08f
1 parent 92527ae
commit 389b08f
Show file tree

Hide file tree

Showing 5 changed files with 30 additions and 22 deletions.
diff --git a/packages/connect-kit/src/@types/index.d.ts b/packages/connect-kit/src/@types/index.d.ts
@@ -1,3 +1,10 @@
+// The WalletConnect packages are currently  not browser safe, there is an
+// open issue since 2020 here: WalletConnect/walletconnect-monorepo#341
+//
+// The solution is to consume the pre bundled packages and adding these
+// declarations. See imports on src/providers/WalletConnectEvm.ts and
+// src/providers/WalletConnectLegacy.ts.
+
 declare module '@walletconnect/ethereum-provider/dist/index.umd.js' {
   import WalletConnectProvider from '@walletconnect/ethereum-provider/dist/index.umd.js';
   export default WalletConnectProvider

diff --git a/packages/connect-kit/src/components/ExtensionInstallModal/ExtensionInstallModal.tsx b/packages/connect-kit/src/components/ExtensionInstallModal/ExtensionInstallModal.tsx
@@ -9,6 +9,7 @@ import { default as LightbulbSvg } from "../../assets/svg/Lightbulb.svg";
 import { default as CheckmarkSvg } from "../../assets/svg/Checkmark.svg";
 import NeedALedgerSection from "../NeedALedgerSection";
 import { getDebugLogger } from "../../lib/logger";
+import { useCallback } from "react";
 
 const log = getDebugLogger('ExtensionInstallModal')
 
@@ -19,12 +20,12 @@ const ExtensionInstallModal = ({
 }: ExtensionInstallModalProps) => {
   log('initializing');
 
-  const onJoinBetaClick = () => {
+  const onJoinBetaClick = useCallback(() => {
     window.open("https://get-connect.ledger.com/onboarding", "_blank");
-  };
+  }, []);
 
   return (
-    <Modal onClose={() => onClose()}>
+    <Modal onClose={onClose}>
       <>
         <ModalSection>
           <ModalTitle>Try Ledger Connect</ModalTitle>

diff --git a/packages/connect-kit/src/components/UseLedgerLiveModal/UseLedgerLiveModal.tsx b/packages/connect-kit/src/components/UseLedgerLiveModal/UseLedgerLiveModal.tsx
@@ -1,4 +1,4 @@
-import { useEffect, useRef, useState } from "react";
+import { useCallback, useEffect, useRef, useState } from "react";
 import { getDebugLogger } from "../../lib/logger";
 import Modal, { ModalProps, setIsModalOpen } from "../Modal/Modal";
 import {
@@ -48,23 +48,23 @@ const UseLedgerLiveModal = ({
     previousUriRef.current = uri;
   });
 
-  const onUseLedgerLiveClick = () => {
+  const onUseLedgerLiveClick = useCallback(() => {
     log('loading Ledger Live, ', wcUri);
     window.location.href = `ledgerlive://wc?uri=${encodeURIComponent(wcUri)}`;
 
     // close the modal so that the current WalletConnect URI cannot be reused
     setIsModalOpen(false);
 
     return false;
-  };
+  }, [wcUri]);
 
-  const onInstallLedgerLiveClick = () => {
+  const onInstallLedgerLiveClick = useCallback(() => {
     window.open('https://www.ledger.com/ledger-live');
     return false;
-  };
+  }, []);
 
   return (
-    <Modal onClose={() => onClose()}>
+    <Modal onClose={onClose}>
       <ModalSection textAlign="center">
         <ModalTitle>Do you have Ledger Live?</ModalTitle>
 

diff --git a/packages/connect-kit/src/providers/WalletConnectEvm.ts b/packages/connect-kit/src/providers/WalletConnectEvm.ts
@@ -94,19 +94,19 @@ function patchWalletConnectProviderRequest (provider: WalletConnectProvider) {
     if (method === 'eth_requestAccounts') {
       log('calling patched', method, params);
 
-      return new Promise(async (resolve, reject) => {
+      return new Promise((resolve, reject) => {
         try {
           if (!provider?.session?.connected) {
             showExtensionOrLLModal('', reject),
 
             // connect initializes the session and waits for connection
-            await provider.connect();
+            provider.connect().then(() => {
+              // call the original provider request
+              resolve(baseRequest({ method, params }));
+            });
           } else {
             log('reusing existing session');
           }
-
-          // call the original provider request
-          return resolve(await baseRequest({ method, params }));
         } catch(err) {
           logError('error', err);
           return reject(err);

diff --git a/packages/connect-kit/src/providers/WalletConnectLegacy.ts b/packages/connect-kit/src/providers/WalletConnectLegacy.ts
@@ -72,19 +72,19 @@ function patchWalletConnectLegacyProviderRequest (provider: WalletConnectProvide
     if (method === 'eth_requestAccounts') {
       log('calling patched', method, params);
 
-      return new Promise(async (resolve, reject) => {
+      return new Promise((resolve, reject) => {
         try {
           if (!provider.connected) {
-            await provider.connector.createSession({
+            provider.connector.createSession({
               chainId: supportOptions.chainId
-            });
+            }).then(() => {
+              // show the extension install modal or the WC URI
+              showExtensionOrLLModal(provider.connector.uri, reject);
 
-            // show the extension install modal or the WC URI
-            showExtensionOrLLModal(provider.connector.uri, reject);
+              // call the original provider request
+              resolve(baseRequest({ method, params }));
+            });
           }
-
-          // call the original provider request
-          return resolve(await baseRequest({ method, params }));
         } catch(err) {
           logError('error', err);
           return reject(err);