Feature/infinite queries

[TkDodo]

No description provided.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment

Name	Status	Preview	Comments	Updated
query	⬜️ Ignored (Inspect)			Feb 27, 2023 at 9:34AM (UTC)

[codesandbox-ci]

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 3bb0c34:

Sandbox	Source
@tanstack/query-example-react-basic-typescript	Configuration
@tanstack/query-example-solid-basic-typescript	Configuration
@tanstack/query-example-svelte-basic	Configuration
@tanstack/query-example-vue-basic	Configuration

[codecov-commenter]

Codecov Report

Base: 90.50% // Head: 90.41% // Decreases project coverage by -0.09%⚠️

Coverage data is based on head (3bb0c34) compared to base (ca2f697).
Patch coverage: 100.00% of modified lines in pull request are covered.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files

@@ Coverage Diff @@## alpha #5004 +/- ##
==========================================
- Coverage 90.50% 90.41% -0.09% 
==========================================
Files 104 104 Lines 3811 3788 -23 Branches 961 948 -13 ==========================================
- Hits 3449 3425 -24 - Misses 329 330 +1 
Partials 33 33 

Impacted Files	Coverage Δ
packages/query-core/src/query.ts	99.02% <ø> (ø)
packages/react-query/src/useInfiniteQuery.ts	100.00% <ø> (ø)
packages/solid-query/src/createInfiniteQuery.ts	100.00% <ø> (ø)
packages/svelte-query/src/createInfiniteQuery.ts	0.00% <ø> (ø)
packages/vue-query/src/queryClient.ts	93.22% <ø> (ø)
packages/vue-query/src/useBaseQuery.ts	100.00% <ø> (ø)
packages/vue-query/src/useInfiniteQuery.ts	100.00% <ø> (ø)
packages/vue-query/src/useQuery.ts	100.00% <ø> (ø)
packages/query-core/src/infiniteQueryBehavior.ts	98.59% <100.00%> (-1.41%)	⬇️
packages/query-core/src/infiniteQueryObserver.ts	88.46% <100.00%> (-0.43%)	⬇️
... and 3 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[DamianOsipiuk]

Hmmm, do we still need pageParams in return of useInfiniteQuery? 🤔

For what it can be usefull?

[TkDodo]

@DamianOsipiuk

Hmmm, do we still need pageParams in return of useInfiniteQuery? 🤔

For what it can be usefull?

I outlined the problems in the initial RFC. We still need the pageParams to trigger refetches in some cases. As an example, suppose we have the following query:

useInfiniteQuery({
queryKey,
queryFn: ({ pageParam }) => Promise.resolve({ num: pageParam }),
defaultPageParam: 0
getNextPage: (lastPage) => lastPage.num + 1,
getNextPage: (firstPage) => firstPage.num - 1,
})

this will start out with:

{
pages: [{ num: 0 }],
pageParam: [0]
}

now let's fetch some more in the forward direction:

{
pages: [{ num: 0 }, { num: 1 }, { num: 2 }],
pageParam: [0, 1, 2]
}

so far, so good. If a refetch happens now, it always starts at the beginning and then refetches all pages in order. so we:

• take 0 from the pageParams to initiate the refetches
• for every subsequent page, we just call getNextPageParam again to re-compute it on the fly.

Now it seems, we could just re-use the defaultPageParam and start with that, but we cannot, for two reasons:

1. what if we have fetched in the previous direction before, and our state is:

{
pages: [{ num: -1 }, { num: 0 }, { num: 1 }, { num: 2 }],
pageParam: [-1, 0, 1, 2]
}

we now have to start the fetch chain with -1 and not with 0. We only have that info in pageParams.

2. in the same way, if you use the new pageSize option and set it to 2, you might be in a situation where you no longer have the "initial" page in the cache, we might only have:

{
pages: [{ num: 1 }, { num: 2 }],
pageParam: [1, 2]
}

now, we would have to start with 1.

Grabbing this info from the pageParam works well, but yeah, it's the only reason we have this extra nesting structure :/ It happens right here and is the only place where we need pageParams:

query/packages/query-core/src/infiniteQueryBehavior.ts

Line 121 in 736edbd

promise=fetchPage([],oldPageParams[0])

Alternatively, we would have to store the information "what's the pageParam of the current first page" somewhere on the query. Would that be better?
I also thought about making defaultPageParam a function that is executed with allPages | undefined, but is there a guarantee that users can always compute that value just from looking at the pages ?

[DamianOsipiuk]

@TkDodo But it seems that it's now more like an implementation detail, rather than something users could use.

If it's not usefull for users it would be good to hide this somewhere. And what we really need is something like firstPageParam internally.
Removing pageParams would make using select and cache manipulation easier.

But question is, how would we store this firstPageParam with persisters? Or do we even need to do that?

[TkDodo]

But it seems that it's now more like an implementation detail, rather than something users could use.

yes, I agree.

Removing pageParams would make using select and cache manipulation easier.

cache manipulation, yes. select, kinda. It should be possible to transform to anything via select. it works at runtime, but not on type level. I've added some failing type level tests. I think we should fix them first before changing the structure.

But question is, how would we store this firstPageParam with persisters? Or do we even need to do that?

yes, we would need to store it. it could be part of the query.state, as we store more than just the data anyways:

query/packages/query-core/src/query.ts

Lines 37 to 50 in 765f619

	exportinterfaceQueryState<TData=unknown,TError=RegisteredError>{
	data: TData|undefined
	dataUpdateCount: number
	dataUpdatedAt: number
	error: TError|null
	errorUpdateCount: number
	errorUpdatedAt: number
	fetchFailureCount: number
	fetchFailureReason: TError|null
	fetchMeta: any
	isInvalidated: boolean
	status: QueryStatus
	fetchStatus: FetchStatus
	}

I will need to look into it. We don't have an infinite version of that query. Maybe storing it on fetchMeta works, idk 😅

[TkDodo]

hmm, it's mostly an implementation detail, but when working directly with the cache, you'd still need to set it. As an example, assume you have 3 pages:

{
pages: [{}, {}, {}]
pageParams: [5, 10, 15]
}

if we take away pageParams from data and store them somewhere else (on queryState), what happens if you call setQueryData and remove the first page? You will also need to adapt pageParams so that it only contains [10, 15]. We can only do that becausepageParams are part of our Infinite Data structure. So it seems that exactly the use-cases where pageParams becomes annoying are actually the ones that might need it ...

[TkDodo]

@ardeora I'll merge this PR into alpha because it contains some good improvements on the API. If we find a way to get rid of the pageParams as part of data, we can still do this separately.

