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

Adding Mode chain definitions #5910

Closed
sshmaxime opened this issue Jan 8, 2025 · 7 comments
Closed

Adding Mode chain definitions #5910

sshmaxime opened this issue Jan 8, 2025 · 7 comments

Comments

@sshmaxime
Copy link

The Mode chain definition is missing. It's pretty weird considering you guys have modeTestnet.

Copy link

linear bot commented Jan 8, 2025

@gregfromstl gregfromstl self-assigned this Jan 8, 2025
@gregfromstl
Copy link
Member

Hey @sshmaxime, I'm not sure why we would have Mode's testnet and not Mode but we'd generally like to minimize the number of predefined chain definitions. You can get Mode's chain definition with defineChain(34443) (which is arguably easier than the predefined chains).

@sshmaxime
Copy link
Author

sshmaxime commented Jan 14, 2025

Hello @gregfromstl , you marked this as completed but I'm not seeing it in the latest version of Thirdweb. I tried to check the branches to see if I wasn't up to date but I checked on main and the type definition for Mode doesn't appear - I can still see Mode Testnet tho.

Did I miss something ?

Would you please mind adding it ? It's more practical for us to import the chain directly than having them in our own repository. Thanks 🙏

@gregfromstl
Copy link
Member

gregfromstl commented Jan 17, 2025

Hey @sshmaxime I may have misunderstood. Is there a reason you can't use defineChain?

@gregfromstl gregfromstl reopened this Jan 17, 2025
@sshmaxime
Copy link
Author

The reason is that I like to have a clean code with the the least customized stuff on our side. We are already implementing few of your chains using thirdweb/chains, I'm just hoping to see the addition of the Mode chain .. it's easier for us and probably for some of your other users too 🙏

@gregfromstl
Copy link
Member

We like to have clean code too 🙂

Adding named imports for every chain (2,500+) would dramatically increase our repo size and upkeep, as chains have their exceptions/edge cases. That's why we prefer to minimize the number of chains that have named imports (and ideally we would have none). That said, because you seem like a great guy and we already have mode testnet I've opened a PR to add it as a predefined chain: #5982

@sshmaxime
Copy link
Author

Thank you @gregfromstl 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants