Skip to content

Conversation

@alexcambose
Copy link
Contributor

@alexcambose alexcambose commented Aug 17, 2025

Summary

Removed Suspense from extraCta to fix server boundary crash in header. This does fix the error but it's not super optimal as we're loosing the <SearchButtonImpl isLoading /> state.

Rationale

I think the issue is that Nextjs can’t stream Suspense data inside props passed to a Client Component;

  <AppShell (client)
    ...
    extraCta={
      <Suspense fallback={<SearchButtonImpl isLoading />}>
        <SearchButton />
      </Suspense>
    }
  >
    {children}
  </AppShell>

And it throws The server could not finish this Suspense boundary, likely due to an error during server rendering. Switched to client rendering.

Another possible way to fix this would be to just fetch the data on the client side with useState/useEffect.

Tried a few other ways, but couldn't really get it to either work or not throw the same error.

How has this been tested?

  • Current tests cover my changes
  • Added new tests
  • Manually tested the code

@alexcambose alexcambose requested a review from cprussin August 17, 2025 16:05
@alexcambose alexcambose self-assigned this Aug 17, 2025
@alexcambose alexcambose requested a review from a team as a code owner August 17, 2025 16:05
@vercel
Copy link

vercel bot commented Aug 17, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
api-reference Ready Ready Preview Comment Aug 18, 2025 10:56am
component-library Ready Ready Preview Comment Aug 18, 2025 10:56am
developer-hub Ready Ready Preview Comment Aug 18, 2025 10:56am
entropy-explorer Ready Ready Preview Comment Aug 18, 2025 10:56am
insights Ready Ready Preview Comment Aug 18, 2025 10:56am
proposals Ready Ready Preview Comment Aug 18, 2025 10:56am
staking Ready Ready Preview Comment Aug 18, 2025 10:56am

Copy link
Collaborator

@cprussin cprussin left a comment

Choose a reason for hiding this comment

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

I'm not sure but I think this will cause more perf issues because it means that all pages will have to delay for the async stuff in <SearchButton> before rendering the layout.

@alexcambose I suggest you first merge your perf improvements PR, then rebase this on top and do some testing to ensure it doesn't cause more perf issues.

We might have to think of another way to implement the global search if we can't use suspense.

@alexcambose
Copy link
Contributor Author

@cprussin I agree. Do you think it would make sense to statically fetch/generate the data at build time and then inject it into these components? Wondering how often things will change in relation to the search.

@alexcambose
Copy link
Contributor Author

Tested some more and somehow it doesn't seem to add any additional latency even without Suspense.

@cprussin
Copy link
Collaborator

Tested some more and somehow it doesn't seem to add any additional latency even without Suspense.

lol ok, that's surprising but maybe just due to the added redis caching... let's ship it!

@alexcambose
Copy link
Contributor Author

This is one of the "I have no idea how it works but it works"

@alexcambose alexcambose merged commit 4a1a0e0 into main Aug 18, 2025
10 checks passed
@alexcambose alexcambose deleted the fix/suspense-props branch August 18, 2025 18:01
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.

3 participants