[TkDodo]

No description provided.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment

Name	Status	Preview	Comments	Updated
query	⬜️ Ignored (Inspect)			Feb 27, 2023 at 9:34AM (UTC)

[codesandbox-ci]

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 3bb0c34:

Sandbox	Source
@tanstack/query-example-react-basic-typescript	Configuration
@tanstack/query-example-solid-basic-typescript	Configuration
@tanstack/query-example-svelte-basic	Configuration
@tanstack/query-example-vue-basic	Configuration

[codecov-commenter]

Codecov Report

Base: 90.50% // Head: 90.41% // Decreases project coverage by -0.09%⚠️

Coverage data is based on head (3bb0c34) compared to base (ca2f697).
Patch coverage: 100.00% of modified lines in pull request are covered.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files

@@ Coverage Diff @@## alpha #5004 +/- ##
==========================================
- Coverage 90.50% 90.41% -0.09% 
==========================================
Files 104 104 Lines 3811 3788 -23 Branches 961 948 -13 ==========================================
- Hits 3449 3425 -24 - Misses 329 330 +1 
Partials 33 33 

Impacted Files	Coverage Δ
packages/query-core/src/query.ts	99.02% <ø> (ø)
packages/react-query/src/useInfiniteQuery.ts	100.00% <ø> (ø)
packages/solid-query/src/createInfiniteQuery.ts	100.00% <ø> (ø)
packages/svelte-query/src/createInfiniteQuery.ts	0.00% <ø> (ø)
packages/vue-query/src/queryClient.ts	93.22% <ø> (ø)
packages/vue-query/src/useBaseQuery.ts	100.00% <ø> (ø)
packages/vue-query/src/useInfiniteQuery.ts	100.00% <ø> (ø)
packages/vue-query/src/useQuery.ts	100.00% <ø> (ø)
packages/query-core/src/infiniteQueryBehavior.ts	98.59% <100.00%> (-1.41%)	⬇️
packages/query-core/src/infiniteQueryObserver.ts	88.46% <100.00%> (-0.43%)	⬇️
... and 3 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[DamianOsipiuk]

Hmmm, do we still need pageParams in return of useInfiniteQuery? 🤔

For what it can be usefull?

[TkDodo]

@DamianOsipiuk

Hmmm, do we still need pageParams in return of useInfiniteQuery? 🤔

For what it can be usefull?

I outlined the problems in the initial RFC. We still need the pageParams to trigger refetches in some cases. As an example, suppose we have the following query:

useInfiniteQuery({
queryKey,
queryFn: ({ pageParam }) => Promise.resolve({ num: pageParam }),
defaultPageParam: 0
getNextPage: (lastPage) => lastPage.num + 1,
getNextPage: (firstPage) => firstPage.num - 1,
})

this will start out with:

{
pages: [{ num: 0 }],
pageParam: [0]
}

now let's fetch some more in the forward direction:

{
pages: [{ num: 0 }, { num: 1 }, { num: 2 }],
pageParam: [0, 1, 2]
}

so far, so good. If a refetch happens now, it always starts at the beginning and then refetches all pages in order. so we:

• take 0 from the pageParams to initiate the refetches
• for every subsequent page, we just call getNextPageParam again to re-compute it on the fly.

Now it seems, we could just re-use the defaultPageParam and start with that, but we cannot, for two reasons:

1. what if we have fetched in the previous direction before, and our state is:

{
pages: [{ num: -1 }, { num: 0 }, { num: 1 }, { num: 2 }],
pageParam: [-1, 0, 1, 2]
}

we now have to start the fetch chain with -1 and not with 0. We only have that info in pageParams.

2. in the same way, if you use the new pageSize option and set it to 2, you might be in a situation where you no longer have the "initial" page in the cache, we might only have:

{
pages: [{ num: 1 }, { num: 2 }],
pageParam: [1, 2]
}

now, we would have to start with 1.

Grabbing this info from the pageParam works well, but yeah, it's the only reason we have this extra nesting structure :/ It happens right here and is the only place where we need pageParams:

query/packages/query-core/src/infiniteQueryBehavior.ts

Line 121 in 736edbd

promise=fetchPage([],oldPageParams[0])

Alternatively, we would have to store the information "what's the pageParam of the current first page" somewhere on the query. Would that be better?
I also thought about making defaultPageParam a function that is executed with allPages | undefined, but is there a guarantee that users can always compute that value just from looking at the pages ?

[DamianOsipiuk]

@TkDodo But it seems that it's now more like an implementation detail, rather than something users could use.

If it's not usefull for users it would be good to hide this somewhere. And what we really need is something like firstPageParam internally.
Removing pageParams would make using select and cache manipulation easier.

But question is, how would we store this firstPageParam with persisters? Or do we even need to do that?

[TkDodo]

But it seems that it's now more like an implementation detail, rather than something users could use.

yes, I agree.

Removing pageParams would make using select and cache manipulation easier.

cache manipulation, yes. select, kinda. It should be possible to transform to anything via select. it works at runtime, but not on type level. I've added some failing type level tests. I think we should fix them first before changing the structure.

But question is, how would we store this firstPageParam with persisters? Or do we even need to do that?

yes, we would need to store it. it could be part of the query.state, as we store more than just the data anyways:

query/packages/query-core/src/query.ts

Lines 37 to 50 in 765f619

	exportinterfaceQueryState<TData=unknown,TError=RegisteredError>{
	data: TData|undefined
	dataUpdateCount: number
	dataUpdatedAt: number
	error: TError|null
	errorUpdateCount: number
	errorUpdatedAt: number
	fetchFailureCount: number
	fetchFailureReason: TError|null
	fetchMeta: any
	isInvalidated: boolean
	status: QueryStatus
	fetchStatus: FetchStatus
	}

I will need to look into it. We don't have an infinite version of that query. Maybe storing it on fetchMeta works, idk 😅

[TkDodo]

hmm, it's mostly an implementation detail, but when working directly with the cache, you'd still need to set it. As an example, assume you have 3 pages:

{
pages: [{}, {}, {}]
pageParams: [5, 10, 15]
}

if we take away pageParams from data and store them somewhere else (on queryState), what happens if you call setQueryData and remove the first page? You will also need to adapt pageParams so that it only contains [10, 15]. We can only do that becausepageParams are part of our Infinite Data structure. So it seems that exactly the use-cases where pageParams becomes annoying are actually the ones that might need it ...

[TkDodo]

@ardeora I'll merge this PR into alpha because it contains some good improvements on the API. If we find a way to get rid of the pageParams as part of data, we can still do this separately.

