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

Subscription closed after 3 events #3139

Closed
3 tasks done
wnederhof opened this issue Apr 8, 2023 · 3 comments
Closed
3 tasks done

Subscription closed after 3 events #3139

wnederhof opened this issue Apr 8, 2023 · 3 comments
Labels
duplicate ♊ This issue has already been reported or has been described in another issue

Comments

@wnederhof
Copy link

wnederhof commented Apr 8, 2023

Describe the bug

When using next-urql ^5.0.0 and urql ^4.0.0 instead of next-urql ^4.0.3 and urql ^3.0.3, in combination with graphql-sse ^2.1, my subscription closes when the third event comes in. The version of graphql-sse does not make a difference.

The setup:

export default withUrqlClient((ssrExchange) => {
  const exchanges = [
    dedupExchange,
    cacheExchange,
    ssrExchange,
    fetchExchange,
  ]
  if (typeof window !== 'undefined') {
    const sseClient = createSseClient({
      url: 'http://localhost:8080/subscriptions',
      credentials: 'include',
    })
    exchanges.push(
      subscriptionExchange({
        forwardSubscription: (operation) => ({
          subscribe: (sink) => ({
            unsubscribe: sseClient.subscribe(
              {
                query: operation.query as string,
                variables: operation.variables,
              },
              sink
            ),
          }),
        }),
      })
    )
  }
  return {
    url: 'http://localhost:8080/graphql',
    exchanges,
    fetchOptions: {
      credentials: 'include',
    },
    requestPolicy: 'cache-and-network',
  }
})(App)

Other than that, I'm simply using useSubscriptions with no bells or whistles.

Reproduction

Not sure how to do that with a subscription and gql-sse

Urql version

next-urql ^5.0.0
urql ^4.0.0
graphql-sse ^2.1.1

Validations

  • I can confirm that this is a bug report, and not a feature request, RFC, question, or discussion, for which GitHub Discussions should be used
  • Read the docs.
  • Follow our Code of Conduct
@kitten
Copy link
Member

kitten commented Apr 8, 2023

Are you sure you're using the latest version 4.0.4 of @urql/core with this fix? #3137

Because this exact bug has already just been reported and fixed 😅

@mnichols
Copy link

mnichols commented Apr 8, 2023

Thank you for this @kitten ..I confirmed this fixed the same bug I was about to come out and report! I had to be sure to install @urql/core along with @urql/svelte to be sure to use the latest (4.0.4).

Just to confirm here...the fetching would remain true for subscribers on a subscriptionExchange receiving args from the subscriptionStore right? It makes sense to me, but figured I'd check

@kitten
Copy link
Member

kitten commented Apr 8, 2023

@mnichols: Yes, just to clarify — although, this should also be documented in the TSDocs — fetching (no matter the API, no matter the binding) is basically UI state that means that data is expected. So, for useQuery or queryStore it means that new data is expected.

stale means that new data is being fetched in the background and that's a derived state from the Client or an exchange, like the normalised cache, or the non-normalised document cache.

hasNext is an API-driven flag and is set directly by the transport, e.g. the fetchExchange or any subscriptionExchange. For subscriptions — which is what the fix was for — it by default is set to true, and means that the transport will deliver new results, so that's directly set on your server-side schema, although transports like graphql-ws may not actually set it, since it's part of the future Incremental Delivery change

Edit: Also, just another reminder here for everyone that bindings (and exchanges) depend on @urql/core, so you may not have to install it manually and add it to your own dependencies. Although there is no harm in it. All package managers support updating transitive dependencies, e.g. in pnpm that's pnpm update @urql/core.

The only note as per the docs and the release announcement, you'll sometimes want to deduplicate dependencies (instructions in the release announcement), so you don't have two installations of @urql/core.

@kitten kitten closed this as completed Apr 8, 2023
@kitten kitten added the duplicate ♊ This issue has already been reported or has been described in another issue label Apr 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate ♊ This issue has already been reported or has been described in another issue
Projects
None yet
Development

No branches or pull requests

3 participants