[RFR] Add enabled options to query hooks

[ValentinH]

Closes #5827

description

This PR introduces the enabled option to the useQuery(), useQueryWithStore() and all the corresponding specialised hooks. The goal is to be able to conditionally make a query. It can be useful for example for a query depending on another one

// fetch posts
const { ids, data: posts, loading: isLoading } = useGetList<Post>(
    'posts',
    { page: 1, perPage: 20 },
    { field: 'name', order: 'ASC' },
    {}
);

// then fetch categories for these posts
const { data: categories, loading: isLoading } = useGetMany<Category>(
    'categories',
    ids.map(id=> posts[id].category_id),
    // run only if the first query returns non-empty result
    { enabled: ids.length > 0 }
);

open questions

• what should the return value contain when enabled: false? For now, it will contain the last value before enabled: false, i.e. the initial date on load and then the last result if enabled switches from false to true.

todos

• tests
• more documentation?

[fzaninotto]

You should implement the enabled prop only once in useDataProvider. When enabled is false, it should return an empty resolved Promise (just like for optimistic queries).

[ValentinH]

I moved the implementation to the useDataProvider hook as you suggested.

One thing I noticed and that I'm not sure I'm happy with is that when enabled: false, the inital result is:

{
  data: {}
  error: null
  ids: []
  loaded: false
  loading: true
  total: undefined
}

I think that loading should be false at this point.

Similarly, if the query was enabled at some point and then disabled, the result is:

{
  data: {...Data}
  error: null
  ids: [...Ids]
  loaded: true
  loading: false
  total: 20
}

Here, I'm wondering if loaded should be false. But I'm not sure for this use case.

Regarding the tests, where would you write them? Inside useDataProvider.spec.js or the tests for the corresponding specialised hooks?

[fzaninotto]

For the result in case of disabled query, I'm not sure this is really important. And I have no idea of what a disabled query should return. Feel free to suggest what you think is best.

As for unit tests, please add tests for the enabled options the existing useDataProvider and useMutation tests.

[fzaninotto]

use the existing UseDataProviderOptions instead

[ValentinH]

Good point, I did it as well for useGetOne().

[fzaninotto]

You should add options.enabled to the dependency list of the useEffect, as you did in usequeryWithStore

[ValentinH]

Having options.enabled in the useEffect there is actually a left-over that I forgot to remove when removing the early return in the hook. As the logic is now handled in useDataProvider and this option is part of the requestSignature, I don't think this is needed.

[ValentinH]

The only think that seems meaningful to me regarding the result is having loading: false on start. However, I agree that this is not important and can be dealt with in the application.

I'll work on the tests later today.

[ValentinH]

@fzaninotto I pushed a first test and I noticed that because we return an empty resolved Promise when enabled is false, the data provider resolves with undefined while the contract says that it should resolved with GetOneResult<Record>.

I don't think this is right.

[fzaninotto]

How do you suggest we fix this Type problem? I see that react-query types the data as optional in the response type.

[ValentinH]

This is actually not only a type problem but can lead to runtime error: see this line in the test I created. The problem is not that data is missing but that the whole result is missing.

In the case of getOne, I think we should resolve with { data: null }. However, it means that we need a special case for every data provider method which feels off.

[ValentinH]

@fzaninotto I'm not sure how to proceed with this PR, do you have any suggestions please?

[fzaninotto]

Caution: your PR needs rebase

[fzaninotto]

Excellent! Now you just have to document the new option and we're good to merge.

[ValentinH]

Documentation added.

[fzaninotto]

Thanks!