[TkDodo]

No description provided.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment

Name	Status	Preview	Comments	Updated
query	⬜️ Ignored (Inspect)			Feb 27, 2023 at 9:34AM (UTC)

[codesandbox-ci]

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 3bb0c34:

Sandbox	Source
@tanstack/query-example-react-basic-typescript	Configuration
@tanstack/query-example-solid-basic-typescript	Configuration
@tanstack/query-example-svelte-basic	Configuration
@tanstack/query-example-vue-basic	Configuration

[codecov-commenter]

Codecov Report

Base: 90.50% // Head: 90.41% // Decreases project coverage by -0.09%⚠️

Coverage data is based on head (3bb0c34) compared to base (ca2f697).
Patch coverage: 100.00% of modified lines in pull request are covered.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files

@@ Coverage Diff @@## alpha #5004 +/- ##
==========================================
- Coverage 90.50% 90.41% -0.09% 
==========================================
Files 104 104 Lines 3811 3788 -23 Branches 961 948 -13 ==========================================
- Hits 3449 3425 -24 - Misses 329 330 +1 
Partials 33 33 

Impacted Files	Coverage Δ
packages/query-core/src/query.ts	99.02% <ø> (ø)
packages/react-query/src/useInfiniteQuery.ts	100.00% <ø> (ø)
packages/solid-query/src/createInfiniteQuery.ts	100.00% <ø> (ø)
packages/svelte-query/src/createInfiniteQuery.ts	0.00% <ø> (ø)
packages/vue-query/src/queryClient.ts	93.22% <ø> (ø)
packages/vue-query/src/useBaseQuery.ts	100.00% <ø> (ø)
packages/vue-query/src/useInfiniteQuery.ts	100.00% <ø> (ø)
packages/vue-query/src/useQuery.ts	100.00% <ø> (ø)
packages/query-core/src/infiniteQueryBehavior.ts	98.59% <100.00%> (-1.41%)	⬇️
packages/query-core/src/infiniteQueryObserver.ts	88.46% <100.00%> (-0.43%)	⬇️
... and 3 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[DamianOsipiuk]

Hmmm, do we still need pageParams in return of useInfiniteQuery? 🤔

For what it can be usefull?

[TkDodo]

@DamianOsipiuk

Hmmm, do we still need pageParams in return of useInfiniteQuery? 🤔

For what it can be usefull?

I outlined the problems in the initial RFC. We still need the pageParams to trigger refetches in some cases. As an example, suppose we have the following query:

useInfiniteQuery({
queryKey,
queryFn: ({ pageParam }) => Promise.resolve({ num: pageParam }),
defaultPageParam: 0
getNextPage: (lastPage) => lastPage.num + 1,
getNextPage: (firstPage) => firstPage.num - 1,
})

this will start out with:

{
pages: [{ num: 0 }],
pageParam: [0]
}

now let's fetch some more in the forward direction:

{
pages: [{ num: 0 }, { num: 1 }, { num: 2 }],
pageParam: [0, 1, 2]
}

so far, so good. If a refetch happens now, it always starts at the beginning and then refetches all pages in order. so we:

• take 0 from the pageParams to initiate the refetches
• for every subsequent page, we just call getNextPageParam again to re-compute it on the fly.

Now it seems, we could just re-use the defaultPageParam and start with that, but we cannot, for two reasons:

1. what if we have fetched in the previous direction before, and our state is:

{
pages: [{ num: -1 }, { num: 0 }, { num: 1 }, { num: 2 }],
pageParam: [-1, 0, 1, 2]
}

we now have to start the fetch chain with -1 and not with 0. We only have that info in pageParams.

2. in the same way, if you use the new pageSize option and set it to 2, you might be in a situation where you no longer have the "initial" page in the cache, we might only have:

{
pages: [{ num: 1 }, { num: 2 }],
pageParam: [1, 2]
}

now, we would have to start with 1.

Grabbing this info from the pageParam works well, but yeah, it's the only reason we have this extra nesting structure :/ It happens right here and is the only place where we need pageParams:

query/packages/query-core/src/infiniteQueryBehavior.ts

Line 121 in 736edbd

promise=fetchPage([],oldPageParams[0])

Alternatively, we would have to store the information "what's the pageParam of the current first page" somewhere on the query. Would that be better?
I also thought about making defaultPageParam a function that is executed with allPages | undefined, but is there a guarantee that users can always compute that value just from looking at the pages ?

[DamianOsipiuk]

@TkDodo But it seems that it's now more like an implementation detail, rather than something users could use.

If it's not usefull for users it would be good to hide this somewhere. And what we really need is something like firstPageParam internally.
Removing pageParams would make using select and cache manipulation easier.

But question is, how would we store this firstPageParam with persisters? Or do we even need to do that?

[TkDodo]

But it seems that it's now more like an implementation detail, rather than something users could use.

yes, I agree.

Removing pageParams would make using select and cache manipulation easier.

cache manipulation, yes. select, kinda. It should be possible to transform to anything via select. it works at runtime, but not on type level. I've added some failing type level tests. I think we should fix them first before changing the structure.

But question is, how would we store this firstPageParam with persisters? Or do we even need to do that?

yes, we would need to store it. it could be part of the query.state, as we store more than just the data anyways:

query/packages/query-core/src/query.ts

Lines 37 to 50 in 765f619

	exportinterfaceQueryState<TData=unknown,TError=RegisteredError>{
	data: TData|undefined
	dataUpdateCount: number
	dataUpdatedAt: number
	error: TError|null
	errorUpdateCount: number
	errorUpdatedAt: number
	fetchFailureCount: number
	fetchFailureReason: TError|null
	fetchMeta: any
	isInvalidated: boolean
	status: QueryStatus
	fetchStatus: FetchStatus
	}

I will need to look into it. We don't have an infinite version of that query. Maybe storing it on fetchMeta works, idk 😅

[TkDodo]

hmm, it's mostly an implementation detail, but when working directly with the cache, you'd still need to set it. As an example, assume you have 3 pages:

{
pages: [{}, {}, {}]
pageParams: [5, 10, 15]
}

if we take away pageParams from data and store them somewhere else (on queryState), what happens if you call setQueryData and remove the first page? You will also need to adapt pageParams so that it only contains [10, 15]. We can only do that becausepageParams are part of our Infinite Data structure. So it seems that exactly the use-cases where pageParams becomes annoying are actually the ones that might need it ...

[TkDodo]

@ardeora I'll merge this PR into alpha because it contains some good improvements on the API. If we find a way to get rid of the pageParams as part of data, we can still do this separately.

