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

useQuery not re requesting if variables changes while request is already sent #1150

Closed
beaussan opened this issue Nov 14, 2020 · 9 comments · Fixed by #1157
Closed

useQuery not re requesting if variables changes while request is already sent #1150

beaussan opened this issue Nov 14, 2020 · 9 comments · Fixed by #1157
Labels
bug 🐛 Oh no! A bug or unintented behaviour.

Comments

@beaussan
Copy link

beaussan commented Nov 14, 2020

urql version & exchanges:

Steps to reproduce

  1. have a query
  2. update the query variables while the query is in flight

Expected behavior

The result is based on the first variable and not the second one

Actual behavior

The result should be a request with the new input variables

Example code

export const useCurrentUser = () => {
// This return first a undefined, then 100 ms later will send the user
  const firebaseUser = useFirebaseUser(userWithCorrectToken$);

  const [
    { data, error, fetching, extensions, operation },
    retry,
  ] = useQuery({
    query: myQuery,
    variables: {
      firebaseId: firebaseUser?.uid ?? 'anonymous',
    },
    requestPolicy: 'network-only',
  });

  console.log('useCurrentUser', {
    data,
    error,
    fetching,
    extensions,
    operation,
  });

  const user = data?.user?.[0];
  return { error, user, fetching, loading: fetching };
};

This error occurs in 1.11 but not in 1.10. This may be related to the change in this pr : #1104

I will make a sandbox soon, I don't have time right now but will soon !

@beaussan beaussan added the bug 🐛 Oh no! A bug or unintented behaviour. label Nov 14, 2020
@kitten kitten added the needs more info ✋ A question or report that needs more info to be addressable label Nov 14, 2020
@kitten
Copy link
Member

kitten commented Nov 14, 2020

We do have tests covering this, so I'd need some more information or a reproduction around this behaviour you're seeing, since not much has changed around how we subscribe/unsubscribe and update during the hook's lifecycle.

@beaussan
Copy link
Author

I managed to reproduce it, it only to this in suspence mode, here is the sandbox : https://codesandbox.io/s/ecstatic-ritchie-lzxvl?file=/src/components/Todos.js

@kitten
Copy link
Member

kitten commented Nov 14, 2020

@beaussan just to speed this up, what are you expecting the sandbox to output if it was working correctly rather than what is happening?

Edit: also are you aware that your temporary state doesn't correctly use a tear down and will also be cleared since it's below your suspense boundary? Once a subtree suspends, all its state is thrown away by React

@beaussan
Copy link
Author

The output should have the same id in Todo for $id and in the body id: $id.

This is to mockup a call to my auth api, witch is done in RxJS anyway.

The id in both places should be ZmlsbXM6Mw==, it is the correct one in the component body (Todos for ...), but not the query output. The call is made (you can look for index in the network tab), the first one for the first Id, and de second one for the next one, however, the second one isn't send back to the react component

@beaussan
Copy link
Author

beaussan commented Nov 14, 2020

I have updated the sandbox to better show the error

Also, using a previous urql version (prior to 1.11) it works well (tested on 1.10.3)

@kitten kitten removed the needs more info ✋ A question or report that needs more info to be addressable label Nov 15, 2020
@kitten
Copy link
Member

kitten commented Nov 16, 2020

Ok, this was a tricky one! As it turns out suspense-on-update behaves very differently to suspense-on-mount since React (Concurrent) has some mechanisms that allow it to continue on with the suspended subtree as usual to delay showing a loading screen (I assume using useTransition)

The side-effect of this is that it doesn't erase any state for our suspended component if nothing has changed, which means that once we resume after suspending from the update we won't apply any of the state changes we've seen.

That's now fixed and it'll be published as 1.11.3.

@beaussan
Copy link
Author

Thank you for the quick fix !

I was migrating from Apollo to urql to get suspense, and this bug drive me crazy until I tried a earlier version !

Thank you for this project, and for your support !

@kitten
Copy link
Member

kitten commented Nov 17, 2020

That's just been published 🙌 #1154

@beaussan
Copy link
Author

Thank you !! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Oh no! A bug or unintented behaviour.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants