refactor(replies): useReplies should use a comment argument to not create infinite comment updates

Author: estebanabaroa

README.md

@@ -27,7 +27,7 @@ useNotifications(): {notifications: Notification[], markAsRead: Function}
 #### Comments Hooks
 ```
 useComment({commentCid: string, onlyIfCached?: boolean}): Comment
-useReplies({commentCid: string, sortType?: string, flat?: boolean, repliesPerPage?: number, filter>: CommentsFilter}): {replies: Comment[], hasMore: boolean, loadMore: function, reset: function, updatedReplies: Comment[], bufferedReplies: Comment[]}
+useReplies({comment: Comment, sortType?: string, flat?: boolean, repliesPerPage?: number, filter>: CommentsFilter}): {replies: Comment[], hasMore: boolean, loadMore: function, reset: function, updatedReplies: Comment[], bufferedReplies: Comment[]}
 useComments({commentCids: string[], onlyIfCached?: boolean}): {comments: Comment[]}
 useEditedComment({comment: Comment}): {editedComment: Comment | undefined}
 useValidateComment({comment: Comment, validateReplies?: boolean}): {valid: boolean}
@@ -144,7 +144,7 @@ const {authorAddress, shortAuthorAddress} = useAuthorAddress({comment: post})
 const post = useComment({commentCid, onlyIfCached: true})
 
 // post.replies are not validated, to show replies
-const {replies, hasMore, loadMore} = useReplies({commentCid})
+const {replies, hasMore, loadMore} = useReplies({comment: post})
 
 // to show a preloaded reply without rerenders, validate manually
 const {valid} = useValidateComment({comment: post.replies.pages.best[0]})

src/hooks/comments.test.ts

@@ -2,6 +2,7 @@ import {act, renderHook} from '@testing-library/react-hooks'
 import testUtils from '../lib/test-utils'
 import {useComment, useComments, useReplies, useValidateComment, setPlebbitJs} from '..'
 import commentsStore from '../stores/comments'
+import repliesCommentsStore from '../stores/replies/replies-comments-store'
 import subplebbitsPagesStore from '../stores/subplebbits-pages'
 import PlebbitJsMock, {Plebbit, Comment, Pages, simulateLoadingTime} from '../lib/plebbit-js/plebbit-js-mock'
 import utils from '../lib/utils'
@@ -339,7 +340,11 @@ describe('comment replies', () => {
     let rendered, waitFor, scrollOnePage
 
     beforeEach(() => {
-      rendered = renderHook<any, any>((useRepliesOptions) => useReplies(useRepliesOptions))
+      rendered = renderHook<any, any>((useRepliesOptions) => {
+        // useReplies accepts only 'comment', but for ease of use also accept a 'commentCid' in the tests
+        const comment = useComment({commentCid: useRepliesOptions?.commentCid})
+        return useReplies({...useRepliesOptions, commentCid: undefined, comment: useRepliesOptions?.comment || comment})
+      })
       waitFor = testUtils.createWaitFor(rendered)
       scrollOnePage = scrollOnePage = async () => {
         const nextFeedLength = (rendered.result.current.replies?.length || 0) + repliesPerPage
@@ -383,6 +388,48 @@ describe('comment replies', () => {
       expect(rendered.result.current.hasMore).toBe(true)
     })
 
+    test('sort type new, switch to sort type old, switch to different comment (comment argument)', async () => {
+      expect(rendered.result.current.replies).toEqual([])
+
+      const createMockComment = (comment) => {
+        comment.timestamp = 1
+        comment.updatedAt = 2
+        comment.replies = {
+          pages: {best: new Pages({comment}).pageToGet(comment.cid + ' page cid best')},
+          pageCids: {
+            new: comment.cid + ' page cid new',
+            newFlat: comment.cid + ' page cid newFlat',
+            old: comment.cid + ' page cid old',
+            oldFlat: comment.cid + ' page cid oldFlat',
+          },
+        }
+        return comment
+      }
+
+      const commentAbc = createMockComment({cid: 'comment cid abc', subplebbitAddress: 'subplebbit address 1'})
+      rendered.rerender({comment: commentAbc, sortType: 'new'})
+      await waitFor(() => rendered.result.current.replies.length === repliesPerPage)
+      expect(rendered.result.current.replies.length).toBe(repliesPerPage)
+      expect(rendered.result.current.replies[0].cid).toBe('comment cid abc page cid new comment cid 100')
+      expect(rendered.result.current.replies[0].timestamp).toBeGreaterThan(rendered.result.current.replies[repliesPerPage - 1].timestamp)
+      expect(rendered.result.current.hasMore).toBe(true)
+
+      rendered.rerender({comment: commentAbc, sortType: 'old'})
+      await waitFor(() => rendered.result.current.replies[0].cid === 'comment cid abc page cid old comment cid 1')
+      expect(rendered.result.current.replies.length).toBe(repliesPerPage)
+      expect(rendered.result.current.replies[0].cid).toBe('comment cid abc page cid old comment cid 1')
+      expect(rendered.result.current.replies[repliesPerPage - 1].timestamp).toBeGreaterThan(rendered.result.current.replies[0].timestamp)
+      expect(rendered.result.current.hasMore).toBe(true)
+
+      const commentXyz = createMockComment({cid: 'comment cid xyz', subplebbitAddress: 'subplebbit address 1'})
+      rendered.rerender({comment: commentXyz, sortType: 'old'})
+      await waitFor(() => rendered.result.current.replies[0].cid === 'comment cid xyz page cid old comment cid 1')
+      expect(rendered.result.current.replies.length).toBe(repliesPerPage)
+      expect(rendered.result.current.replies[0].cid).toBe('comment cid xyz page cid old comment cid 1')
+      expect(rendered.result.current.replies[repliesPerPage - 1].timestamp).toBeGreaterThan(rendered.result.current.replies[0].timestamp)
+      expect(rendered.result.current.hasMore).toBe(true)
+    })
+
     test('scroll pages', async () => {
       expect(rendered.result.current.replies).toEqual([])
 
@@ -603,40 +650,6 @@ describe('comment replies', () => {
       expect(Object.keys(repliesStore.getState().feedsOptions).length).toBe(2)
     })
 
-    test('can useComment({onlyIfCached: true}) on a comment from replies pages', async () => {
-      rendered = renderHook<any, any>(({useCommentOptions, useRepliesOptions}) => {
-        const comment = useComment(useCommentOptions)
-        const replies = useReplies(useRepliesOptions)
-        return {comment, replies}
-      })
-      waitFor = testUtils.createWaitFor(rendered)
-
-      const useRepliesOptions = {
-        commentCid: 'comment cid 1',
-        // the preloaded page is best
-        sortType: 'best',
-        // the preloaded page size is 100
-        repliesPerPage: 200,
-      }
-      rendered.rerender({useRepliesOptions})
-      await waitFor(() => rendered.result.current.replies.replies.length === useRepliesOptions.repliesPerPage)
-      expect(rendered.result.current.replies.replies.length).toBe(useRepliesOptions.repliesPerPage)
-
-      // first reply (preloaded) is defined
-      const firstCommentCid = rendered.result.current.replies.replies[0].cid
-      rendered.rerender({useRepliesOptions, useCommentOptions: {commentCid: firstCommentCid, onlyIfCached: true}})
-      await waitFor(() => typeof rendered.result.current.comment.updatedAt === 'number')
-      expect(typeof rendered.result.current.comment.updatedAt).toBe('number')
-      expect(rendered.result.current.comment.cid).toBe(firstCommentCid)
-
-      // last reply (from a page) is defined
-      const lastCommentCid = rendered.result.current.replies.replies[useRepliesOptions.repliesPerPage - 1].cid
-      rendered.rerender({useRepliesOptions, useCommentOptions: {commentCid: lastCommentCid, onlyIfCached: true}})
-      await waitFor(() => typeof rendered.result.current.comment.updatedAt === 'number')
-      expect(typeof rendered.result.current.comment.updatedAt).toBe('number')
-      expect(rendered.result.current.comment.cid).toBe(lastCommentCid)
-    })
-
     test.todo('has account comments', async () => {})
 
     test.todo('nested scroll pages', async () => {})
@@ -718,7 +731,12 @@ describe('comment replies', () => {
     let rendered, waitFor, scrollOnePage
 
     beforeEach(() => {
-      rendered = renderHook<any, any>((useRepliesOptions) => useReplies(useRepliesOptions))
+      rendered = renderHook<any, any>((useRepliesOptions) => {
+        // useReplies accepts only 'comment', but for ease of use also accept a 'commentCid' in the tests
+        const comment = useComment({commentCid: useRepliesOptions?.commentCid})
+        return useReplies({...useRepliesOptions, commentCid: undefined, comment: useRepliesOptions?.comment || comment})
+      })
+
       waitFor = testUtils.createWaitFor(rendered)
       scrollOnePage = scrollOnePage = async () => {
         const nextFeedLength = (rendered.result.current.replies?.length || 0) + repliesPerPage
@@ -1019,11 +1037,11 @@ describe('comment replies', () => {
 
       // second comment update (updatedAt doesn't change, so shouldn't update)
       comment.simulateUpdateEvent()
-      await waitFor(() => commentsStore.getState().comments['comment cid 1'].replies.pages.best.comments[0].upvoteCount === 2)
+      await waitFor(() => repliesCommentsStore.getState().comments['comment cid 1'].replies.pages.best.comments[0].upvoteCount === 2)
       expect(pages.length).toBe(2)
       // comment in store updated, but the updatedAt didn't change so no change in useReplies().updatedReplies
-      expect(commentsStore.getState().comments['comment cid 1'].replies.pages.best.comments[0].updatedAt).toBe(1)
-      expect(commentsStore.getState().comments['comment cid 1'].replies.pages.best.comments[0].upvoteCount).toBe(2)
+      expect(repliesCommentsStore.getState().comments['comment cid 1'].replies.pages.best.comments[0].updatedAt).toBe(1)
+      expect(repliesCommentsStore.getState().comments['comment cid 1'].replies.pages.best.comments[0].upvoteCount).toBe(2)
       expect(rendered.result.current.replies.length).toBe(1)
       expect(rendered.result.current.updatedReplies.length).toBe(1)
       expect(rendered.result.current.replies[0].cid).toBe('comment cid 1')

src/hooks/comments.ts

@@ -187,7 +187,7 @@ export function useComments(options?: UseCommentsOptions): UseCommentsResult {
 
 export function useReplies(options?: UseRepliesOptions): UseRepliesResult {
   assert(!options || typeof options === 'object', `useReplies options argument '${options}' not an object`)
-  let {commentCid, sortType, accountName, flat, accountComments, repliesPerPage, filter} = options || {}
+  let {comment, sortType, accountName, flat, accountComments, repliesPerPage, filter} = options || {}
   if (!sortType) {
     sortType = 'best'
   }
@@ -198,23 +198,23 @@ export function useReplies(options?: UseRepliesOptions): UseRepliesResult {
     accountComments = true
   }
 
-  validator.validateUseRepliesArguments(commentCid, sortType, accountName, flat, accountComments, repliesPerPage, filter)
+  validator.validateUseRepliesArguments(comment?.cid, sortType, accountName, flat, accountComments, repliesPerPage, filter)
   const account = useAccount({accountName})
-  const addFeedToStore = useRepliesStore((state: RepliesState) => state.addFeedToStore)
+  const addFeedToStoreOrUpdateComment = useRepliesStore((state: RepliesState) => state.addFeedToStoreOrUpdateComment)
   const incrementFeedPageNumber = useRepliesStore((state: RepliesState) => state.incrementFeedPageNumber)
   const resetFeed = useRepliesStore((state: RepliesState) => state.resetFeed)
-  const repliesFeedName = useRepliesFeedName(account?.id, commentCid, sortType, flat, accountComments, repliesPerPage, filter)
+  const repliesFeedName = useRepliesFeedName(account?.id, comment?.cid, sortType, flat, accountComments, repliesPerPage, filter)
   const [errors, setErrors] = useState<Error[]>([])
 
   // add replies to store
   useEffect(() => {
-    if (!commentCid || !account) {
+    if (!comment?.cid || !account || !comment) {
       return
     }
-    addFeedToStore(repliesFeedName, commentCid, sortType, account, flat, accountComments, repliesPerPage, filter).catch((error: unknown) =>
-      log.error('useReplies addFeedToStore error', {repliesFeedName, error})
+    addFeedToStoreOrUpdateComment(repliesFeedName, comment, sortType, account, flat, accountComments, repliesPerPage, filter).catch((error: unknown) =>
+      log.error('useReplies addFeedToStoreOrUpdateComment error', {repliesFeedName, error})
     )
-  }, [repliesFeedName])
+  }, [repliesFeedName, comment])
 
   const replies = useRepliesStore((state: RepliesState) => state.loadedFeeds[repliesFeedName || ''])
   const bufferedReplies = useRepliesStore((state: RepliesState) => state.bufferedFeeds[repliesFeedName || ''])
@@ -224,14 +224,14 @@ export function useReplies(options?: UseRepliesOptions): UseRepliesResult {
   if (!repliesFeedName || typeof hasMore !== 'boolean') {
     hasMore = true
   }
-  // if the replies is not yet defined, but no comment cid, doesn't have more
-  if (!commentCid) {
+  // if the replies is not yet defined, but no comment, doesn't have more
+  if (!comment) {
     hasMore = false
   }
 
   const loadMore = async () => {
     try {
-      if (!commentCid || !account) {
+      if (!comment?.cid || !account) {
         throw Error('useReplies cannot load more replies not initalized yet')
       }
       incrementFeedPageNumber(repliesFeedName)
@@ -244,7 +244,7 @@ export function useReplies(options?: UseRepliesOptions): UseRepliesResult {
 
   const reset = async () => {
     try {
-      if (!commentCid || !account) {
+      if (!comment?.cid || !account) {
         throw Error('useReplies cannot reset replies not initalized yet')
       }
       resetFeed(repliesFeedName)
@@ -255,11 +255,11 @@ export function useReplies(options?: UseRepliesOptions): UseRepliesResult {
     }
   }
 
-  if (account && commentCid) {
+  if (account && comment?.cid) {
     log('useReplies', {
       repliesLength: replies?.length || 0,
       hasMore,
-      commentCid,
+      commentCid: comment.cid,
       sortType,
       account,
       repliesStoreOptions: useRepliesStore.getState().feedsOptions,

src/hooks/feeds/feeds.test.ts

@@ -592,7 +592,7 @@ describe('feeds', () => {
 
     test('dynamic filter', async () => {
       const createCidMatchFilter = (cid: string) => ({
-        filter: (comment: Comment) => !!comment.cid.match(cid),
+        filter: (comment: Comment) => !!comment?.cid?.match(cid),
         key: `cid-match-${cid}`,
       })
 
@@ -601,19 +601,19 @@ describe('feeds', () => {
         filter: createCidMatchFilter('13'),
       })
       await waitFor(() => rendered.result.current.feed?.length > 0)
-      expect(rendered.result.current.feed[0].cid).match(/13$/)
-      expect(rendered.result.current.feed[1].cid).match(/13$/)
-      expect(rendered.result.current.feed[2].cid).match(/13$/)
+      expect(rendered.result.current.feed[0].cid).toMatch(/13$/)
+      expect(rendered.result.current.feed[1].cid).toMatch(/13$/)
+      expect(rendered.result.current.feed[2].cid).toMatch(/13$/)
       expect(Object.keys(feedsStore.getState().feedsOptions).length).toBe(1)
 
       rendered.rerender({
         subplebbitAddresses: ['subplebbit address 1', 'subplebbit address 2', 'subplebbit address 3'],
         filter: createCidMatchFilter('14'),
       })
       await waitFor(() => rendered.result.current.feed?.length > 0)
-      expect(rendered.result.current.feed[0].cid).match(/14$/)
-      expect(rendered.result.current.feed[1].cid).match(/14$/)
-      expect(rendered.result.current.feed[2].cid).match(/14$/)
+      expect(rendered.result.current.feed[0].cid).toMatch(/14$/)
+      expect(rendered.result.current.feed[1].cid).toMatch(/14$/)
+      expect(rendered.result.current.feed[2].cid).toMatch(/14$/)
       expect(Object.keys(feedsStore.getState().feedsOptions).length).toBe(2)
     })
 

src/lib/utils/utils.ts

@@ -95,6 +95,7 @@ export const flattenCommentsPages = (pageInstanceOrPagesInstance: any) => {
     // @ts-ignore
     uniqueFlattened.push(flattenedCommentsObject[cid])
   }
+
   return uniqueFlattened
 }
 

src/lib/validator.ts

@@ -282,7 +282,7 @@ export const validateRepliesSortType = (sortType: any) => {
 }
 export const validateUseRepliesArguments = (commentCid?: any, sortType?: any, accountName?: any, flat?: any, accountComments?: any, postsPerPage?: any, filter?: any) => {
   if (commentCid) {
-    assert(typeof commentCid === 'string', `useReplies commentCid argument '${commentCid}' not a string`)
+    assert(typeof commentCid === 'string', `useReplies comment.cid argument '${commentCid}' not a string`)
   }
   assert(repliesSortTypes.has(sortType), `useReplies sortType argument '${sortType}' invalid`)
   if (accountName) {

src/stores/feeds/feeds-store.ts

@@ -36,9 +36,6 @@ export const defaultPostsPerPage = 25
 // keep large buffer because fetching cids is slow
 export const subplebbitPostsLeftBeforeNextPage = 50
 
-// reset all event listeners in between tests
-export const listeners: any = []
-
 export type FeedsState = {
   feedsOptions: FeedsOptions
   bufferedFeeds: Feeds
@@ -92,7 +89,7 @@ const feedsStore = createStore<FeedsState>((setState: Function, getState: Functi
 
     const {feedsOptions, updateFeeds} = getState()
     // feed is in store already, do nothing
-    // if the feed already exist but is at page 1, reset it to page 1
+    // if the feed already exist but is at page 0, reset it to page 1
     if (feedsOptions[feedName] && feedsOptions[feedName].pageNumber !== 0) {
       return
     }
@@ -446,8 +443,6 @@ export const resetFeedsStore = async () => {
   previousFeedsSubplebbitsPostsPagesFirstUpdatedAts = ''
   previousSubplebbitsPages = {}
   updateFeedsPending = false
-  // remove all event listeners
-  listeners.forEach((listener: any) => listener.removeAllListeners())
   // destroy all component subscriptions to the store
   feedsStore.destroy()
   // restore original state

src/stores/replies-pages/replies-pages-store.ts

@@ -4,7 +4,7 @@ import Logger from '@plebbit/plebbit-logger'
 const log = Logger('plebbit-react-hooks:replies:stores')
 import {RepliesPage, RepliesPages, Account, Comment, Comments} from '../../types'
 import accountsStore from '../accounts'
-import commentsStore, {CommentsState} from '../comments'
+import repliesCommentsStore, {RepliesCommentsState} from '../replies/replies-comments-store'
 import localForageLru from '../../lib/localforage-lru'
 import createStore from 'zustand'
 import assert from 'assert'
@@ -149,7 +149,7 @@ const repliesPagesStore = createStore<RepliesPagesState>((setState: Function, ge
 
 // set clients states on comments store so the frontend can display it, dont persist in db because a reload cancels updating
 const onCommentRepliesClientsStateChange = (commentCid: string) => (clientState: string, clientType: string, sortType: string, clientUrl: string) => {
-  commentsStore.setState((state: CommentsState) => {
+  repliesCommentsStore.setState((state: RepliesCommentsState) => {
     // make sure not undefined, sometimes happens in e2e tests
     if (!state.comments[commentCid]) {
       return {}

src/stores/replies/replies-comments-store.ts

@@ -0,0 +1,22 @@
+import {Comment, Comments} from '../../types'
+import createStore from 'zustand'
+
+export type RepliesCommentsState = {
+  comments: Comments
+  addCommentToStoreOrUpdateComment: Function
+}
+
+const repliesCommentsStore = createStore<RepliesCommentsState>((setState: Function, getState: Function) => ({
+  comments: {},
+  addCommentToStoreOrUpdateComment(comment: Comment) {
+    setState((state: RepliesCommentsState) => {
+      // updatedAt hasn't changed so no need to update the comment
+      if (state.comments[comment.cid] && comment.updatedAt <= state.comments[comment.cid].updatedAt) {
+        return
+      }
+      return {comments: {...state.comments, [comment.cid]: comment}}
+    })
+  },
+}))
+
+export default repliesCommentsStore