Refactor post threads to use react query #1851

pfrazee · 2023-11-09T01:31:40Z

This PR refactors the Post Thread code to use React Query (RQ).

In addition to refactoring the Post Thread, this PR introduces a "Post Shadow" cache which is used for optimistic updates that we want to keep synced across views (like, repost, and deletion state).

Some notes:

~~This PR is using a temporary useSession hook that lightly wraps the existing session manager.~~
There are some commented out TODOs in the component which have to do with the previous post cache. We don't want to get rid of that yet because it's a case we're still going to need to handle -- using posts from a feed to create a pre-fetch skeleton of threads.
There are a couple of components which I've temporarily replaced by adding a 2 to the filename, since there's still existing code depending on them.

estrattonbailey · 2023-11-09T16:37:47Z

src/state/queries/post-thread.ts

+import {RQKEY as POST_RQKEY, getCachedPost} from './post'
+import {ThreadViewPreference} from '../models/ui/preferences'
+
+export const RQKEY = (uri: string) => ['post-thread', uri]


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.

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

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

I'm +1 to this, nice to have a quick reference to what query keys are out there

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.

estrattonbailey · 2023-11-09T16:40:58Z

src/state/queries/post-thread.ts

+  )
+}
+
+export function sortThreadSkeleton(


These functions are so clear 🥺 I love them

oh yeah why dont you marry them

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.

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

estrattonbailey · 2023-11-09T16:44:36Z

src/state/queries/post-thread.ts

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!

totally open to different directory structures here

ansh · 2023-11-09T18:11:34Z

src/state/queries/post.ts

+} from '@tanstack/react-query'
+import {useSession} from '../session'
+
+export const RQKEY = (postUri: string) => ['post', postUri]


Would be nice to have this in a typed key generating factory as previously mentioned

ansh · 2023-11-09T18:11:54Z

src/state/queries/post.ts

+        return res.data.posts[0]
+      }
+
+      throw new Error('No data')


Is this error propagated somewhere?

it becomes RQ error state, right?

ansh · 2023-11-09T18:13:43Z

src/state/queries/post.ts

+      })
+      return {likeCount}
+    },
+    onSuccess(data, variables) {


React Query v5 is out which simplifies the optimistic update workflow. We no longer have to manually update the cache every time:

https://medium.com/@stojanovic.nemanja71/optimistic-updates-in-tanstack-query-v5-dfbcbb124113

This applies to other optimistic updates as well. Not adding the comment everywhere fyi.

The link above is how we should do things in most cases. In this case, the post shadow is synced across components and it's better to put in these handlers

ansh · 2023-11-09T18:15:24Z

src/view/com/post-thread/PostThread.tsx

+    isRefetching,
+    data: threadSkeleton,
+  } = usePostThreadQuery(resolvedUri)
+  const {data: rootPost} = usePostQuery(threadSkeleton?.uri)


Love how the code is simplified!

ansh · 2023-11-09T18:16:09Z

src/view/com/post-thread/PostThread.tsx

+          Post hidden
+        </Text>
+        <Text type="md" style={[pal.text, s.mb10]}>
+          You have blocked the author or you have been blocked by the author.


Please merge with main and add <Trans> here

ansh · 2023-11-09T18:16:20Z

src/view/com/post-thread/PostThread.tsx

+    <CenteredView>
+      <View style={[pal.view, pal.border, styles.notFoundContainer]}>
+        <Text type="title-lg" style={[pal.text, s.mb5]}>
+          Post hidden


<Trans> here too

src/view/screens/PostThread.tsx

pfrazee added 2 commits November 8, 2023 17:21

Add post and post-thread queries

245b489

Update PostThread components to use new queries

76e67f9

estrattonbailey reviewed Nov 9, 2023

View reviewed changes

ansh reviewed Nov 9, 2023

View reviewed changes

src/view/screens/PostThread.tsx Outdated Show resolved Hide resolved

pfrazee added 9 commits November 9, 2023 11:33

Move from normalized cache to shadow cache model

cf0cd6d

Merge branch 'main' into rq-postthread

754ff30

Merge post shadow into the post automatically

d21e456

Remove dead code

1e79e3a

Merge branch 'main' into rq-postthread

5b9b996

Remove old temporary session

109f31f

Fix: set agent on session creation

98f9355

Temporarily double-login

70e530d

Handle post-thread uri resolution errors

33d10c0

pfrazee marked this pull request as ready for review November 9, 2023 23:34

pfrazee merged commit fb4f570 into main Nov 9, 2023
4 checks passed

pfrazee deleted the rq-postthread branch November 9, 2023 23:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor post threads to use react query #1851

Refactor post threads to use react query #1851

pfrazee commented Nov 9, 2023 •

edited

Loading

estrattonbailey Nov 9, 2023

pfrazee Nov 9, 2023

ansh Nov 9, 2023

estrattonbailey Nov 9, 2023

estrattonbailey Nov 9, 2023

estrattonbailey Nov 9, 2023

pfrazee Nov 9, 2023

ansh Nov 9, 2023

pfrazee Nov 9, 2023

estrattonbailey Nov 9, 2023

pfrazee Nov 9, 2023

ansh Nov 9, 2023

ansh Nov 9, 2023

pfrazee Nov 9, 2023

ansh Nov 9, 2023 •

edited

Loading

ansh Nov 9, 2023

pfrazee Nov 9, 2023

ansh Nov 9, 2023

ansh Nov 9, 2023

ansh Nov 9, 2023

Refactor post threads to use react query #1851

Refactor post threads to use react query #1851

Conversation

pfrazee commented Nov 9, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ansh Nov 9, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pfrazee commented Nov 9, 2023 •

edited

Loading

ansh Nov 9, 2023 •

edited

Loading