Don't throw error when `nodes` is undefined

nikita-fuchs commented 2 years ago

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 ?

davidyuk commented 2 years ago

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.

nikita-fuchs commented 1 year ago

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 ?

davidyuk commented 1 year ago

<!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.

aeternity / aepp-sdk-js

Don't throw error when `nodes` is undefined #1674