wevm / wagmi

Reactive primitives for Ethereum apps
https://wagmi.sh
MIT License
5.97k stars 1.05k forks source link

bug: autoConnect fails to reconnect #2511

Closed kenkunz closed 1 year ago

kenkunz commented 1 year ago

Is there an existing issue for this?

Package Version

1.1.0

Current Behavior

I am using @wagmi/core in a SvelteKit project. I am using the InjectedConnector to connect an injected wallet.

With an easier version (0.10.10), the autoConnect feature worked as expected – if the user had already connected their injected wallet and returned to the site, the wallet would auto-reconnect.

After upgrading to 1.x, this feature stopped working.

See #2439 for additional details / discussion.

Expected Behavior

When using autoConnect, the injected wallet automatically reconnects when a user returns to the site (or just on page-refresh).

Steps To Reproduce

See the linked minimal repro for a more detailed example.

  1. Configure wagmi with autoConnect: true:
const { publicClient, webSocketPublicClient } = configureChains(
  [mainnet],
  [publicProvider()]
);

createConfig({
  autoConnect: true,
  publicClient,
  webSocketPublicClient,
});
  1. Connect a wallet using InjectedConnector:
connect({ connector: new InjectedConnector() });
  1. Refresh page, note that wallet doesn't reconnect.

Link to Minimal Reproducible Example (StackBlitz, CodeSandbox, GitHub repo etc.)

https://github.com/kenkunz/wagmi-autoconnect-repro

Anything else?

No response

skplunkerin commented 1 year ago

I'm having the same issue in a Blitz.js project; on version 0.12.12 after our users connected there wallet the connection info would be retained if they left/came back, after upgrading wagmi to 1.0.2 after our users connect their wallet, the info is lost by just refreshing the page.

