aepp-sdk-js icon indicating copy to clipboard operation
aepp-sdk-js copied to clipboard

Don't throw error when `nodes` is undefined

Open nikita-fuchs opened this issue 3 years ago • 3 comments

When initialising the SDK without nodes, one is greeted by this error:

https://github.com/aeternity/aepp-sdk-js/blob/a4df699f8e3d84ff1e729849cc90d7627b6362fd/src/AeSdkBase.ts#L159

In frontend context, as all data is fetched through the wallet, it's not necessary to have nodes defined. Wouldn't the correct way be to throw this error when something about the Node API is actually used? Or is this what actually happens, but the SDK initialisation just doesn't catch that error? Maybe the SDK could also skip using whatever node API in the initialisation when no nodes are present ?

nikita-fuchs avatar Sep 08 '22 12:09 nikita-fuchs

The idea is that sdk can be initialised with no accounts, nodes, compiler, but when one of SDK methods try evaluate one of these then exception would be thrown. So, you can use SDK with no nodes unless you call a methods that requires a node.

as all data is fetched through the wallet

It is not implemented yet, we have only connectNode that will ask wallet to share the node URL (probably it doesn't supported by sh wallet) https://docs.aeternity.com/aepp-sdk-js/v12.1.3/api/classes/AeSdkAepp.html#connectToWallet

Would be nice to have a reproduction if it is still an issue.

davidyuk avatar Sep 08 '22 19:09 davidyuk

It's easy to reproduce, I have the SH wallet installed and initialise the SDK like:

  async initSDK(onNetworkChange: any) {
    this.aeSdk = new AeSdkAepp({
      name: projectName,
/*       nodes: [
        {
          name: networkId,
          instance: new Node(nodeUrl),
        },
      ], */
      compilerUrl: nodeCompilerUrl,
      onAddressChange:  ({ current }) => console.log('new address'),
      // TODO: adjust view to display change
      onNetworkChange,
      onDisconnect: () => {
        return new Error('Disconnected');
      },
    });

    const walletNetworkId: string = await this.scanForWallet();
    return { walletNetworkId, aeSdk: this.aeSdk};
  }

and get the error, probably because I fetch the current chain hight etc, which is currently fetched through the node still:

https://github.com/aeternity/aepp-sdk-js/issues/1676

do you think you'll have time to fix the dependency on the node ?

nikita-fuchs avatar Nov 15 '22 10:11 nikita-fuchs

<!DOCTYPE html>
<html lang="en">
<head>
  <meta charset="UTF-8">
  <title>SDK test in browser</title>
</head>
<body>
Open developer console
<script src="../../dist/aepp-sdk.browser-script.js"></script>
<script type="text/javascript">
const { AeSdkAepp, BrowserWindowMessageConnection, walletDetector } = Aeternity;

const aeSdk = new AeSdkAepp({
  name: 'aepp example'
});

async function scanForWallets () {
  return new Promise((resolve) => {
    const handleWallets = async ({ wallets, newWallet }) => {
      newWallet = newWallet || Object.values(wallets)[0]
      if (confirm(`Do you want to connect to wallet ${newWallet.info.name} with id ${newWallet.info.id}`)) {
        console.log('newWallet', newWallet)
        stopScan()

        await aeSdk.connectToWallet(newWallet.getConnection(), { connectNode: true })
        resolve()
      }
    }

    const scannerConnection = new BrowserWindowMessageConnection()
    const stopScan = walletDetector(scannerConnection, handleWallets)
  })
}

(async () => {
  await scanForWallets();
  console.log('Height:', await aeSdk.getHeight());
})();
</script>
</body>
</html>

This is a working example, though it is not compatible with Superhero Wallet because it uses an outdated sdk. It works for me using wallet-web-extension from sdk repo.

davidyuk avatar Nov 20 '22 09:11 davidyuk