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

Refactor post threads to use react query #1851

Merged
merged 11 commits into from
Nov 9, 2023
4 changes: 4 additions & 0 deletions src/state/models/root-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import {ImageSizesCache} from './cache/image-sizes'
import {reset as resetNavigation} from '../../Navigation'
import {logger} from '#/logger'

import {unstable_setAgent} from '../session'

// TEMPORARY (APP-700)
// remove after backend testing finishes
// -prf
Expand Down Expand Up @@ -49,6 +51,7 @@ export class RootStoreModel {

constructor(agent: BskyAgent) {
this.agent = agent
unstable_setAgent(agent)
makeAutoObservable(this, {
agent: false,
serialize: false,
Expand Down Expand Up @@ -114,6 +117,7 @@ export class RootStoreModel {
) {
logger.debug('RootStoreModel:handleSessionChange')
this.agent = agent
unstable_setAgent(agent)
applyDebugHeader(this.agent)
this.me.clear()
await this.preferences.sync()
Expand Down
200 changes: 200 additions & 0 deletions src/state/queries/post-thread.ts
Copy link
Member

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!

Copy link
Collaborator Author

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

Original file line number Diff line number Diff line change
@@ -0,0 +1,200 @@
import {AppBskyFeedDefs, AppBskyFeedGetPostThread} from '@atproto/api'
import {useQuery, QueryClient, useQueryClient} from '@tanstack/react-query'
import {useSession} from '../session'
import {RQKEY as POST_RQKEY, getCachedPost} from './post'
import {ThreadViewPreference} from '../models/ui/preferences'

export const RQKEY = (uri: string) => ['post-thread', uri]
Copy link
Member

Choose a reason for hiding this comment

The 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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I assumed we'd drop all RQ cache on a session change

Copy link
Contributor

Choose a reason for hiding this comment

The 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

Copy link
Member

Choose a reason for hiding this comment

The 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

Copy link
Member

Choose a reason for hiding this comment

The 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. usePostQuery instead of useQuery(RQKEY('uri'), () => {}). So finding all usages is pretty easy either way.

type ThreadViewNode = AppBskyFeedGetPostThread.OutputSchema['thread']

export interface PostThreadSkeletonCtx {
depth: number
isHighlightedPost?: boolean
hasMore?: boolean
showChildReplyLine?: boolean
showParentReplyLine?: boolean
}

export type PostThreadSkeletonPost = {
type: 'post'
_reactKey: string
uri: string
parent?: PostThreadSkeletonNode
replies?: PostThreadSkeletonNode[]
viewer?: AppBskyFeedDefs.ViewerThreadState
ctx: PostThreadSkeletonCtx
}

export type PostThreadSkeletonNotFound = {
type: 'not-found'
_reactKey: string
uri: string
ctx: PostThreadSkeletonCtx
}

export type PostThreadSkeletonBlocked = {
type: 'blocked'
_reactKey: string
uri: string
ctx: PostThreadSkeletonCtx
}

export type PostThreadSkeletonUnknown = {
type: 'unknown'
uri: string
}

export type PostThreadSkeletonNode =
| PostThreadSkeletonPost
| PostThreadSkeletonNotFound
| PostThreadSkeletonBlocked
| PostThreadSkeletonUnknown

export function usePostThreadQuery(uri: string | undefined) {
const {agent} = useSession()
const queryClient = useQueryClient()
return useQuery<PostThreadSkeletonNode, Error>(
RQKEY(uri || ''),
async () => {
const res = await agent.getPostThread({uri: uri!})
if (res.success) {
hydrateCache(queryClient, res.data.thread)
return threadViewToSkeleton(res.data.thread)
}
return {type: 'unknown', uri: uri!}
},
{enabled: !!uri},
)
}

export function sortThreadSkeleton(
Copy link
Member

Choose a reason for hiding this comment

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

These functions are so clear 🥺 I love them

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

oh yeah why dont you marry them

Copy link
Contributor

Choose a reason for hiding this comment

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

Would love a comment overview of how this function sorts the posts for future contributors:

// 1. If node is not a post return it unmodified, otherwise continue.
// 2. If the node has replies, start the sorting process.
// 3. Retrieve the cached post for the current node using its URI.
// 4. Sort the replies array in place using a custom comparator.
// 5. Inside the comparator, give priority to posts over non-posts.
// 6. Retrieve cached posts for both comparison nodes using their URIs.
// 7. If the cached post for 'a' is not found, it is moved down in the sort order.
// 8. If the cached post for 'b' is not found, it is moved up in the sort order.
// 9. Check if the author of post 'a' is the original poster (OP) and the same for post 'b'.
// 10. If both are by the OP, sort by the timestamp 'indexedAt' to get the oldest post.
// 11. If only 'a' is by the OP, it gets higher priority.
// 12. If only 'b' is by the OP, 'a' gets higher priority.
// 13. If user preference is to prioritize followed users, compare the following status of each post's author.
// 14. Sort by followed status, giving priority to followed user's posts.
// 15. If sorting by 'oldest', sort by 'indexedAt' to get the oldest post.
// 16. If sorting by 'newest', sort by 'indexedAt' in reverse to get the newest post.
// 17. If sorting by 'most-likes', compare like counts; in case of a tie, use 'indexedAt' to prioritize newer posts.
// 18. If sorting by 'random', return a random order by using Math.random.
// 19. Default to sorting by 'indexedAt' in reverse if no other conditions are met.
// 20. Recursively apply the sorting function to each reply.
// 21. Finally, return the sorted node.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm not a huge fan of describing code a second time in the comments -- only summarizing or explaining

queryClient: QueryClient,
node: PostThreadSkeletonNode,
opts: ThreadViewPreference,
): PostThreadSkeletonNode {
if (node.type !== 'post') {
return node
}
if (node.replies) {
const post = getCachedPost(queryClient, node.uri)
node.replies.sort(
(a: PostThreadSkeletonNode, b: PostThreadSkeletonNode) => {
if (a.type !== 'post') {
return 1
}
if (b.type !== 'post') {
return -1
}

const postA = getCachedPost(queryClient, a.uri)
const postB = getCachedPost(queryClient, b.uri)
if (!postA) {
return 1
}
if (!postB) {
return -1
}

const aIsByOp = postA.author.did === post?.author.did
const bIsByOp = postB.author.did === post?.author.did
if (aIsByOp && bIsByOp) {
return postA.indexedAt.localeCompare(postB.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 = postA.author.viewer?.following
const bf = postB.author.viewer?.following
if (af && !bf) {
return -1
} else if (!af && bf) {
return 1
}
}
if (opts.sort === 'oldest') {
return postA.indexedAt.localeCompare(postB.indexedAt)
} else if (opts.sort === 'newest') {
return postB.indexedAt.localeCompare(postA.indexedAt)
} else if (opts.sort === 'most-likes') {
if (postA.likeCount === postB.likeCount) {
return postB.indexedAt.localeCompare(postA.indexedAt) // newest
} else {
return (postB.likeCount || 0) - (postA.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 postB.indexedAt.localeCompare(postA.indexedAt)
},
)
node.replies.forEach(reply => sortThreadSkeleton(queryClient, reply, opts))
}
return node
}

// internal methods
// =

function threadViewToSkeleton(
node: ThreadViewNode,
depth = 0,
direction: 'up' | 'down' | 'start' = 'start',
): PostThreadSkeletonNode {
if (AppBskyFeedDefs.isThreadViewPost(node)) {
return {
type: 'post',
_reactKey: node.post.uri,
uri: node.post.uri,
parent:
node.parent && direction !== 'down'
? threadViewToSkeleton(node.parent, depth - 1, 'up')
: undefined,
replies:
node.replies?.length && direction !== 'up'
? node.replies.map(reply =>
threadViewToSkeleton(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: ''}
}
}

function hydrateCache(queryClient: QueryClient, node: ThreadViewNode) {
if (AppBskyFeedDefs.isThreadViewPost(node)) {
queryClient.setQueryData(POST_RQKEY(node.post.uri), node.post)
if (node.parent) {
hydrateCache(queryClient, node.parent)
}
if (node.replies?.length) {
for (const reply of node.replies) {
hydrateCache(queryClient, reply)
}
}
} else if (
AppBskyFeedDefs.isBlockedPost(node) ||
AppBskyFeedDefs.isNotFoundPost(node)
) {
queryClient.setQueryData(POST_RQKEY(node.uri), undefined)
}
}
Loading