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(next): warn for missing suspense boundary #3449

Merged
merged 1 commit into from
Dec 6, 2023

Conversation

JoviDeCroock
Copy link
Collaborator

@JoviDeCroock JoviDeCroock commented Dec 6, 2023

Related to #3448 and #3408

Summary

We have seen a few issues where folks forget their Suspense boundary which leads to infinite requests. We will shield from infinite requests by providing our own underneath the urql provider so the client doesn't get re-created and give the user a warning when we are not in production.

Copy link

changeset-bot bot commented Dec 6, 2023

🦋 Changeset detected

Latest commit: 1106f99

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@urql/next Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@JoviDeCroock JoviDeCroock requested a review from kitten December 6, 2023 15:26
DataHydrationContextProvider,
{ nonce },
React.createElement(
React.Suspense,
Copy link
Member

Choose a reason for hiding this comment

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

Hm, my only concern here is that there could be legitimate uses for this. I'm imagining that some router systems may use suspense in unexpected ways, so not sure if we can somehow make detection work without a custom boundary?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That feels counter-intuitive as for the Provider to work you would need a boundary either way so even-if something higher up is a Suspense boundary that would render the urql provider pointless

Copy link
Contributor

Choose a reason for hiding this comment

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

If this Suspense is inserted on the server side, the Promise resolution in the React component will change to streaming instead of waiting.

I'm sorry, but SSR in urql without Suspense will be impossible.

https://next-blog.croud.jp/contents/7m4dShGLIpFkc0GKHIrW

Can you please adjust the server side so that Suspense is not interrupted?

@JoviDeCroock JoviDeCroock merged commit fa692c1 into urql-graphql:main Dec 6, 2023
25 checks passed
@github-actions github-actions bot mentioned this pull request Dec 6, 2023
@JoviDeCroock JoviDeCroock deleted the next-suspense-warning branch December 7, 2023 08:13
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