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

result parameter in combine function in useQueries initially empty array #6612

Closed
dhulme opened this issue Dec 29, 2023 · 1 comment · Fixed by #6614
Closed

result parameter in combine function in useQueries initially empty array #6612

dhulme opened this issue Dec 29, 2023 · 1 comment · Fixed by #6614
Labels
bug Something isn't working package: query-core

Comments

@dhulme
Copy link

dhulme commented Dec 29, 2023

Describe the bug

The result parameter in combine function in React useQueries is initially an empty array. This doesn't match the typing that says each item is a UseQueryResult.

Your minimal, reproducible example

https://codesandbox.io/p/devbox/divine-pine-l3nw5l

Steps to reproduce

Run example and look in the console, it shows an error (see screenshot below).

For an unknown reason, the code works in code sandbox, but not when I run the same code example on my machine or my colleague's machine. Please could you try and reproduce locally copying the 3 files in the example?

Expected behavior

I expect queries to be defined on first render, but I receive undefined

How often does this bug happen?

Every time

Screenshots or Videos

Error shown on Chrome on my machine:
image

Platform

  • OS - Windows 11
  • Browser - Chrome 120

Tanstack Query adapter

react-query

TanStack Query version

v5.15.3

TypeScript version

No response

Additional context

No response

@TkDodo
Copy link
Collaborator

TkDodo commented Dec 29, 2023

For an unknown reason, the code works in code sandbox, but not when I run the same code example on my machine or my colleague's machine.

not sure what's up with the sandbox, but it's somehow using an older version of react-query.

here is a fork with an update to the latest version that shows the issue:

this seems like a regression from:

I need to see if I can fix that issue differently.

@TkDodo TkDodo added bug Something isn't working package: query-core labels Dec 29, 2023
TkDodo added a commit that referenced this issue Dec 29, 2023
The initial fix for #6369 (#6431) introduced a regression where we'd always call combine with an empty array initially.

Turns out it was actually on purpose that we didn't call combine in the constructor, but delayed that until the `setQueries` call - because there, we already have complete query result structures available

However, there is an optimization that bails out early without calling this.#setResult if "nothing changed"; however, if we call useQueries with an empty array, that means nothing every changes, and the #combinedResult is actually never set, leading to structural sharing never having anything to share with, thus yielding a new reference every time (this is the root issue for #6369).

This fix reverts those changes (to avoid calling combine with an empty array every time, thus fixing #6612) and makes sure #setResult is always invoked before bailing out, so that #combinedResult is set correctly even for this case
TkDodo added a commit that referenced this issue Dec 29, 2023
we only want to stop bailing out if we have no queries; moving the #setResult call unconditionally before the bail-out leads to more re-renders for other useQueries calls

now, we only stop the bail-out when the special empty case sets in, which is what fixes #6612
TkDodo added a commit that referenced this issue Dec 29, 2023
…queries (#6614)

* fix(core): make sure to call setResult before the early bail-out

The initial fix for #6369 (#6431) introduced a regression where we'd always call combine with an empty array initially.

Turns out it was actually on purpose that we didn't call combine in the constructor, but delayed that until the `setQueries` call - because there, we already have complete query result structures available

However, there is an optimization that bails out early without calling this.#setResult if "nothing changed"; however, if we call useQueries with an empty array, that means nothing every changes, and the #combinedResult is actually never set, leading to structural sharing never having anything to share with, thus yielding a new reference every time (this is the root issue for #6369).

This fix reverts those changes (to avoid calling combine with an empty array every time, thus fixing #6612) and makes sure #setResult is always invoked before bailing out, so that #combinedResult is set correctly even for this case

* fix: add a special length check to the bail-out case

we only want to stop bailing out if we have no queries; moving the #setResult call unconditionally before the bail-out leads to more re-renders for other useQueries calls

now, we only stop the bail-out when the special empty case sets in, which is what fixes #6612
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working package: query-core
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants