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: useQuery loading and debounce issues #1313

Merged
merged 2 commits into from
May 3, 2022

Conversation

neil585456525
Copy link
Contributor

@neil585456525 neil585456525 commented Feb 1, 2022

#1235
Should let loading ref be true when refetch.

#1271
Should use currentOptions.value instead of just currentOptions.

@neil585456525 neil585456525 changed the title fix: when refetch, loading won't changed to true. fix: useQuery bug. Feb 1, 2022
@ferm10n
Copy link

ferm10n commented Feb 14, 2022

is it possible that this comment is talking about currentOptions itself being uninitialized, and not currentOptions.value?

@ferm10n
Copy link

ferm10n commented Feb 14, 2022

My interpretation of the watch on the optionsRef is basically: "if the options (throttle or debounce) have changed, update the restart fn, then call it with the new options"

At least, that's what I think the intent was. It doesn't actually work that way because it only updates the options when changed, not initialized.

Another part to solving the debounce issue might be something like this:

  watch(() => isRef(optionsRef) ? optionsRef.value : optionsRef, value => {
    const oldThrottle = currentOptions.value?.throttle;
    const oldDebounce = currentOptions.value?.debounce;
    currentOptions.value = value;
    if (oldThrottle !== value.throttle || oldDebounce !== value.debounce)) {
      updateRestartFn()
    }
    restart()
  }, {
    deep: true,
    immediate: true,
  })

Both this solution and the one in this PR will solve the issue, but I think it makes sense that both are used (assuming my earlier comment above isn't relevant)

@neil585456525
Copy link
Contributor Author

neil585456525 commented Feb 15, 2022

is it possible that this comment is talking about currentOptions itself being uninitialized, and not currentOptions.value?

Hi, @ferm10n ! Welcome to join the discussion. In my personal opinion, maybe? First, because the currentOptions has been defined when this function implementation start. As it use ref as the defined type, all value changing should be based on the currentOptions.value to keep the Vue reactivity.
But in other perspective, maybe in server environment the currentOptions will truly be uninitialize for real? We could do some test to find out:)
Or another possibility what I guess is that maybe in server side, there are something additional logic should be run, since it still in progress.
But to be honest , I think if this code's writer can give us some details about it, we could understand it without too much confusion.

@neil585456525
Copy link
Contributor Author

  watch(() => isRef(optionsRef) ? optionsRef.value : optionsRef, value => {
    const oldThrottle = currentOptions.value?.throttle;
    const oldDebounce = currentOptions.value?.debounce;
    currentOptions.value = value;
    if (oldThrottle !== value.throttle || oldDebounce !== value.debounce)) {
      updateRestartFn()
    }
    restart()
  }, {
    deep: true,
    immediate: true,
  })

@ferm10n LGTM👍
BTW, after I have read the logic you mentioned, I come up with another solution that like this

watch([documentRef,optionsRef,variablesRef],()=>{
//do logic here
})

Possibly, it could solve the asynchronous problem which you want to deal with.

@Akryum Akryum changed the title fix: useQuery bug. fix: useQuery loading and debounce issues May 3, 2022
@Akryum Akryum merged commit 082acf9 into vuejs:v4 May 3, 2022
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