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

useQueries hook #2402

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

useQueries hook #2402

wants to merge 3 commits into from

Conversation

phryneas
Copy link
Member

@phryneas phryneas commented Jun 10, 2022

This is a WIP towards building a useQueries hook , per #2398 .

So far I have a slightly wonky prototype useQuerySubscriptions hook. It doesn't break any existing tests which I find kinda unsettling.

@codesandbox-ci
Copy link

codesandbox-ci bot commented Jun 10, 2022

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 4e707bf:

Sandbox Source
Vanilla Configuration
Vanilla Typescript Configuration
rsk-github-issues-example Configuration
@examples-query-react/basic Configuration
@examples-query-react/advanced Configuration
@examples-action-listener/counter Configuration

@phryneas
Copy link
Member Author

With the implementation I just pushed, it is now possible to do

const [lastPage, currentPage, nextPage] = api.endpoints.getPage.useQueries({page: 1}, {page: 2}, {page: 3})

So that's a bit of a win.

But this now shows a fundamental design flaw:

An array-based useQueries hook will (at least as I implemented it right now) lose the "lagged queries" functionality when switching between [{ page: 2}, {page:3}] to [{ page: 2}, {page: 4}] and also will always reset isLoading, just like isSuccess.

We would essentially be in need of "component keys" (to compare this to React) to figure out "which array member changed, which was inserted, which was deleted" etc.
Because who can say what switching from [{ page: 2}, {page:3}, {page: 4}] to [{ page: 2}, {page: 4}] actually means?

There are two ways I see out of that:

  • only implement that for useQuery, but not for useQueries
  • don't base useQueries on an array argument, but on a "keyed" object:
const { lastPage, currentPage, nextPage } = useQueries({ lastPage: {page: 1}, currentPage: {page:2}, nextPage: {page: 3} })

While this is more code, it might also be more readable in the end.
On the other hand, it's more difficult to just do const results = api.endpoints.getPage.useQueries(dynamicArrayOrArguments) if we go for that api design.
It would also affect #2245 and the useSuspenseAll api design since that would essentially need to have the same style as this api.

So, yeah... open to any comments you might have on this.

Actually, @TkDodo - I'd love your opinion on this. As far as I understand you also added a useQueries hook. You went for an array. Do you see drawbacks from that?

@TkDodo
Copy link

TkDodo commented Jul 22, 2022

We have query keys, so we know which observer to reuse in the next render. Lagged queries were difficult to get right though if the key changes. I think we still rely on index as a fallback. It's not perfect. I think if you change keys and change the length / order of the array at the same time and use lagged queries with keepPreviousData, it might not work. It's a pretty edgy case though

Base automatically changed from v1.9-integration to master November 4, 2022 14:06
mrkvon added a commit to solidcouch/solidcouch that referenced this pull request Apr 23, 2023
Incremental means that we show data as soon as they're discovered

We're using react-query, because useQueries is missing in rtk-query:
reduxjs/redux-toolkit#2402
mrkvon added a commit to solidcouch/solidcouch that referenced this pull request Apr 23, 2023
Incremental means that we show data as soon as they're discovered

We're using react-query, because useQueries is missing in rtk-query:
reduxjs/redux-toolkit#2402
@NunoCardoso
Copy link

Any updates on this? I really need something like that on a project I am working on

@markerikson
Copy link
Collaborator

@NunoCardoso nope, none (as evidenced by the fact that the PR has sat here for two years untouched).

I'm hoping to spend some time working on RTKQ features in the next few months, but no ETA or guarantees on which features will end up getting worked on first.

@NunoCardoso
Copy link

@NunoCardoso nope, none (as evidenced by the fact that the PR has sat here for two years untouched).

I'm hoping to spend some time working on RTKQ features in the next few months, but no ETA or guarantees on which features will end up getting worked on first.

Of course. I will use this snippet from kellengreen for now, I think it does the trick while this is still on backlog.

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.

4 participants