[TkDodo]

No description provided.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment

Name	Status	Preview	Comments	Updated
query	⬜️ Ignored (Inspect)			Feb 27, 2023 at 9:34AM (UTC)

[codesandbox-ci]

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 3bb0c34:

Sandbox	Source
@tanstack/query-example-react-basic-typescript	Configuration
@tanstack/query-example-solid-basic-typescript	Configuration
@tanstack/query-example-svelte-basic	Configuration
@tanstack/query-example-vue-basic	Configuration

[codecov-commenter]

Codecov Report

Base: 90.50% // Head: 90.41% // Decreases project coverage by -0.09%⚠️

Coverage data is based on head (3bb0c34) compared to base (ca2f697).
Patch coverage: 100.00% of modified lines in pull request are covered.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files

@@ Coverage Diff @@## alpha #5004 +/- ##
==========================================
- Coverage 90.50% 90.41% -0.09% 
==========================================
Files 104 104 Lines 3811 3788 -23 Branches 961 948 -13 ==========================================
- Hits 3449 3425 -24 - Misses 329 330 +1 
Partials 33 33 

Impacted Files	Coverage Δ
packages/query-core/src/query.ts	99.02% <ø> (ø)
packages/react-query/src/useInfiniteQuery.ts	100.00% <ø> (ø)
packages/solid-query/src/createInfiniteQuery.ts	100.00% <ø> (ø)
packages/svelte-query/src/createInfiniteQuery.ts	0.00% <ø> (ø)
packages/vue-query/src/queryClient.ts	93.22% <ø> (ø)
packages/vue-query/src/useBaseQuery.ts	100.00% <ø> (ø)
packages/vue-query/src/useInfiniteQuery.ts	100.00% <ø> (ø)
packages/vue-query/src/useQuery.ts	100.00% <ø> (ø)
packages/query-core/src/infiniteQueryBehavior.ts	98.59% <100.00%> (-1.41%)	⬇️
packages/query-core/src/infiniteQueryObserver.ts	88.46% <100.00%> (-0.43%)	⬇️
... and 3 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[DamianOsipiuk]

Hmmm, do we still need pageParams in return of useInfiniteQuery? 🤔

For what it can be usefull?

[TkDodo]

@DamianOsipiuk

Hmmm, do we still need pageParams in return of useInfiniteQuery? 🤔

For what it can be usefull?

I outlined the problems in the initial RFC. We still need the pageParams to trigger refetches in some cases. As an example, suppose we have the following query:

useInfiniteQuery({
queryKey,
queryFn: ({ pageParam }) => Promise.resolve({ num: pageParam }),
defaultPageParam: 0
getNextPage: (lastPage) => lastPage.num + 1,
getNextPage: (firstPage) => firstPage.num - 1,
})

this will start out with:

{
pages: [{ num: 0 }],
pageParam: [0]
}

now let's fetch some more in the forward direction:

{
pages: [{ num: 0 }, { num: 1 }, { num: 2 }],
pageParam: [0, 1, 2]
}

so far, so good. If a refetch happens now, it always starts at the beginning and then refetches all pages in order. so we:

• take 0 from the pageParams to initiate the refetches
• for every subsequent page, we just call getNextPageParam again to re-compute it on the fly.

Now it seems, we could just re-use the defaultPageParam and start with that, but we cannot, for two reasons:

1. what if we have fetched in the previous direction before, and our state is:

{
pages: [{ num: -1 }, { num: 0 }, { num: 1 }, { num: 2 }],
pageParam: [-1, 0, 1, 2]
}

we now have to start the fetch chain with -1 and not with 0. We only have that info in pageParams.

2. in the same way, if you use the new pageSize option and set it to 2, you might be in a situation where you no longer have the "initial" page in the cache, we might only have:

{
pages: [{ num: 1 }, { num: 2 }],
pageParam: [1, 2]
}

now, we would have to start with 1.

Grabbing this info from the pageParam works well, but yeah, it's the only reason we have this extra nesting structure :/ It happens right here and is the only place where we need pageParams:

query/packages/query-core/src/infiniteQueryBehavior.ts

Line 121 in 736edbd

promise=fetchPage([],oldPageParams[0])

Alternatively, we would have to store the information "what's the pageParam of the current first page" somewhere on the query. Would that be better?
I also thought about making defaultPageParam a function that is executed with allPages | undefined, but is there a guarantee that users can always compute that value just from looking at the pages ?

[DamianOsipiuk]

@TkDodo But it seems that it's now more like an implementation detail, rather than something users could use.

If it's not usefull for users it would be good to hide this somewhere. And what we really need is something like firstPageParam internally.
Removing pageParams would make using select and cache manipulation easier.

But question is, how would we store this firstPageParam with persisters? Or do we even need to do that?

[TkDodo]

But it seems that it's now more like an implementation detail, rather than something users could use.

yes, I agree.

Removing pageParams would make using select and cache manipulation easier.

cache manipulation, yes. select, kinda. It should be possible to transform to anything via select. it works at runtime, but not on type level. I've added some failing type level tests. I think we should fix them first before changing the structure.

But question is, how would we store this firstPageParam with persisters? Or do we even need to do that?

yes, we would need to store it. it could be part of the query.state, as we store more than just the data anyways:

query/packages/query-core/src/query.ts

Lines 37 to 50 in 765f619

	exportinterfaceQueryState<TData=unknown,TError=RegisteredError>{
	data: TData|undefined
	dataUpdateCount: number
	dataUpdatedAt: number
	error: TError|null
	errorUpdateCount: number
	errorUpdatedAt: number
	fetchFailureCount: number
	fetchFailureReason: TError|null
	fetchMeta: any
	isInvalidated: boolean
	status: QueryStatus
	fetchStatus: FetchStatus
	}

I will need to look into it. We don't have an infinite version of that query. Maybe storing it on fetchMeta works, idk 😅

[TkDodo]

hmm, it's mostly an implementation detail, but when working directly with the cache, you'd still need to set it. As an example, assume you have 3 pages:

{
pages: [{}, {}, {}]
pageParams: [5, 10, 15]
}

if we take away pageParams from data and store them somewhere else (on queryState), what happens if you call setQueryData and remove the first page? You will also need to adapt pageParams so that it only contains [10, 15]. We can only do that becausepageParams are part of our Infinite Data structure. So it seems that exactly the use-cases where pageParams becomes annoying are actually the ones that might need it ...

[TkDodo]

@ardeora I'll merge this PR into alpha because it contains some good improvements on the API. If we find a way to get rid of the pageParams as part of data, we can still do this separately.

