Avoid double-initialization on page reload

[kronosapiens]

I'm hearing from @rsodre that the Connector is being duplicated several times due to react hooks being called somewhat randomly. This duplication of the Connector is creating issues.

We should check to see if starknet_controller already exists in local storage, and no-op if so.

[kronosapiens]

@broody any thoughts on this? @rsodre anything else to add?

[broody]

A current limitation is ControllerConnector should be created outside of a react component/hook to avoid being reinited. Since we create a message channel when it gets instantiated, reiniting it would cause issues.

@rsodre if you could link to ur code we can take a look.

[rsodre]

I understand that reinitiating it can cause issues, but breaking the controller if that happens is bad.

It would be a much better experience if subsequent inites did nothing to the controller, and print a warning in the console, just telling the user that it should be created only once, to make sure all options are defined once it is created.

And in the docs, instead of suggesting not to use it in a hook (btw, there are safe ways to do it in hooks), say that the first call is definitive, make sure all options are correct to call it once.

What is happening right now is that the starknetkit example in their docs does not require a hook, since it uses injected wallets. Adding the controller to that connector array is breaking starknetkit.

https://www.starknetkit.com/docs/latest/getting-started/usage#defining-chains-providers-and-connectors

// this is safe to use inside a component
// but if we add the controller starknetkit stops working
const connectors = [
  new InjectedConnector({
    options: { id: "argentX", name: "Ready Wallet (formerly Argent)" },
  }),
  new InjectedConnector({
    options: { id: "braavos", name: "Braavos" },
  })
]

return (
  <StarknetConfig
    chains={[mainnet, sepolia]}
    provider={publicProvider()}
    connectors={connectors as Connector[]}
    explorer={voyager}
  >
    {children}
  </StarknetConfig>
)

[rsodre]

Also, the Controller docs in starknetkit do not provide a React example.
If it requires special treatment, it should be noted here.
https://www.starknetkit.com/docs/latest/connectors/controller

[kronosapiens]

Interesting. Does passing the connectors into the StarknetConfig component result in re-initialization? Or only if it were initialized inside of a component definition itself?

A partial solution which is minimally invasive would be to log a warning here if the starknet_controller attribute was set, directing the user to the docs.

Another possible solution would be to no-op the initialization if starknet_controller was set, although that might create unintended side-effects and likely warrants some more discussion.

I can reach out to Meck about updating the Starknetkit docs, they maintain them separately from the Starknetkit repo itself.

[rsodre]

Does passing the connectors into the StarknetConfig component result in re-initialization? Or only if it were initialized inside of a component definition itself?

If done like that, every time the page renders the array of connectors will be rebuilt. The proper way to do this in React is with a useMemo(). But it's hard to avoid it being called multiple times.

As I understand, the InjectedConnector is just a wrapper, so there's no problem calling it multiple times, as the browser extension injected the connector. You could do something similar.

Or, instead of calling new ControllerConnector(), calling a function that checks if the controller connector was is created, returning it, or creates a new connector.

this is what I'm doing in the Starknet provider...

  const controller: Connector = useMemo(() => {
    return makeControllerConnector(
      networkConfig.starknet.chainId,
      networkConfig.starknet.rpcUrl,
      networkConfig.starknet.slotName,
      {}
    )
  }, [networkConfig]);

  const connectors: Connector[] = [
    controller,
    new InjectedConnector({ options: { id: 'braavos', name: 'Braavos' } }),
    new InjectedConnector({ options: { id: 'argentX', name: 'Ready (formerly Argent)' } }),
  ];

import { ControllerConnector } from '@cartridge/connector'
import type { ControllerOptions, Tokens } from '@cartridge/controller'
import { bigintToHex } from '@/utils/misc/types'
import { stringToFelt } from '@/utils/misc/starknet'

let _controllerConnector: ControllerConnector | undefined;

export const makeControllerConnector = (
  chainId: string,
  rpcUrl: string,
  slot: string,
  tokens: Tokens | any,
): ControllerConnector => {
  if (!_controllerConnector) {
    const options: ControllerOptions = {
      // ProviderOptions
      defaultChainId: bigintToHex(stringToFelt(chainId)),
      chains: [{ rpcUrl }],
      // IFrameOptions
      preset: 'karat',
      // KeychainOptions
      namespace: 'kode',
      policies: undefined,
      slot,
      tokens,
    };
    if (typeof window !== 'undefined') {
      console.log(`-------- ControllerOptions:`, options);
      try {
        _controllerConnector = new ControllerConnector(options) as never as ControllerConnector;
      } catch (e) {
        console.warn(`makeControllerConnector() ERROR:`, e);
      }
    }
  }
  return _controllerConnector;
}

[kronosapiens]

@rsodre what do you think of my suggestion to check if window.starknet_controller is set and no-op if so?

[rsodre]

I think so