-
-
Notifications
You must be signed in to change notification settings - Fork 5.4k
Update react-query depency to require at least v5.83 #10838
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
Conversation
@@ -1174,6 +1174,26 @@ const ProductList = () => ( | |||
) | |||
``` | |||
|
|||
## Enabling Data Fetching Conditionally |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure about where to put this documentation as it's actually useGetList
that we configure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's fine here 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you also prepare the changelog to mention that there is a risk of duplicate react-query, and how to fix that?
@@ -57,8 +57,7 @@ | |||
"react-router-dom": "^6.28.1 || ^7.1.1" | |||
}, | |||
"dependencies": { | |||
"@tanstack/react-query": "^5.21.7", | |||
"clsx": "^2.1.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 good catch
@@ -248,7 +248,6 @@ export type UseInfiniteGetListOptions< | |||
GetInfiniteListResult<RecordType>, | |||
ErrorType, | |||
InfiniteData<GetInfiniteListResult<RecordType>>, | |||
GetInfiniteListResult<RecordType>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does that mean that the type of UseInfiniteQueryOptions
has changed in react-query? That would be a breaking change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No I believe we used it incorrectly before but it wasn't detected until this update
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's due to TanStack/query#8956. It doesn't seem to be a breaking change.
@@ -176,7 +176,7 @@ export const useGetList = < | |||
} | |||
: result, | |||
[result] | |||
) as UseQueryResult<RecordType[], Error> & { | |||
) as unknown as UseQueryResult<RecordType[], Error> & { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question: Why do we only need to do that when calling dataProvider.getList()
and not for every other usage of useQuery
(e.g. useGetOne
) ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because useGetOne
returns the same format as react query and we made the decision to do things differently in all hooks that also return a total. Instead of data.total
we wanted to have total
directly on the result.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the record, I'd love to change that in v6
const ConditionalDataFetchingView = () => { | ||
const context = useListContext(); | ||
|
||
if (context.filterValues.q == null || context.filterValues.q === '') { | ||
return ( | ||
<CardContentInner> | ||
Type a search term to fetch data | ||
</CardContentInner> | ||
); | ||
} | ||
|
||
return <BookList />; | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This story is working, but since you are rendering a different child when the filter condition is not true, we can't really tell if the dataProvider is called or not. It would help to enable the fakerest debug logs.
docs/List.md
Outdated
context.filterValues.q == null || | ||
context.filterValues.q === '' ? ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Too bad you copied the wrong condition: in the docs the condition was q.length > 2
and not q !== ''
😅
Problem
We received some requests and questions for enabling list components data fetching conditionally.
It's actually easier to do with react-query versions more recent than the one we install by default.
Solution
How To Test
Additional Checks
master
for a bugfix or a documentation fix, ornext
for a featureAlso, please make sure to read the contributing guidelines.