[TkDodo]

No description provided.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment

Name	Status	Preview	Comments	Updated
query	⬜️ Ignored (Inspect)			Feb 27, 2023 at 9:34AM (UTC)

[codesandbox-ci]

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 3bb0c34:

Sandbox	Source
@tanstack/query-example-react-basic-typescript	Configuration
@tanstack/query-example-solid-basic-typescript	Configuration
@tanstack/query-example-svelte-basic	Configuration
@tanstack/query-example-vue-basic	Configuration

[codecov-commenter]

Codecov Report

Base: 90.50% // Head: 90.41% // Decreases project coverage by -0.09%⚠️

Coverage data is based on head (3bb0c34) compared to base (ca2f697).
Patch coverage: 100.00% of modified lines in pull request are covered.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files

@@ Coverage Diff @@## alpha #5004 +/- ##
==========================================
- Coverage 90.50% 90.41% -0.09% 
==========================================
Files 104 104 Lines 3811 3788 -23 Branches 961 948 -13 ==========================================
- Hits 3449 3425 -24 - Misses 329 330 +1 
Partials 33 33 

Impacted Files	Coverage Δ
packages/query-core/src/query.ts	99.02% <ø> (ø)
packages/react-query/src/useInfiniteQuery.ts	100.00% <ø> (ø)
packages/solid-query/src/createInfiniteQuery.ts	100.00% <ø> (ø)
packages/svelte-query/src/createInfiniteQuery.ts	0.00% <ø> (ø)
packages/vue-query/src/queryClient.ts	93.22% <ø> (ø)
packages/vue-query/src/useBaseQuery.ts	100.00% <ø> (ø)
packages/vue-query/src/useInfiniteQuery.ts	100.00% <ø> (ø)
packages/vue-query/src/useQuery.ts	100.00% <ø> (ø)
packages/query-core/src/infiniteQueryBehavior.ts	98.59% <100.00%> (-1.41%)	⬇️
packages/query-core/src/infiniteQueryObserver.ts	88.46% <100.00%> (-0.43%)	⬇️
... and 3 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[DamianOsipiuk]

Hmmm, do we still need pageParams in return of useInfiniteQuery? 🤔

For what it can be usefull?

[TkDodo]

@DamianOsipiuk

Hmmm, do we still need pageParams in return of useInfiniteQuery? 🤔

For what it can be usefull?

I outlined the problems in the initial RFC. We still need the pageParams to trigger refetches in some cases. As an example, suppose we have the following query:

useInfiniteQuery({
queryKey,
queryFn: ({ pageParam }) => Promise.resolve({ num: pageParam }),
defaultPageParam: 0
getNextPage: (lastPage) => lastPage.num + 1,
getNextPage: (firstPage) => firstPage.num - 1,
})

this will start out with:

{
pages: [{ num: 0 }],
pageParam: [0]
}

now let's fetch some more in the forward direction:

{
pages: [{ num: 0 }, { num: 1 }, { num: 2 }],
pageParam: [0, 1, 2]
}

so far, so good. If a refetch happens now, it always starts at the beginning and then refetches all pages in order. so we:

• take 0 from the pageParams to initiate the refetches
• for every subsequent page, we just call getNextPageParam again to re-compute it on the fly.

Now it seems, we could just re-use the defaultPageParam and start with that, but we cannot, for two reasons:

1. what if we have fetched in the previous direction before, and our state is:

{
pages: [{ num: -1 }, { num: 0 }, { num: 1 }, { num: 2 }],
pageParam: [-1, 0, 1, 2]
}

we now have to start the fetch chain with -1 and not with 0. We only have that info in pageParams.

2. in the same way, if you use the new pageSize option and set it to 2, you might be in a situation where you no longer have the "initial" page in the cache, we might only have:

{
pages: [{ num: 1 }, { num: 2 }],
pageParam: [1, 2]
}

now, we would have to start with 1.

Grabbing this info from the pageParam works well, but yeah, it's the only reason we have this extra nesting structure :/ It happens right here and is the only place where we need pageParams:

query/packages/query-core/src/infiniteQueryBehavior.ts

Line 121 in 736edbd

promise=fetchPage([],oldPageParams[0])

Alternatively, we would have to store the information "what's the pageParam of the current first page" somewhere on the query. Would that be better?
I also thought about making defaultPageParam a function that is executed with allPages | undefined, but is there a guarantee that users can always compute that value just from looking at the pages ?

[DamianOsipiuk]

@TkDodo But it seems that it's now more like an implementation detail, rather than something users could use.

If it's not usefull for users it would be good to hide this somewhere. And what we really need is something like firstPageParam internally.
Removing pageParams would make using select and cache manipulation easier.

But question is, how would we store this firstPageParam with persisters? Or do we even need to do that?

[TkDodo]

But it seems that it's now more like an implementation detail, rather than something users could use.

yes, I agree.

Removing pageParams would make using select and cache manipulation easier.

cache manipulation, yes. select, kinda. It should be possible to transform to anything via select. it works at runtime, but not on type level. I've added some failing type level tests. I think we should fix them first before changing the structure.

But question is, how would we store this firstPageParam with persisters? Or do we even need to do that?

yes, we would need to store it. it could be part of the query.state, as we store more than just the data anyways:

query/packages/query-core/src/query.ts

Lines 37 to 50 in 765f619

	exportinterfaceQueryState<TData=unknown,TError=RegisteredError>{
	data: TData|undefined
	dataUpdateCount: number
	dataUpdatedAt: number
	error: TError|null
	errorUpdateCount: number
	errorUpdatedAt: number
	fetchFailureCount: number
	fetchFailureReason: TError|null
	fetchMeta: any
	isInvalidated: boolean
	status: QueryStatus
	fetchStatus: FetchStatus
	}

I will need to look into it. We don't have an infinite version of that query. Maybe storing it on fetchMeta works, idk 😅

[TkDodo]

hmm, it's mostly an implementation detail, but when working directly with the cache, you'd still need to set it. As an example, assume you have 3 pages:

{
pages: [{}, {}, {}]
pageParams: [5, 10, 15]
}

if we take away pageParams from data and store them somewhere else (on queryState), what happens if you call setQueryData and remove the first page? You will also need to adapt pageParams so that it only contains [10, 15]. We can only do that becausepageParams are part of our Infinite Data structure. So it seems that exactly the use-cases where pageParams becomes annoying are actually the ones that might need it ...

[TkDodo]

@ardeora I'll merge this PR into alpha because it contains some good improvements on the API. If we find a way to get rid of the pageParams as part of data, we can still do this separately.

