Skip to content
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

adding resetPages to clear cached pages on new searches #243

Closed
wants to merge 7 commits into from

Conversation

patrickus
Copy link

@patrickus patrickus commented Jan 24, 2020

@shuding
Copy link
Member

shuding commented Jan 24, 2020

Published to swr@0.1.17-beta.1!

@shuding
Copy link
Member

shuding commented Feb 5, 2020

Published swr@0.1.17-beta.4

@innocentiv
Copy link
Contributor

I am faced with issues with pagination this pull request is trying to solve, but I do not like that this pull request changes the function type signature to remove types altogether. This is a breaking change that blocks type inference for the arguments passed to the useSWRPages hook. Could we try to restore types before merging this pull request?

@gabrielperales
Copy link

@patrickus looks like you have configured eslint or prettier to add semicolons to your code and is changing the code style this repo has. It won't be merged with those changes in the code style.

@gabrielperales
Copy link

I need this feature to be merged. If @patrickus doesn't want to keep working on it, I can take over this branch and finish the work if the SWR team thinks that this is the right approach :)

@innocentiv
Copy link
Contributor

Hi, I had the same problem as @gabrielperales so I worked on a fix on my account.
Really sorry for being impolite by taking over this issue. I have created a second PR (#269), but If @patrickus want to work on this, I will close my PR.

import {
pagesResponseInterface,
responseInterface,
pageComponentType,
pageOffsetMapperType
} from './types'

import { cacheGet, cacheSet } from './config'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be

Suggested change
import { cacheGet, cacheSet } from './config'
import { cache } from './config'

Then use cache.get and cache.set instead

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks, hadn't pulled the latest changes, will integrate

const isEmpty = isReachingEnd && pageCount === 1 && emptyPageRef.current
const loadMore = useCallback(() => {
if (isLoadingMore || isReachingEnd) return
if (isLoadingMore || isReachingEnd)
return
setPageCount(c => {
cacheSet(pageCountKey, c + 1)
return c + 1
})
}, [isLoadingMore || isReachingEnd])

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


// Reset page if the key changes
useEffect(() => resetPages(), [pageOffsetKey])
// Doesn't have a next page
const isReachingEnd = pageOffsets[pageCount] === null

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it be pageOffsets[Math.max(1, pageOffsets.length - 1)] === null ?

@shuding shuding mentioned this pull request Jun 9, 2020
7 tasks
@shuding
Copy link
Member

shuding commented Jul 25, 2020

I think we can close this due to the new useSWRInfinite API: #435. It should have solved this problem (and other related ones). As always, thank you for your awesome contribution! 🙏

@shuding shuding closed this Jul 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants