wagmi icon indicating copy to clipboard operation
wagmi copied to clipboard

injected.ts - check shimDisconnect before requestPermissions

Open mqklin opened this issue 2 years ago • 5 comments

Following https://github.com/wevm/wagmi/issues/3603

Description

shimDisconnect should be considered before showing prompt for selecting account

Additional Information

Before submitting this issue, please make sure you do the following.

  • [x] Read the contributing guide
  • [ ] Added documentation related to the changes made.
  • [ ] Added or updated tests (and snapshots) related to the changes made.

mqklin avatar Feb 16 '24 17:02 mqklin

🦋 Changeset detected

Latest commit: 46e33cee2c7f924f4ee16627664f10e6215c45c5

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@wagmi/core Patch
@wagmi/connectors Patch
wagmi Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

changeset-bot[bot] avatar Feb 16 '24 17:02 changeset-bot[bot]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
wagmi ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 27, 2024 6:49pm

vercel[bot] avatar Feb 16 '24 17:02 vercel[bot]

From my understanding there is only one-line change? Also not sure what to do with changeset..

mqklin avatar Feb 16 '24 18:02 mqklin

I tested it on SanR.app codebase, no bugs encountered.

mqklin avatar Feb 16 '24 19:02 mqklin

@tmm any chance for this to be reviewed / merged / released?

mqklin avatar Feb 21 '24 18:02 mqklin

I donated $200 in hope this will be prioritized https://explorer.zksync.io/tx/0xc8316292532d5636fe4086e0e9434bf7ea63b33538ce6a7890ae73cd2c39d5cb https://explorer.zksync.io/tx/0x495e573050afd1fe07cf5b0e0ad5ecc8f745a129f6c5c19c20e16b6f205f778d

mqklin avatar Feb 24 '24 15:02 mqklin

So I believe the way to resolve is 1) merge this PR 2) fix the bug I described here: https://github.com/wevm/wagmi/pull/3608#discussion_r1511539660

IDK how to fix it. shimDisconnect is only passed to injected, but how to save it and pass further to createConfig? I see 3 ways:

  1. pass shimDisconnect to createConfig, but this duplicates the code:
const wagmiConfig = createConfig({
  ...,
  shimDisconnect: false,
  connectors: [
    injected({shimDisconnect: false}),
  ],
});
  1. Save shimDisconnect as a closure in injected.ts, which looks hacky:
let shimDisconnect;
export function injected(parameters = {}) {
  shimDisconnect ??= parameters.shimDisconnect ?? true;
  const { unstable_shimAsyncInject } = parameters;
  ...

To make it work "clear", shimDisconnect should be passed to midp lib, which is I believe not what you want. So seems like 2nd option I provided is the best way to fix the bug.

  1. nd option is to save shimDisconnect inside createConnector config:
...
        get shimDisconnect() {
          return shimDisconnect;
        }

Then create connectors inside createConfig just to get this shimDisconnect:

...
  const shimDisconnect = rest.connectors.map(f => f()).some(connector => connector.shimDisconnect);
  ...
  ? mipd?.getProviders().map(providerDetailToConnector.bind(null, shimDisconnect)) ?? []
...

And receive it in providerDetailToConnector function below:

...
    function providerDetailToConnector(providerDetail, shimDisconnect) {
        const { info } = providerDetail;
        const provider = providerDetail.provider;
        return injected({ shimDisconnect, target: { ...info, id: info.rdns, provider } });
    }
...

mqklin avatar Mar 04 '24 17:03 mqklin

@jxom please let me know if you want me to PR 2nd option I provided above

mqklin avatar Mar 04 '24 18:03 mqklin