[TkDodo]

No description provided.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment

Name	Status	Preview	Comments	Updated
query	⬜️ Ignored (Inspect)			Feb 27, 2023 at 9:34AM (UTC)

[codesandbox-ci]

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 3bb0c34:

Sandbox	Source
@tanstack/query-example-react-basic-typescript	Configuration
@tanstack/query-example-solid-basic-typescript	Configuration
@tanstack/query-example-svelte-basic	Configuration
@tanstack/query-example-vue-basic	Configuration

[codecov-commenter]

Codecov Report

Base: 90.50% // Head: 90.41% // Decreases project coverage by -0.09%⚠️

Coverage data is based on head (3bb0c34) compared to base (ca2f697).
Patch coverage: 100.00% of modified lines in pull request are covered.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files

@@ Coverage Diff @@## alpha #5004 +/- ##
==========================================
- Coverage 90.50% 90.41% -0.09% 
==========================================
Files 104 104 Lines 3811 3788 -23 Branches 961 948 -13 ==========================================
- Hits 3449 3425 -24 - Misses 329 330 +1 
Partials 33 33 

Impacted Files	Coverage Δ
packages/query-core/src/query.ts	99.02% <ø> (ø)
packages/react-query/src/useInfiniteQuery.ts	100.00% <ø> (ø)
packages/solid-query/src/createInfiniteQuery.ts	100.00% <ø> (ø)
packages/svelte-query/src/createInfiniteQuery.ts	0.00% <ø> (ø)
packages/vue-query/src/queryClient.ts	93.22% <ø> (ø)
packages/vue-query/src/useBaseQuery.ts	100.00% <ø> (ø)
packages/vue-query/src/useInfiniteQuery.ts	100.00% <ø> (ø)
packages/vue-query/src/useQuery.ts	100.00% <ø> (ø)
packages/query-core/src/infiniteQueryBehavior.ts	98.59% <100.00%> (-1.41%)	⬇️
packages/query-core/src/infiniteQueryObserver.ts	88.46% <100.00%> (-0.43%)	⬇️
... and 3 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[DamianOsipiuk]

Hmmm, do we still need pageParams in return of useInfiniteQuery? 🤔

For what it can be usefull?

[TkDodo]

@DamianOsipiuk

Hmmm, do we still need pageParams in return of useInfiniteQuery? 🤔

For what it can be usefull?

I outlined the problems in the initial RFC. We still need the pageParams to trigger refetches in some cases. As an example, suppose we have the following query:

useInfiniteQuery({
queryKey,
queryFn: ({ pageParam }) => Promise.resolve({ num: pageParam }),
defaultPageParam: 0
getNextPage: (lastPage) => lastPage.num + 1,
getNextPage: (firstPage) => firstPage.num - 1,
})

this will start out with:

{
pages: [{ num: 0 }],
pageParam: [0]
}

now let's fetch some more in the forward direction:

{
pages: [{ num: 0 }, { num: 1 }, { num: 2 }],
pageParam: [0, 1, 2]
}

so far, so good. If a refetch happens now, it always starts at the beginning and then refetches all pages in order. so we:

• take 0 from the pageParams to initiate the refetches
• for every subsequent page, we just call getNextPageParam again to re-compute it on the fly.

Now it seems, we could just re-use the defaultPageParam and start with that, but we cannot, for two reasons:

1. what if we have fetched in the previous direction before, and our state is:

{
pages: [{ num: -1 }, { num: 0 }, { num: 1 }, { num: 2 }],
pageParam: [-1, 0, 1, 2]
}

we now have to start the fetch chain with -1 and not with 0. We only have that info in pageParams.

2. in the same way, if you use the new pageSize option and set it to 2, you might be in a situation where you no longer have the "initial" page in the cache, we might only have:

{
pages: [{ num: 1 }, { num: 2 }],
pageParam: [1, 2]
}

now, we would have to start with 1.

Grabbing this info from the pageParam works well, but yeah, it's the only reason we have this extra nesting structure :/ It happens right here and is the only place where we need pageParams:

query/packages/query-core/src/infiniteQueryBehavior.ts

Line 121 in 736edbd

promise=fetchPage([],oldPageParams[0])

Alternatively, we would have to store the information "what's the pageParam of the current first page" somewhere on the query. Would that be better?
I also thought about making defaultPageParam a function that is executed with allPages | undefined, but is there a guarantee that users can always compute that value just from looking at the pages ?

[DamianOsipiuk]

@TkDodo But it seems that it's now more like an implementation detail, rather than something users could use.

If it's not usefull for users it would be good to hide this somewhere. And what we really need is something like firstPageParam internally.
Removing pageParams would make using select and cache manipulation easier.

But question is, how would we store this firstPageParam with persisters? Or do we even need to do that?

[TkDodo]

But it seems that it's now more like an implementation detail, rather than something users could use.

yes, I agree.

Removing pageParams would make using select and cache manipulation easier.

cache manipulation, yes. select, kinda. It should be possible to transform to anything via select. it works at runtime, but not on type level. I've added some failing type level tests. I think we should fix them first before changing the structure.

But question is, how would we store this firstPageParam with persisters? Or do we even need to do that?

yes, we would need to store it. it could be part of the query.state, as we store more than just the data anyways:

query/packages/query-core/src/query.ts

Lines 37 to 50 in 765f619

	exportinterfaceQueryState<TData=unknown,TError=RegisteredError>{
	data: TData|undefined
	dataUpdateCount: number
	dataUpdatedAt: number
	error: TError|null
	errorUpdateCount: number
	errorUpdatedAt: number
	fetchFailureCount: number
	fetchFailureReason: TError|null
	fetchMeta: any
	isInvalidated: boolean
	status: QueryStatus
	fetchStatus: FetchStatus
	}

I will need to look into it. We don't have an infinite version of that query. Maybe storing it on fetchMeta works, idk 😅

[TkDodo]

hmm, it's mostly an implementation detail, but when working directly with the cache, you'd still need to set it. As an example, assume you have 3 pages:

{
pages: [{}, {}, {}]
pageParams: [5, 10, 15]
}

if we take away pageParams from data and store them somewhere else (on queryState), what happens if you call setQueryData and remove the first page? You will also need to adapt pageParams so that it only contains [10, 15]. We can only do that becausepageParams are part of our Infinite Data structure. So it seems that exactly the use-cases where pageParams becomes annoying are actually the ones that might need it ...

[TkDodo]

@ardeora I'll merge this PR into alpha because it contains some good improvements on the API. If we find a way to get rid of the pageParams as part of data, we can still do this separately.

