fix(AddressType): cleanup overrides

[sverps]

Description

Cleanup of AddressType overrides

Additional Information

• I have read the contributing docs (if this is your first contribution)
• This is not a duplicate of any existing pull request

Related Issues

Fixes #514

Your ENS/address: solidixen.eth

[sverps]

I also updated viem and wagmi on this branch, and didn't run into any type issues. @technophile-04 would be good if you also had a look and check if the issue you used to encounter can happen here, maybe there's something I missed when trying to reproduce.

[rin-st]

Hey @sverps !

Looks good, but next:check-types script gives me node_modules/viem errors again.

Found 684 errors in 42 files.

[sverps]

Looks good, but next:check-types script gives me node_modules/viem errors again.

Hmm, strange, did you do anything in particular, or did you just checkout the branch and then yarn install? Cause I didn't seem to get any errors on next:check-types

[rin-st]

no, just checkout -> install -> next:check-types

[sverps]

I tried a bunch of different things, but can't seem to reproduce this behavior. Can you try removing your node_modules and re-run yarn install?

[rin-st]

Can you try removing your node_modules and re-run yarn install?

It worked!

I'm wondering is it only for me or for other users too. Hope we don't need to force them to remove node_modules. Let's wait a bit and see how it works for others.

[technophile-04]

Current Behavior
To not have things break, we currently override the AddressType in abi.d.ts to be string. This resulted in broken types when viem and wagmi had versions that didn't match well.
~ From #514

I also updated viem and wagmi on this branch, and didn't run into any type issues. ~From #515 (comment)

We actually already solved this at #499, So now using a different version of viem / wagmi should not cause any issues in the future

The solution was: separately targeting Viem's AddressType as suggested in abitype docs extending config and we are now doing this at :

scaffold-eth-2/packages/nextjs/types/abitype/abi.d.ts

Line 3 in 82d6dca

declare module "viem/node_modules/abitype" {

---

... while still maintaining newcomer-friendly plain string address typing for the components we expect them to use. (better DX especially for newcomers) ...
... without leading them to believe recent wagmi and viem versions take plain strings if they end up using those libraries without our wrappers in some project.
~ From #514

And could you please explain how it decrease dx?
~ From #492 (comment)

The reason I don't like so much stricter type is because it does not go well with other tool's / libraries. As we can see in the above case even though I have the initial state set as "0x...." react's useState infers it as string which causes issue in args

And now I have to assert it at useState : useState<Address>

The error at args gets solved but now I get an error at input :

Another option might be instead of asserting it at useState we do check at args : args: [isAddress(inputAddress) ? inputAddress : undefined],

But beginners / quick tinkerers would rather just show the error on UI which wagmi / viem nicely gives you :

Also if people use other libraries, for examle Uniswap sdk or Safe SDK it have function getAddress but still I need I need to do extra checks while passing it to args, etc

LOL really sorry for ranting 😂 and over-exaggerated examples, But I feel since wagmi/viem natively support overriding ( docs extending config ) I think we can do and We can mention it in our docs.scaffoldeth.io about it that we have overridden it

But I completely get your points and makes full sense to me 🙌 I am up for overriding if we all come to a consensus

---

Nevertheless, I like these changes that we are using a consistent Address type instead of keeping it string

In case if we don't override AddressType: string, I think we can mention in our docs that we have overridden it, and If you want to be stricter with types you can just delete the abi.d.ts file and everything will work and people can also be stricter OR by default have them stricter and we can tell people that they can add abi.d.ts to have types beginner friendly 🙌

[technophile-04]

I'm wondering is it only for me or for other users too. Hope we don't need to force them to remove node_modules.

It worked for me directly without needing to remove node_modules 🙌, I just checked out and did yarn install and yarn next:check-types and everything was passing 🙌

[sverps]

Great points of discussion, @technophile-04 👍

On thinking a bit further, I tried to boil it down for myself what I think would be ideal:

• We want good DX, targeting also people that are fairly new to TypeScript. So with that in mind, I agree with your points that letting them use addresses that are of type string would be good.
• The places where they should be allowed to do so are, in my opinion, mainly our wrapper hooks and custom scaffold-eth components.
• Using viem/wagmi directly, is probably already a step further in their learning process, and devs that do that likely have some better understanding of types already. So I'd expect them to better understand the Address type.
• Overriding types via d.ts files is an even more advanced concept, and I feel it's not something obvious even for slightly more advanced users. That's why I'd prefer not to do that and opt for keeping things simpler.

So to conclude, I'd try and find all touch points where users would use addresses in our custom hooks and components and ensure those places to allow string, then cast them to Address in our internal implementation (i.e. in the [args] example see my last commit).

So in any case, no need to rush anything, I probably still need to fix return types from our hooks to return it as string.

(Need to let this simmer a bit more, the stew isn't quite ready yet 🍲 😅 )

[damianmarti]

I just checked out and did yarn install and yarn next:check-types and everything was passing 🙌

I did the same and everything was passing here too!

[technophile-04]

The places where they should be allowed to do so are, in my opinion, mainly our wrapper hooks and custom scaffold-eth components.
Using viem/wagmi directly, is probably already a step further in their learning process, and devs that do that likely have some better understanding of types already. So I'd expect them to better understand the Address type.
Overriding types via d.ts files is an even more advanced concept, and I feel it's not something obvious even for slightly more advanced users. That's why I'd prefer not to do that and opt for keeping things simple

Yup yup, I agree and this makes complete sense to me !! Thanks Samuel 🙌

[sverps]

Closed in favor of #630