redux-toolkit icon indicating copy to clipboard operation
redux-toolkit copied to clipboard

`useQueries` hook

Open phryneas opened this issue 3 years ago • 6 comments

This is a WIP towards building a useQueries hook , per https://github.com/reduxjs/redux-toolkit/discussions/2398 .

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

phryneas avatar Jun 10 '22 20:06 phryneas

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 4e707bf867a065eef022ee2bfa38851bda82f2f4:

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

codesandbox-ci[bot] avatar Jun 10 '22 20:06 codesandbox-ci[bot]

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?

phryneas avatar Jul 22 '22 15:07 phryneas

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

TkDodo avatar Jul 22 '22 15:07 TkDodo