[TkDodo]

No description provided.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment

Name	Status	Preview	Comments	Updated
query	⬜️ Ignored (Inspect)			Feb 27, 2023 at 9:34AM (UTC)

[codesandbox-ci]

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 3bb0c34:

Sandbox	Source
@tanstack/query-example-react-basic-typescript	Configuration
@tanstack/query-example-solid-basic-typescript	Configuration
@tanstack/query-example-svelte-basic	Configuration
@tanstack/query-example-vue-basic	Configuration

[codecov-commenter]

Codecov Report

Base: 90.50% // Head: 90.41% // Decreases project coverage by -0.09%⚠️

Coverage data is based on head (3bb0c34) compared to base (ca2f697).
Patch coverage: 100.00% of modified lines in pull request are covered.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files

@@ Coverage Diff @@## alpha #5004 +/- ##
==========================================
- Coverage 90.50% 90.41% -0.09% 
==========================================
Files 104 104 Lines 3811 3788 -23 Branches 961 948 -13 ==========================================
- Hits 3449 3425 -24 - Misses 329 330 +1 
Partials 33 33 

Impacted Files	Coverage Δ
packages/query-core/src/query.ts	99.02% <ø> (ø)
packages/react-query/src/useInfiniteQuery.ts	100.00% <ø> (ø)
packages/solid-query/src/createInfiniteQuery.ts	100.00% <ø> (ø)
packages/svelte-query/src/createInfiniteQuery.ts	0.00% <ø> (ø)
packages/vue-query/src/queryClient.ts	93.22% <ø> (ø)
packages/vue-query/src/useBaseQuery.ts	100.00% <ø> (ø)
packages/vue-query/src/useInfiniteQuery.ts	100.00% <ø> (ø)
packages/vue-query/src/useQuery.ts	100.00% <ø> (ø)
packages/query-core/src/infiniteQueryBehavior.ts	98.59% <100.00%> (-1.41%)	⬇️
packages/query-core/src/infiniteQueryObserver.ts	88.46% <100.00%> (-0.43%)	⬇️
... and 3 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[DamianOsipiuk]

Hmmm, do we still need pageParams in return of useInfiniteQuery? 🤔

For what it can be usefull?

[TkDodo]

@DamianOsipiuk

Hmmm, do we still need pageParams in return of useInfiniteQuery? 🤔

For what it can be usefull?

I outlined the problems in the initial RFC. We still need the pageParams to trigger refetches in some cases. As an example, suppose we have the following query:

useInfiniteQuery({
queryKey,
queryFn: ({ pageParam }) => Promise.resolve({ num: pageParam }),
defaultPageParam: 0
getNextPage: (lastPage) => lastPage.num + 1,
getNextPage: (firstPage) => firstPage.num - 1,
})

this will start out with:

{
pages: [{ num: 0 }],
pageParam: [0]
}

now let's fetch some more in the forward direction:

{
pages: [{ num: 0 }, { num: 1 }, { num: 2 }],
pageParam: [0, 1, 2]
}

so far, so good. If a refetch happens now, it always starts at the beginning and then refetches all pages in order. so we:

• take 0 from the pageParams to initiate the refetches
• for every subsequent page, we just call getNextPageParam again to re-compute it on the fly.

Now it seems, we could just re-use the defaultPageParam and start with that, but we cannot, for two reasons:

1. what if we have fetched in the previous direction before, and our state is:

{
pages: [{ num: -1 }, { num: 0 }, { num: 1 }, { num: 2 }],
pageParam: [-1, 0, 1, 2]
}

we now have to start the fetch chain with -1 and not with 0. We only have that info in pageParams.

2. in the same way, if you use the new pageSize option and set it to 2, you might be in a situation where you no longer have the "initial" page in the cache, we might only have:

{
pages: [{ num: 1 }, { num: 2 }],
pageParam: [1, 2]
}

now, we would have to start with 1.

Grabbing this info from the pageParam works well, but yeah, it's the only reason we have this extra nesting structure :/ It happens right here and is the only place where we need pageParams:

query/packages/query-core/src/infiniteQueryBehavior.ts

Line 121 in 736edbd

promise=fetchPage([],oldPageParams[0])

Alternatively, we would have to store the information "what's the pageParam of the current first page" somewhere on the query. Would that be better?
I also thought about making defaultPageParam a function that is executed with allPages | undefined, but is there a guarantee that users can always compute that value just from looking at the pages ?

[DamianOsipiuk]

@TkDodo But it seems that it's now more like an implementation detail, rather than something users could use.

If it's not usefull for users it would be good to hide this somewhere. And what we really need is something like firstPageParam internally.
Removing pageParams would make using select and cache manipulation easier.

But question is, how would we store this firstPageParam with persisters? Or do we even need to do that?

[TkDodo]

But it seems that it's now more like an implementation detail, rather than something users could use.

yes, I agree.

Removing pageParams would make using select and cache manipulation easier.

cache manipulation, yes. select, kinda. It should be possible to transform to anything via select. it works at runtime, but not on type level. I've added some failing type level tests. I think we should fix them first before changing the structure.

But question is, how would we store this firstPageParam with persisters? Or do we even need to do that?

yes, we would need to store it. it could be part of the query.state, as we store more than just the data anyways:

query/packages/query-core/src/query.ts

Lines 37 to 50 in 765f619

	exportinterfaceQueryState<TData=unknown,TError=RegisteredError>{
	data: TData|undefined
	dataUpdateCount: number
	dataUpdatedAt: number
	error: TError|null
	errorUpdateCount: number
	errorUpdatedAt: number
	fetchFailureCount: number
	fetchFailureReason: TError|null
	fetchMeta: any
	isInvalidated: boolean
	status: QueryStatus
	fetchStatus: FetchStatus
	}

I will need to look into it. We don't have an infinite version of that query. Maybe storing it on fetchMeta works, idk 😅

[TkDodo]

hmm, it's mostly an implementation detail, but when working directly with the cache, you'd still need to set it. As an example, assume you have 3 pages:

{
pages: [{}, {}, {}]
pageParams: [5, 10, 15]
}

if we take away pageParams from data and store them somewhere else (on queryState), what happens if you call setQueryData and remove the first page? You will also need to adapt pageParams so that it only contains [10, 15]. We can only do that becausepageParams are part of our Infinite Data structure. So it seems that exactly the use-cases where pageParams becomes annoying are actually the ones that might need it ...

[TkDodo]

@ardeora I'll merge this PR into alpha because it contains some good improvements on the API. If we find a way to get rid of the pageParams as part of data, we can still do this separately.

