-
-
Notifications
You must be signed in to change notification settings - Fork 3.4k
test(react-query/useInfiniteQuery): use precise time in 'advanceTimersByTimeAsync', and add 'expect' using 'toBeInTheDocument' #9446
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
…sByTimeAsync', and add 'expect' using 'toBeInTheDocument'
…e-time-advanceTimersByTimeAsync
View your CI Pipeline Execution ↗ for commit f2371dd
☁️ Nx Cloud last updated this comment at |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #9446 +/- ##
===========================================
+ Coverage 45.30% 84.38% +39.07%
===========================================
Files 208 26 -182
Lines 8283 365 -7918
Branches 1869 107 -1762
===========================================
- Hits 3753 308 -3445
+ Misses 4085 48 -4037
+ Partials 445 9 -436 🚀 New features to boost your workflow:
|
…e-time-advanceTimersByTimeAsync
@@ -81,7 +81,7 @@ describe('useInfiniteQuery', () => { | |||
|
|||
renderWithClient(queryClient, <Page />) | |||
|
|||
await vi.advanceTimersByTimeAsync(100) | |||
await vi.advanceTimersByTimeAsync(0) |
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.
await vi.advanceTimersByTimeAsync(0) | |
await vi.advanceTimersByTimeAsync(10) |
Could you add queryFn like below
function Page() {
const state = useInfiniteQuery({
queryKey: key,
queryFn: ({ pageParam }) => sleep(10).then(() => Number(pageParam)),
getNextPageParam: (lastPage) => lastPage + 1,
initialPageParam: 0,
})
states.push(state)
return null
}
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 reflected your opinion. 👍
… states for a successful query case
…sByTimeAsync', and add 'expect' using 'toBeInTheDocument'
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.
Much tighter and more realistic now 👍
@@ -1422,7 +1422,7 @@ describe('useInfiniteQuery', () => { | |||
function Page() { | |||
const state = useInfiniteQuery({ | |||
queryKey: key, | |||
queryFn: ({ pageParam }): number => pageParam, | |||
queryFn: ({ pageParam }) => sleep(10).then(() => pageParam), |
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.
👍
@@ -1383,7 +1383,7 @@ describe('useInfiniteQuery', () => { | |||
function Page() { | |||
const state = useInfiniteQuery({ | |||
queryKey: key, | |||
queryFn: ({ pageParam }): number => pageParam, | |||
queryFn: ({ pageParam }) => sleep(10).then(() => pageParam), |
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.
👍
const queryFn = async () => { | ||
return Promise.resolve('custom client') | ||
} | ||
const queryFn = () => sleep(10).then(() => 'custom client') |
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.
👍
@@ -335,7 +336,7 @@ describe('useInfiniteQuery', () => { | |||
function Page() { | |||
const state = useInfiniteQuery({ | |||
queryKey: key, | |||
queryFn: () => ({ count: 1 }), | |||
queryFn: () => sleep(10).then(() => ({ count: 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.
👍
@@ -71,7 +71,7 @@ describe('useInfiniteQuery', () => { | |||
function Page() { | |||
const state = useInfiniteQuery({ | |||
queryKey: key, | |||
queryFn: ({ pageParam }) => Number(pageParam), | |||
queryFn: ({ pageParam }) => sleep(10).then(() => Number(pageParam)), |
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.
queryFn: ({ pageParam }) => sleep(10).then(() => Number(pageParam)), | |
queryFn: ({ pageParam }) => sleep(10).then(() => pageParam), |
This seems to be a leftover from TanStack Query v4! Now, TanStack Query v5 has improved this type of thing and I think this part should be improved incrementally throughout the entire code.
We can remove unnecessary code like this. I would appreciate it if you could take care of this gradually too!
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 seems to be a leftover from TanStack Query v4! Now, TanStack Query v5 has improved this type of thing and I think this part should be improved incrementally throughout the entire code. We can remove unnecessary code like this. I would appreciate it if you could take care of this gradually too!
I totally agree with you. I’ll take care of this part as well in future tasks.
…e-time-advanceTimersByTimeAsync
No description provided.