-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Refactor post threads to use react query #1851
Changes from 3 commits
245b489
76e67f9
cf0cd6d
754ff30
d21e456
1e79e3a
5b9b996
109f31f
98f9355
70e530d
33d10c0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,71 @@ | ||
import {useEffect, useState, useCallback, useRef} from 'react' | ||
import EventEmitter from 'eventemitter3' | ||
import {AppBskyFeedDefs} from '@atproto/api' | ||
|
||
const emitter = new EventEmitter() | ||
|
||
export interface PostShadow { | ||
likeUri: string | undefined | ||
likeCount: number | undefined | ||
repostUri: string | undefined | ||
repostCount: number | undefined | ||
isDeleted: boolean | ||
} | ||
|
||
export type UpdatePostShadowFn = (cache: Partial<PostShadow>) => void | ||
|
||
interface CacheEntry { | ||
ts: number | ||
value: PostShadow | ||
} | ||
|
||
export function usePostShadow( | ||
post: AppBskyFeedDefs.PostView, | ||
ifAfterTS: number, | ||
): PostShadow { | ||
const [state, setState] = useState<CacheEntry>({ | ||
ts: Date.now(), | ||
value: fromPost(post), | ||
}) | ||
const firstRun = useRef(true) | ||
|
||
const onUpdate = useCallback( | ||
(value: Partial<PostShadow>) => { | ||
setState(s => ({ts: Date.now(), value: {...s.value, ...value}})) | ||
}, | ||
[setState], | ||
) | ||
|
||
// react to shadow updates | ||
useEffect(() => { | ||
emitter.addListener(post.uri, onUpdate) | ||
return () => { | ||
emitter.removeListener(post.uri, onUpdate) | ||
} | ||
}, [post.uri, onUpdate]) | ||
|
||
// react to post updates | ||
useEffect(() => { | ||
// dont fire on first run to avoid needless re-renders | ||
if (!firstRun.current) { | ||
setState({ts: Date.now(), value: fromPost(post)}) | ||
} | ||
firstRun.current = false | ||
}, [post]) | ||
|
||
return state.ts > ifAfterTS ? state.value : fromPost(post) | ||
} | ||
|
||
export function updatePostShadow(uri: string, value: Partial<PostShadow>) { | ||
emitter.emit(uri, value) | ||
} | ||
|
||
function fromPost(post: AppBskyFeedDefs.PostView): PostShadow { | ||
return { | ||
likeUri: post.viewer?.like, | ||
likeCount: post.likeCount, | ||
repostUri: post.viewer?.repost, | ||
repostCount: post.repostCount, | ||
isDeleted: false, | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,177 @@ | ||
import { | ||
AppBskyFeedDefs, | ||
AppBskyFeedPost, | ||
AppBskyFeedGetPostThread, | ||
} from '@atproto/api' | ||
import {useQuery} from '@tanstack/react-query' | ||
import {useSession} from '../session' | ||
import {ThreadViewPreference} from '../models/ui/preferences' | ||
|
||
export const RQKEY = (uri: string) => ['post-thread', uri] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not positive we'd really see an issue with this, but we might want to include the user did or something in this keyset so that switching users without a reload to clear memory never results in stale data being shown from cache. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I assumed we'd drop all RQ cache on a session change There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would be nice to have our React Query Keys strongly typed in one place so we can refer to them for future updates. A very simple factory model should work: https://tkdodo.eu/blog/effective-react-query-keys#use-query-key-factories There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm +1 to this, nice to have a quick reference to what query keys are out there There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually @ansh does it change your mind any if we have separate query hooks for each query like Paul has here? I.e. |
||
type ThreadViewNode = AppBskyFeedGetPostThread.OutputSchema['thread'] | ||
|
||
export interface ThreadCtx { | ||
depth: number | ||
isHighlightedPost?: boolean | ||
hasMore?: boolean | ||
showChildReplyLine?: boolean | ||
showParentReplyLine?: boolean | ||
} | ||
|
||
export type ThreadPost = { | ||
type: 'post' | ||
_reactKey: string | ||
uri: string | ||
post: AppBskyFeedDefs.PostView | ||
record: AppBskyFeedPost.Record | ||
parent?: ThreadNode | ||
replies?: ThreadNode[] | ||
viewer?: AppBskyFeedDefs.ViewerThreadState | ||
ctx: ThreadCtx | ||
} | ||
|
||
export type ThreadNotFound = { | ||
type: 'not-found' | ||
_reactKey: string | ||
uri: string | ||
ctx: ThreadCtx | ||
} | ||
|
||
export type ThreadBlocked = { | ||
type: 'blocked' | ||
_reactKey: string | ||
uri: string | ||
ctx: ThreadCtx | ||
} | ||
|
||
export type ThreadUnknown = { | ||
type: 'unknown' | ||
uri: string | ||
} | ||
|
||
export type ThreadNode = | ||
| ThreadPost | ||
| ThreadNotFound | ||
| ThreadBlocked | ||
| ThreadUnknown | ||
|
||
export function usePostThreadQuery(uri: string | undefined) { | ||
const {agent} = useSession() | ||
return useQuery<ThreadNode, Error>( | ||
RQKEY(uri || ''), | ||
async () => { | ||
const res = await agent.getPostThread({uri: uri!}) | ||
if (res.success) { | ||
return responseToThreadNodes(res.data.thread) | ||
} | ||
return {type: 'unknown', uri: uri!} | ||
}, | ||
{enabled: !!uri}, | ||
) | ||
} | ||
|
||
export function sortThread( | ||
node: ThreadNode, | ||
opts: ThreadViewPreference, | ||
): ThreadNode { | ||
if (node.type !== 'post') { | ||
return node | ||
} | ||
if (node.replies) { | ||
node.replies.sort((a: ThreadNode, b: ThreadNode) => { | ||
if (a.type !== 'post') { | ||
return 1 | ||
} | ||
if (b.type !== 'post') { | ||
return -1 | ||
} | ||
|
||
const aIsByOp = a.post.author.did === node.post?.author.did | ||
const bIsByOp = b.post.author.did === node.post?.author.did | ||
if (aIsByOp && bIsByOp) { | ||
return a.post.indexedAt.localeCompare(b.post.indexedAt) // oldest | ||
} else if (aIsByOp) { | ||
return -1 // op's own reply | ||
} else if (bIsByOp) { | ||
return 1 // op's own reply | ||
} | ||
if (opts.prioritizeFollowedUsers) { | ||
const af = a.post.author.viewer?.following | ||
const bf = b.post.author.viewer?.following | ||
if (af && !bf) { | ||
return -1 | ||
} else if (!af && bf) { | ||
return 1 | ||
} | ||
} | ||
if (opts.sort === 'oldest') { | ||
return a.post.indexedAt.localeCompare(b.post.indexedAt) | ||
} else if (opts.sort === 'newest') { | ||
return b.post.indexedAt.localeCompare(a.post.indexedAt) | ||
} else if (opts.sort === 'most-likes') { | ||
if (a.post.likeCount === b.post.likeCount) { | ||
return b.post.indexedAt.localeCompare(a.post.indexedAt) // newest | ||
} else { | ||
return (b.post.likeCount || 0) - (a.post.likeCount || 0) // most likes | ||
} | ||
} else if (opts.sort === 'random') { | ||
return 0.5 - Math.random() // this is vaguely criminal but we can get away with it | ||
} | ||
return b.post.indexedAt.localeCompare(a.post.indexedAt) | ||
}) | ||
node.replies.forEach(reply => sortThread(reply, opts)) | ||
} | ||
return node | ||
} | ||
|
||
// internal methods | ||
// = | ||
|
||
function responseToThreadNodes( | ||
node: ThreadViewNode, | ||
depth = 0, | ||
direction: 'up' | 'down' | 'start' = 'start', | ||
): ThreadNode { | ||
if ( | ||
AppBskyFeedDefs.isThreadViewPost(node) && | ||
AppBskyFeedPost.isRecord(node.post.record) && | ||
AppBskyFeedPost.validateRecord(node.post.record).success | ||
) { | ||
return { | ||
type: 'post', | ||
_reactKey: node.post.uri, | ||
uri: node.post.uri, | ||
post: node.post, | ||
record: node.post.record, | ||
parent: | ||
node.parent && direction !== 'down' | ||
? responseToThreadNodes(node.parent, depth - 1, 'up') | ||
: undefined, | ||
replies: | ||
node.replies?.length && direction !== 'up' | ||
? node.replies.map(reply => | ||
responseToThreadNodes(reply, depth + 1, 'down'), | ||
) | ||
: undefined, | ||
viewer: node.viewer, | ||
ctx: { | ||
depth, | ||
isHighlightedPost: depth === 0, | ||
hasMore: | ||
direction === 'down' && !node.replies?.length && !!node.replyCount, | ||
showChildReplyLine: | ||
direction === 'up' || | ||
(direction === 'down' && !!node.replies?.length), | ||
showParentReplyLine: | ||
(direction === 'up' && !!node.parent) || | ||
(direction === 'down' && depth !== 1), | ||
}, | ||
} | ||
} else if (AppBskyFeedDefs.isBlockedPost(node)) { | ||
return {type: 'blocked', _reactKey: node.uri, uri: node.uri, ctx: {depth}} | ||
} else if (AppBskyFeedDefs.isNotFoundPost(node)) { | ||
return {type: 'not-found', _reactKey: node.uri, uri: node.uri, ctx: {depth}} | ||
} else { | ||
return {type: 'unknown', uri: ''} | ||
} | ||
} |
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.
Nit: conceptually I think I like @gaearon's distinction between state and "cache" i.e. a local cache of remote data that only changes on a server. In respect to that, I'm kinda inclined to say we should put queries in a
src/data/*
dir or something outside the state dir, but that's a very soft opinion. Remote cache/data is "state" in a way too. Idk!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.
totally open to different directory structures here