[TkDodo]

No description provided.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment

Name	Status	Preview	Comments	Updated
query	⬜️ Ignored (Inspect)			Feb 27, 2023 at 9:34AM (UTC)

[codesandbox-ci]

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 3bb0c34:

Sandbox	Source
@tanstack/query-example-react-basic-typescript	Configuration
@tanstack/query-example-solid-basic-typescript	Configuration
@tanstack/query-example-svelte-basic	Configuration
@tanstack/query-example-vue-basic	Configuration

[codecov-commenter]

Codecov Report

Base: 90.50% // Head: 90.41% // Decreases project coverage by -0.09%⚠️

Coverage data is based on head (3bb0c34) compared to base (ca2f697).
Patch coverage: 100.00% of modified lines in pull request are covered.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files

@@ Coverage Diff @@## alpha #5004 +/- ##
==========================================
- Coverage 90.50% 90.41% -0.09% 
==========================================
Files 104 104 Lines 3811 3788 -23 Branches 961 948 -13 ==========================================
- Hits 3449 3425 -24 - Misses 329 330 +1 
Partials 33 33 

Impacted Files	Coverage Δ
packages/query-core/src/query.ts	99.02% <ø> (ø)
packages/react-query/src/useInfiniteQuery.ts	100.00% <ø> (ø)
packages/solid-query/src/createInfiniteQuery.ts	100.00% <ø> (ø)
packages/svelte-query/src/createInfiniteQuery.ts	0.00% <ø> (ø)
packages/vue-query/src/queryClient.ts	93.22% <ø> (ø)
packages/vue-query/src/useBaseQuery.ts	100.00% <ø> (ø)
packages/vue-query/src/useInfiniteQuery.ts	100.00% <ø> (ø)
packages/vue-query/src/useQuery.ts	100.00% <ø> (ø)
packages/query-core/src/infiniteQueryBehavior.ts	98.59% <100.00%> (-1.41%)	⬇️
packages/query-core/src/infiniteQueryObserver.ts	88.46% <100.00%> (-0.43%)	⬇️
... and 3 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[DamianOsipiuk]

Hmmm, do we still need pageParams in return of useInfiniteQuery? 🤔

For what it can be usefull?

[TkDodo]

@DamianOsipiuk

Hmmm, do we still need pageParams in return of useInfiniteQuery? 🤔

For what it can be usefull?

I outlined the problems in the initial RFC. We still need the pageParams to trigger refetches in some cases. As an example, suppose we have the following query:

useInfiniteQuery({
queryKey,
queryFn: ({ pageParam }) => Promise.resolve({ num: pageParam }),
defaultPageParam: 0
getNextPage: (lastPage) => lastPage.num + 1,
getNextPage: (firstPage) => firstPage.num - 1,
})

this will start out with:

{
pages: [{ num: 0 }],
pageParam: [0]
}

now let's fetch some more in the forward direction:

{
pages: [{ num: 0 }, { num: 1 }, { num: 2 }],
pageParam: [0, 1, 2]
}

so far, so good. If a refetch happens now, it always starts at the beginning and then refetches all pages in order. so we:

• take 0 from the pageParams to initiate the refetches
• for every subsequent page, we just call getNextPageParam again to re-compute it on the fly.

Now it seems, we could just re-use the defaultPageParam and start with that, but we cannot, for two reasons:

1. what if we have fetched in the previous direction before, and our state is:

{
pages: [{ num: -1 }, { num: 0 }, { num: 1 }, { num: 2 }],
pageParam: [-1, 0, 1, 2]
}

we now have to start the fetch chain with -1 and not with 0. We only have that info in pageParams.

2. in the same way, if you use the new pageSize option and set it to 2, you might be in a situation where you no longer have the "initial" page in the cache, we might only have:

{
pages: [{ num: 1 }, { num: 2 }],
pageParam: [1, 2]
}

now, we would have to start with 1.

Grabbing this info from the pageParam works well, but yeah, it's the only reason we have this extra nesting structure :/ It happens right here and is the only place where we need pageParams:

query/packages/query-core/src/infiniteQueryBehavior.ts

Line 121 in 736edbd

promise=fetchPage([],oldPageParams[0])

Alternatively, we would have to store the information "what's the pageParam of the current first page" somewhere on the query. Would that be better?
I also thought about making defaultPageParam a function that is executed with allPages | undefined, but is there a guarantee that users can always compute that value just from looking at the pages ?

[DamianOsipiuk]

@TkDodo But it seems that it's now more like an implementation detail, rather than something users could use.

If it's not usefull for users it would be good to hide this somewhere. And what we really need is something like firstPageParam internally.
Removing pageParams would make using select and cache manipulation easier.

But question is, how would we store this firstPageParam with persisters? Or do we even need to do that?

[TkDodo]

But it seems that it's now more like an implementation detail, rather than something users could use.

yes, I agree.

Removing pageParams would make using select and cache manipulation easier.

cache manipulation, yes. select, kinda. It should be possible to transform to anything via select. it works at runtime, but not on type level. I've added some failing type level tests. I think we should fix them first before changing the structure.

But question is, how would we store this firstPageParam with persisters? Or do we even need to do that?

yes, we would need to store it. it could be part of the query.state, as we store more than just the data anyways:

query/packages/query-core/src/query.ts

Lines 37 to 50 in 765f619

	exportinterfaceQueryState<TData=unknown,TError=RegisteredError>{
	data: TData|undefined
	dataUpdateCount: number
	dataUpdatedAt: number
	error: TError|null
	errorUpdateCount: number
	errorUpdatedAt: number
	fetchFailureCount: number
	fetchFailureReason: TError|null
	fetchMeta: any
	isInvalidated: boolean
	status: QueryStatus
	fetchStatus: FetchStatus
	}

I will need to look into it. We don't have an infinite version of that query. Maybe storing it on fetchMeta works, idk 😅

[TkDodo]

hmm, it's mostly an implementation detail, but when working directly with the cache, you'd still need to set it. As an example, assume you have 3 pages:

{
pages: [{}, {}, {}]
pageParams: [5, 10, 15]
}

if we take away pageParams from data and store them somewhere else (on queryState), what happens if you call setQueryData and remove the first page? You will also need to adapt pageParams so that it only contains [10, 15]. We can only do that becausepageParams are part of our Infinite Data structure. So it seems that exactly the use-cases where pageParams becomes annoying are actually the ones that might need it ...

[TkDodo]

@ardeora I'll merge this PR into alpha because it contains some good improvements on the API. If we find a way to get rid of the pageParams as part of data, we can still do this separately.