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

chore: Fix wallet hydration issue on next.js #1117

Merged
merged 7 commits into from
Aug 23, 2024

Conversation

abcrane123
Copy link
Contributor

@abcrane123 abcrane123 commented Aug 20, 2024

What changed? Why?
Following up on #1077

User reported receiving a hydration warning from their next.js app when integrating the Wallet component which is likely because the Wallet component depends on client-specific data. The user is using pages routing and components in the pages/ directory are considered to be client-side by default, which, in this case, results in a hydration error.

I am resolving this issue by returning null in Wallet component until the component mounts.

Notes to reviewers

How has it been tested?

Copy link

vercel bot commented Aug 20, 2024

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

Name Status Preview Comments Updated (UTC)
onchainkit-coverage ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 22, 2024 8:24pm
onchainkit-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 22, 2024 8:24pm
onchainkit-routes ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 22, 2024 8:24pm

const [isClient, setIsClient] = useState(false);

useEffect(() => {
setIsClient(true);
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's try to solve this for 2~3 components at the same time.

As I see this being an issue for Identity components as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i tested Identity in my app and it didn't seem to be an issue

@@ -43,6 +43,17 @@ const WalletContent = ({ children }: WalletReact) => {
};

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's make sure there is a changeset for this PR as well.

@abcrane123 abcrane123 marked this pull request as draft August 22, 2024 15:52
@@ -26,6 +27,13 @@ export function Swap({
};
}, [children]);

const isMounted = useIsMounted();
Copy link
Contributor

Choose a reason for hiding this comment

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

This is really nice

@@ -0,0 +1,9 @@
import { useEffect, useState } from 'react';

export function useIsMounted() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Love this!!!!

@Zizzamia Zizzamia merged commit 4382d93 into main Aug 23, 2024
16 checks passed
@Zizzamia Zizzamia deleted the alissa.crane/hydration-issue branch August 23, 2024 00:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants