Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(address type): allows user to set his preference in scaffold config #630

Merged
merged 1 commit into from
Dec 2, 2023

Conversation

sverps
Copy link
Collaborator

@sverps sverps commented Nov 29, 2023

I aligned all addresses to use the Address type of viem (see #617).

Also added a config option for a dev to use more strict Address if he wants additional typesafety (default is current behavior where addresses are typed as string).

(This also basically fixes #514 by making it optional, with a sensible default)

@technophile-04
Copy link
Collaborator

technophile-04 commented Nov 30, 2023

Also added a config option for a dev to use a more strict Address if he wants additional typesafety (default is current behavior where addresses are typed as string).

Love this idea, best approach !!!!! Lets fix the conflicts will test this out later today 🙌

Also I think this also closes #515

@sverps sverps mentioned this pull request Nov 30, 2023
2 tasks
@sverps
Copy link
Collaborator Author

sverps commented Nov 30, 2023

@technophile-04 conflicts resolved

Copy link
Member

@rin-st rin-st left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Collaborator

@technophile-04 technophile-04 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice !! Tested it thoroughly and works like a charm !!! Tysm @sverps !

Just a small discussion question should we keep useStrictAddressType: true by default (so that we are inlined with wagmi / viem by default and people can tweak it to false if they want ?)

I would kind of vote for keeping it useStrictAddressType: false (which is already current state) but not completely sure and would love to know others opinion 🙌

But yes really love the approach and already is ready for merge 🙌

@sverps
Copy link
Collaborator Author

sverps commented Dec 2, 2023

As much as I prefer stricter types, I'd go for the newbie-friendly approach as that's a big group of the target users.

@sverps sverps merged commit fd24f2b into main Dec 2, 2023
1 check passed
@sverps sverps deleted the fix/617-address-type branch December 2, 2023 22:44
@github-actions github-actions bot mentioned this pull request Dec 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cleanup of abi Address type
3 participants