Fix state lifecycle management with post-feed query, solving the duplicate key issue (#2034)

* Assign keys to feed slices via a counter, to enable duplicate items in the feed if needed

* Move post-feed query state into the query's page params to consistently bind their lifecycles
zio/stable
Paul Frazee 2023-11-29 16:58:14 -08:00 committed by GitHub
parent a59d235e8b
commit 630637874d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 99 additions and 91 deletions

View File

@ -16,14 +16,7 @@ export type FeedTunerFn = (
export class FeedViewPostsSlice { export class FeedViewPostsSlice {
isFlattenedReply = false isFlattenedReply = false
constructor(public items: FeedViewPost[] = []) {} constructor(public items: FeedViewPost[], public _reactKey: string) {}
get _reactKey() {
const rootItem = this.isFlattenedReply ? this.items[1] : this.items[0]
return `slice-${rootItem.post.uri}-${
rootItem.reason?.indexedAt || rootItem.post.indexedAt
}`
}
get uri() { get uri() {
if (this.isFlattenedReply) { if (this.isFlattenedReply) {
@ -118,28 +111,34 @@ export class FeedViewPostsSlice {
} }
export class NoopFeedTuner { export class NoopFeedTuner {
reset() {} private keyCounter = 0
reset() {
this.keyCounter = 0
}
tune( tune(
feed: FeedViewPost[], feed: FeedViewPost[],
_tunerFns: FeedTunerFn[] = [],
_opts?: {dryRun: boolean; maintainOrder: boolean}, _opts?: {dryRun: boolean; maintainOrder: boolean},
): FeedViewPostsSlice[] { ): FeedViewPostsSlice[] {
return feed.map(item => new FeedViewPostsSlice([item])) return feed.map(
item => new FeedViewPostsSlice([item], `slice-${this.keyCounter++}`),
)
} }
} }
export class FeedTuner { export class FeedTuner {
private keyCounter = 0
seenUris: Set<string> = new Set() seenUris: Set<string> = new Set()
constructor() {} constructor(public tunerFns: FeedTunerFn[]) {}
reset() { reset() {
this.keyCounter = 0
this.seenUris.clear() this.seenUris.clear()
} }
tune( tune(
feed: FeedViewPost[], feed: FeedViewPost[],
tunerFns: FeedTunerFn[] = [],
{dryRun, maintainOrder}: {dryRun: boolean; maintainOrder: boolean} = { {dryRun, maintainOrder}: {dryRun: boolean; maintainOrder: boolean} = {
dryRun: false, dryRun: false,
maintainOrder: false, maintainOrder: false,
@ -148,7 +147,9 @@ export class FeedTuner {
let slices: FeedViewPostsSlice[] = [] let slices: FeedViewPostsSlice[] = []
if (maintainOrder) { if (maintainOrder) {
slices = feed.map(item => new FeedViewPostsSlice([item])) slices = feed.map(
item => new FeedViewPostsSlice([item], `slice-${this.keyCounter++}`),
)
} else { } else {
// arrange the posts into thread slices // arrange the posts into thread slices
for (let i = feed.length - 1; i >= 0; i--) { for (let i = feed.length - 1; i >= 0; i--) {
@ -164,12 +165,14 @@ export class FeedTuner {
continue continue
} }
} }
slices.unshift(new FeedViewPostsSlice([item])) slices.unshift(
new FeedViewPostsSlice([item], `slice-${this.keyCounter++}`),
)
} }
} }
// run the custom tuners // run the custom tuners
for (const tunerFn of tunerFns) { for (const tunerFn of this.tunerFns) {
slices = tunerFn(this, slices.slice()) slices = tunerFn(this, slices.slice())
} }

View File

@ -180,7 +180,7 @@ class MergeFeedSource {
} }
class MergeFeedSource_Following extends MergeFeedSource { class MergeFeedSource_Following extends MergeFeedSource {
tuner = new FeedTuner() tuner = new FeedTuner(this.feedTuners)
reset() { reset() {
super.reset() super.reset()
@ -197,7 +197,7 @@ class MergeFeedSource_Following extends MergeFeedSource {
): Promise<AppBskyFeedGetTimeline.Response> { ): Promise<AppBskyFeedGetTimeline.Response> {
const res = await getAgent().getTimeline({cursor, limit}) const res = await getAgent().getTimeline({cursor, limit})
// run the tuner pre-emptively to ensure better mixing // run the tuner pre-emptively to ensure better mixing
const slices = this.tuner.tune(res.data.feed, this.feedTuners, { const slices = this.tuner.tune(res.data.feed, {
dryRun: false, dryRun: false,
maintainOrder: true, maintainOrder: true,
}) })

View File

@ -1,5 +1,4 @@
import {useCallback, useMemo} from 'react' import {AppBskyFeedDefs, AppBskyFeedPost} from '@atproto/api'
import {AppBskyFeedDefs, AppBskyFeedPost, moderatePost} from '@atproto/api'
import { import {
useInfiniteQuery, useInfiniteQuery,
InfiniteData, InfiniteData,
@ -8,7 +7,7 @@ import {
useQueryClient, useQueryClient,
} from '@tanstack/react-query' } from '@tanstack/react-query'
import {useFeedTuners} from '../preferences/feed-tuners' import {useFeedTuners} from '../preferences/feed-tuners'
import {FeedTuner, NoopFeedTuner} from 'lib/api/feed-manip' import {FeedTuner, FeedTunerFn, NoopFeedTuner} from 'lib/api/feed-manip'
import {FeedAPI, ReasonFeedSource} from 'lib/api/feed/types' import {FeedAPI, ReasonFeedSource} from 'lib/api/feed/types'
import {FollowingFeedAPI} from 'lib/api/feed/following' import {FollowingFeedAPI} from 'lib/api/feed/following'
import {AuthorFeedAPI} from 'lib/api/feed/author' import {AuthorFeedAPI} from 'lib/api/feed/author'
@ -16,7 +15,6 @@ import {LikesFeedAPI} from 'lib/api/feed/likes'
import {CustomFeedAPI} from 'lib/api/feed/custom' import {CustomFeedAPI} from 'lib/api/feed/custom'
import {ListFeedAPI} from 'lib/api/feed/list' import {ListFeedAPI} from 'lib/api/feed/list'
import {MergeFeedAPI} from 'lib/api/feed/merge' import {MergeFeedAPI} from 'lib/api/feed/merge'
import {useModerationOpts} from '#/state/queries/preferences'
import {logger} from '#/logger' import {logger} from '#/logger'
import {STALE} from '#/state/queries' import {STALE} from '#/state/queries'
import {precacheFeedPosts as precacheResolvedUris} from './resolve-uri' import {precacheFeedPosts as precacheResolvedUris} from './resolve-uri'
@ -41,7 +39,9 @@ export interface FeedParams {
mergeFeedSources?: string[] mergeFeedSources?: string[]
} }
type RQPageParam = string | undefined type RQPageParam =
| {cursor: string | undefined; api: FeedAPI; tuner: FeedTuner | NoopFeedTuner}
| undefined
export function RQKEY(feedDesc: FeedDescriptor, params?: FeedParams) { export function RQKEY(feedDesc: FeedDescriptor, params?: FeedParams) {
return ['post-feed', feedDesc, params || {}] return ['post-feed', feedDesc, params || {}]
@ -63,6 +63,8 @@ export interface FeedPostSlice {
} }
export interface FeedPage { export interface FeedPage {
api: FeedAPI
tuner: FeedTuner | NoopFeedTuner
cursor: string | undefined cursor: string | undefined
slices: FeedPostSlice[] slices: FeedPostSlice[]
} }
@ -75,64 +77,8 @@ export function usePostFeedQuery(
const queryClient = useQueryClient() const queryClient = useQueryClient()
const feedTuners = useFeedTuners(feedDesc) const feedTuners = useFeedTuners(feedDesc)
const enabled = opts?.enabled !== false const enabled = opts?.enabled !== false
const moderationOpts = useModerationOpts()
const api: FeedAPI = useMemo(() => { return useInfiniteQuery<
if (feedDesc === 'home') {
return new MergeFeedAPI(params || {}, feedTuners)
} else if (feedDesc === 'following') {
return new FollowingFeedAPI()
} else if (feedDesc.startsWith('author')) {
const [_, actor, filter] = feedDesc.split('|')
return new AuthorFeedAPI({actor, filter})
} else if (feedDesc.startsWith('likes')) {
const [_, actor] = feedDesc.split('|')
return new LikesFeedAPI({actor})
} else if (feedDesc.startsWith('feedgen')) {
const [_, feed] = feedDesc.split('|')
return new CustomFeedAPI({feed})
} else if (feedDesc.startsWith('list')) {
const [_, list] = feedDesc.split('|')
return new ListFeedAPI({list})
} else {
// shouldnt happen
return new FollowingFeedAPI()
}
}, [feedDesc, params, feedTuners])
const disableTuner = !!params?.disableTuner
const tuner = useMemo(
() => (disableTuner ? new NoopFeedTuner() : new FeedTuner()),
[disableTuner],
)
const pollLatest = useCallback(async () => {
if (!enabled) {
return false
}
logger.debug('usePostFeedQuery: pollLatest')
const post = await api.peekLatest()
if (post && moderationOpts) {
const slices = tuner.tune([post], feedTuners, {
dryRun: true,
maintainOrder: true,
})
if (slices[0]) {
if (
!moderatePost(slices[0].items[0].post, moderationOpts).content.filter
) {
return true
}
}
}
return false
}, [api, tuner, feedTuners, moderationOpts, enabled])
const out = useInfiniteQuery<
FeedPage, FeedPage,
Error, Error,
InfiniteData<FeedPage>, InfiniteData<FeedPage>,
@ -143,13 +89,23 @@ export function usePostFeedQuery(
queryKey: RQKEY(feedDesc, params), queryKey: RQKEY(feedDesc, params),
async queryFn({pageParam}: {pageParam: RQPageParam}) { async queryFn({pageParam}: {pageParam: RQPageParam}) {
logger.debug('usePostFeedQuery', {feedDesc, pageParam}) logger.debug('usePostFeedQuery', {feedDesc, pageParam})
if (!pageParam) {
tuner.reset() const {api, tuner, cursor} = pageParam
} ? pageParam
const res = await api.fetch({cursor: pageParam, limit: 30}) : {
api: createApi(feedDesc, params || {}, feedTuners),
tuner: params?.disableTuner
? new NoopFeedTuner()
: new FeedTuner(feedTuners),
cursor: undefined,
}
const res = await api.fetch({cursor, limit: 30})
precacheResolvedUris(queryClient, res.feed) // precache the handle->did resolution precacheResolvedUris(queryClient, res.feed) // precache the handle->did resolution
const slices = tuner.tune(res.feed, feedTuners) const slices = tuner.tune(res.feed)
return { return {
api,
tuner,
cursor: res.cursor, cursor: res.cursor,
slices: slices.map(slice => ({ slices: slices.map(slice => ({
_reactKey: slice._reactKey, _reactKey: slice._reactKey,
@ -180,11 +136,60 @@ export function usePostFeedQuery(
} }
}, },
initialPageParam: undefined, initialPageParam: undefined,
getNextPageParam: lastPage => lastPage.cursor, getNextPageParam: lastPage => ({
api: lastPage.api,
tuner: lastPage.tuner,
cursor: lastPage.cursor,
}),
enabled, enabled,
}) })
}
return {...out, pollLatest} export async function pollLatest(page: FeedPage | undefined) {
if (!page) {
return false
}
logger.debug('usePostFeedQuery: pollLatest')
const post = await page.api.peekLatest()
if (post) {
const slices = page.tuner.tune([post], {
dryRun: true,
maintainOrder: true,
})
if (slices[0]) {
return true
}
}
return false
}
function createApi(
feedDesc: FeedDescriptor,
params: FeedParams,
feedTuners: FeedTunerFn[],
) {
if (feedDesc === 'home') {
return new MergeFeedAPI(params, feedTuners)
} else if (feedDesc === 'following') {
return new FollowingFeedAPI()
} else if (feedDesc.startsWith('author')) {
const [_, actor, filter] = feedDesc.split('|')
return new AuthorFeedAPI({actor, filter})
} else if (feedDesc.startsWith('likes')) {
const [_, actor] = feedDesc.split('|')
return new LikesFeedAPI({actor})
} else if (feedDesc.startsWith('feedgen')) {
const [_, feed] = feedDesc.split('|')
return new CustomFeedAPI({feed})
} else if (feedDesc.startsWith('list')) {
const [_, list] = feedDesc.split('|')
return new ListFeedAPI({list})
} else {
// shouldnt happen
return new FollowingFeedAPI()
}
} }
/** /**

View File

@ -23,6 +23,7 @@ import {
FeedDescriptor, FeedDescriptor,
FeedParams, FeedParams,
usePostFeedQuery, usePostFeedQuery,
pollLatest,
} from '#/state/queries/post-feed' } from '#/state/queries/post-feed'
import {useModerationOpts} from '#/state/queries/preferences' import {useModerationOpts} from '#/state/queries/preferences'
@ -84,22 +85,21 @@ let Feed = ({
hasNextPage, hasNextPage,
isFetchingNextPage, isFetchingNextPage,
fetchNextPage, fetchNextPage,
pollLatest,
} = usePostFeedQuery(feed, feedParams, opts) } = usePostFeedQuery(feed, feedParams, opts)
const isEmpty = !isFetching && !data?.pages[0]?.slices.length const isEmpty = !isFetching && !data?.pages[0]?.slices.length
const checkForNew = React.useCallback(async () => { const checkForNew = React.useCallback(async () => {
if (!isFetched || isFetching || !onHasNew) { if (!data?.pages[0] || isFetching || !onHasNew) {
return return
} }
try { try {
if (await pollLatest()) { if (await pollLatest(data.pages[0])) {
onHasNew(true) onHasNew(true)
} }
} catch (e) { } catch (e) {
logger.error('Poll latest failed', {feed, error: String(e)}) logger.error('Poll latest failed', {feed, error: String(e)})
} }
}, [feed, isFetched, isFetching, pollLatest, onHasNew]) }, [feed, data, isFetching, onHasNew])
React.useEffect(() => { React.useEffect(() => {
// we store the interval handler in a ref to avoid needless // we store the interval handler in a ref to avoid needless