wslyvh / nexth

A Next.js + Ethereum starter kit with Viem, Wagmi, Web3Modal, SIWE, Tailwind, daisyUI and more to quickly ship production-ready Web3 Apps ⚡
https://nexth.vercel.app/
MIT License
671 stars 141 forks source link

feat: foundry framework support and OZ bump version #23

Closed jpgonzalezra closed 12 months ago

jpgonzalezra commented 1 year ago

Hey @wslyvh, I hope this message finds you well.

In the course of utilizing nexth, I recognized an opportunity to extend its compatibility beyond its current framework. Specifically, I've been employing Foundry as my primary framework for Solidity, and saw great potential in integrating it with nexth's capabilities.

I'm excited about the prospect of sharing these enhancements with you and the broader nexth community.

Here's an outline of the key changes:

vercel[bot] commented 1 year ago

@jpgonzalezra is attempting to deploy a commit to the useWeb3 Team on Vercel.

A member of the Team first needs to authorize it.

wslyvh commented 1 year ago

Hi @jpgonzalezra this is great! Thank you for submitting a PR 🙏

I've been meaning to try out Foundry, but haven't found the time for it yet. So its' great to see a contribution here! I wonder a little bit if it makes sense to combine these in the same branch tho. I doubt you want to use both Hardhat and Foundry simultaneously while building, right?

Should we keep 1 branch for hardhat contract and a separate branch for Foundry? What are your thoughts?

jpgonzalezra commented 1 year ago

Thank you for the prompt response. I think you're right; it doesn't make much sense to have both frameworks together. We could proceed as follows, if you agree:

We can keep Hardhat as the default on the main branch, create a "foundry" branch with the implementation for Foundry, and document this in the readme.

main -> Hardhat only foundry -> Foundry only

My only concern is that it might be challenging to keep both branches updated. What are your thoughts on this?

wslyvh commented 1 year ago

Hmm, you're right. That's fair! Maybe a better approach could be to just create another package and keep them side by side. So we'd end up with /packages/app /packages/hardhat /packages/foundry

If both hardhat and foundry have the same example, with similar commands and output, it's up to devs to pick their preference and delete the other.

jpgonzalezra commented 1 year ago

Great, I like this approach much better. I start working on it right now 💪

wslyvh commented 1 year ago

Awesome! Sounds good :)

jpgonzalezra commented 1 year ago

@wslyvh I think it's a bit better now. We could work from this point with your feedback to improve the PR. I've left some comments to see what you think 🙌

wslyvh commented 12 months ago

Hi @jpgonzalezra sorry for the delay as I was working on some other priorities. Left some comments on a few of your questions. Hope that makes sense! But should be good to merge either way

jpgonzalezra commented 12 months ago

Hi @wslyvh no problem, this part of the year is terrible, a lot of work. I finished the comments, let me know if you would like to modify anything else 💪

wslyvh commented 12 months ago

Looks good! Thanks again for contributing. Exciting to finally have Foundry as part of the stack