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

fix(react): set fetching to the lack or presence of source #2667

Merged
merged 2 commits into from
Sep 4, 2022

Conversation

JoviDeCroock
Copy link
Collaborator

Resolves #2648

Summary

When we changes variables onEnd seems to fire later than us establishing the new connection, this means we set fetching to a truthy value and then switch it off as we assume that we aren't fetching anymore.

Now when a subscription ends we check whether there is a new source already and base our fetching state off that, sandbox

CC @elias-pap

Set of changes

  • rely on source to flip fetching state

@JoviDeCroock JoviDeCroock requested a review from kitten September 3, 2022 10:12
@changeset-bot
Copy link

changeset-bot bot commented Sep 3, 2022

🦋 Changeset detected

Latest commit: 25c5cdf

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

This PR includes changesets to release 1 package
Name Type
urql 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 changed the title fix(react):"set fetching to the lack or presence of source fix(react): set fetching to the lack or presence of source Sep 3, 2022
@JoviDeCroock JoviDeCroock merged commit 1b15c5f into main Sep 4, 2022
@JoviDeCroock JoviDeCroock deleted the onend-firing-late branch September 4, 2022 07:47
@urql-ci urql-ci mentioned this pull request Sep 4, 2022
@elias-pap
Copy link

This change might have introduced a different bug. I use wonka's fromPromise to mock executeSubscription on testing when creating a mock urql client, like this:

executeSubscription: ({ query, variables }) =>
      executeMockOperation({ schemaWithMocks, query, variables }),

const executeMockOperation = ({ schemaWithMocks, query, variables }) => {
  const result = graphql({
    schema: schemaWithMocks,
    source: query.loc.source.body,
    variableValues: variables,
  });
  return fromPromise(result);
};

Before the fix (3.0.2) fetching is eventually set to false when the data comes (expected). After the fix (3.0.3) fetching is set to true and never turns false again. I had to implement a custom source with makeSubject to work around this.

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.

fetching is not set to true for subscriptions on variable change
3 participants