A workaround the OP found (and posted in #2439 comment here) was creating a Local Storage key/val that the old wagmi version created which is no longer created in 1.x:

localStorage.setItem('wagmi.injected.shimDisconnect', 'true')

I looked into ways to not need to manually create this by adding the shimDisconnect option to the InjectedConnector, but setting that to true or false did nothing to fix this.

kenkunz commented 1 year ago

FYI, the workaround mentioned by @skplunkerin above is demonstrated in the GH repro linked in the description above.

ombayoeco11 commented 1 year ago

did you placed the ethereumClient, createConfig, and configureChains outside you function App() or function MyApp() ? I assume you're using web3modal too.

ombayoeco11 commented 1 year ago

oops sorry i did'n see you minimum repro. have you tried this ? import { InjectedConnector } from '@wagmi/core'

const connector = new InjectedConnector({ options: { shimDisconnect: true, }, })

skplunkerin commented 1 year ago

const connector = new InjectedConnector({ options: { shimDisconnect: true, }, })

@ombayoeco11 I tried that and it did not work. I also tried setting it to false (since the default value of shimDisconnect is already true), which also did nothing.

kenkunz commented 1 year ago

@ombayoeco11 I had the same results as @skplunkerin – setting shimDisconnect to true (or false) has no observable impact on the autoConnect behavior.

I checked localStorage values with shimDisconnect set explicitly – the wagmi.injected.shimDisconnect key is never set.

So it appears that:

So perhaps the source of the defect is with InjectedConnector handling of shimDisconnect setting.

kenkunz commented 1 year ago

Comparing the behavior of the latest @wagmi/core release prior to 1.x with 1.x

autoConnect shimDisconnect v0.10.11 v1.x
default (false) default (true) does not reconnect does not reconnect
default (false) true does not reconnect does not reconnect
default (false) false does not reconnect does not reconnect
false default (true) does not reconnect does not reconnect
false true does not reconnect does not reconnect
false false does not reconnect does not reconnect
true default (true) ✅ reconnects ❌ does not reconnect
true true ✅ reconnects ❌ does not reconnect
true false does not reconnect does not reconnect
ombayoeco11 commented 1 year ago

i agree, probably because the shimDisconnect is not working perfectly. i got the autoConnect problem with web3Modal and connecting it with TrustWallet. In metamask, it's running as i wanted. but In TrustWallet, it keeps disconnecting, even when i change the route.

jchaselubitz commented 1 year ago

I am having this problem with Metamask, but only in production. It seems to hold it's connection on my local/dev environment. Coinbasewallet (not Injected) does not hold it's connection in any case.

glitch-txs commented 1 year ago

Hi guys! I think I found the issue in this PR: https://github.com/kenkunz/wagmi-autoconnect-repro/pull/1

let me explain what I see, connectors are constructed with an empty storage property, the function createConfig takes connectors as arguments and add them a customized storage which is the one the connector will use to store the shimDisconnect data when you call the connect function

if the connector is not passed through the createConfig function, when calling connect its storage will be empty, so it won't save this config on localStorage.

skplunkerin commented 1 year ago

thanks for the suggestion @glitch-txs ! It looks like this works for me as well when I put the InjectedConnector inside the CreateConfig(). For my project it's not as simple as re-using the const injectedConnector since it's a separate React file from where CreateConfig() is defined, but doing the following is working for me:

_app.tsx

// ...
import { createConfig } from 'wagmi'
import { InjectedConnector } from 'wagmi/connectors/injected'
// ...

const config = createConfig({
  autoConnect: true,
  connectors: [new InjectedConnector()], // <-- added this line
  publicClient,
  webSocketPublicClient
})

// ...

export default withBlitz(function App({ Component, pageProps }: AppProps) {
  // ...
  const getLayout = Component.getLayout || ((page) => page)
  return (
    <NextQueryParamProvider>
      {/* ... */}
        <WagmiConfig config={config}>
          {/* ... */}
        </WagmiConfig>
      {/* ... */}
    </NextQueryParamProvider>
  )
})

// ...

useWalletSignin.ts

import {useConfig, useConnect} from 'wagmi'
// ...

export default function useWalletSignin() {
  // pull the InjectedConnector from the wagmi config (created in `_app.tsx`)
  const { connectors } = useConfig()
  const { connectAsync } = useConnect({
    // connector: new InjectedConnector() // <-- removed this line
    connector: connectors[0] // <-- added this
  })

  // ...

}
tmm commented 1 year ago

@glitch-txs is correct! Since the connector doesn't have storage defined, it can't read from and write to storage. The easiest way to set up the storage property for connectors is to "register" them with the config through createConfig. Alternatively, you can call connector.setStorage to set things up. We'll work on a better API and documentation for the latter case in a future version.

kenkunz commented 1 year ago

Thanks @glitch-txs for your diagnosis / explanation, and @tmm for further clarification.

I just wanted to note that the Getting Started guide perfectly demonstrates this issue (my minimal repro follows the guide line-for-line). I think it would really help if the guide were updated to reflect a working scenario (especially since autoConnect: true is explicitly configured there).

Additionally, this behavior is a departure from v0.10.x, so I would recommend it be listed as a 1.x.x breaking change in the Migration Guide.

@tmm let me know if you agree – I would be happy to submit a PR with the above documentation improvements.

tmm commented 1 year ago

@kenkunz sounds great!

S1nPur1ty commented 1 year ago

So I just tried using Wagmi for the first time, and the quick tutorial on the home page is still not updated. I came here to report the same issue. I use NextJS 13.4.2 and I thought that this was an issue with Next/React.

Here is my band aid solution:

    const { address, isConnected } = useAccount()
    const { connect } = useConnect({
      connector: new MetaMaskConnector(),
    })
    const { disconnect } = useDisconnect()

    useEffect(() => {
        const state = localStorage.getItem('walletConnectState') || '';

        if( !isConnected && state === 'true' ) connect();
    }, []);

    useEffect(() => {
        localStorage.setItem("walletConnectState", isConnected.toString());
    }, [isConnected]);

I also removed the autoConnect: true within the _app.tsx file where the createConfig is. Funny enough, this resulted in fixing the hydration issue too.

No idea why this issue is closed since the problem is still there.

AlphaDio commented 1 year ago

No idea why this issue is closed since the problem is still there.

I also run into that problem, after a 0.10.x -> 1.0.x upgrade, although the problem might come from create-react-app on my side.

3hrab commented 1 year ago

This changes nothing in my setup. autoConnect still does not function. localstorage var is set, but wagmi does not pick it back up.