Skip to content

Commit efb12d6

Browse files
committed
feat: live comments indicator for bottomed-out replies, ncomments updates; fix: nested comment structures
- new comments indicator for bottomed-out replies - ncomments sync for parent and its ancestors - limited comments fragment for comments that don't have CommentsRecursive - reduce cache complexity by removing useless roundtrips ux: live comments indicator on bottomedOut replies fix: dedupe newComments before displaying ShowNewComments to avoid false positives enhance: store ids of new comments in the cache, instead of carrying full comments that would get discarded anyway hotfix: newComments deduplication ID mismatch, filter null comments from freshNewComments fix: ncomments not updating for all comment levels; refactor: share Reply update ancestors' ncomments function with ShowNewComments cleanup: better naming to indicate the total number of comments including nested comments fix: increment parent comment ncomments cleanup: Items that will have comments will always have a structure where item.comments is true cleanup: reduce code complexity checking the nested comment update result instead of preventively reading the fragment cleanup: avoid double-updating ncomments on parent fix: don't use CommentsRecursive for bottomed-out comments cleanup: better fragment naming; add TODO for absolute bottom comments
1 parent 05785db commit efb12d6

File tree

8 files changed

+124
-65
lines changed

8 files changed

+124
-65
lines changed

components/comment.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -263,7 +263,7 @@ export default function Comment ({
263263
{root.bounty && !bountyPaid && <PayBounty item={item} />}
264264
<div className='ms-auto'>
265265
{item.newComments?.length > 0 && (
266-
<ShowNewComments newComments={item.newComments} itemId={item.id} />
266+
<ShowNewComments comments={item.comments.comments} newComments={item.newComments} itemId={item.id} />
267267
)}
268268
</div>
269269
</Reply>}
@@ -310,8 +310,9 @@ function ReplyOnAnotherPage ({ item }) {
310310
}
311311

312312
return (
313-
<Link href={`/items/${rootId}?commentId=${item.id}`} as={`/items/${rootId}`} className='d-block pb-2 fw-bold text-muted'>
313+
<Link href={`/items/${rootId}?commentId=${item.id}`} as={`/items/${rootId}`} className='pb-2 fw-bold d-flex align-items-center gap-2 text-muted'>
314314
{text}
315+
{item.newComments?.length > 0 && <div className={styles.newCommentDot} />}
315316
</Link>
316317
)
317318
}

components/comments.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ export default function Comments ({
9393
/>
9494
: null}
9595
{newComments?.length > 0 && (
96-
<ShowNewComments topLevel newComments={newComments} itemId={parentId} sort={router.query.sort} />
96+
<ShowNewComments topLevel comments={comments} newComments={newComments} itemId={parentId} sort={router.query.sort} />
9797
)}
9898
{pins.map(item => (
9999
<Fragment key={item.id}>

components/item-full.js

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -182,18 +182,19 @@ export default function ItemFull ({ item, fetchMoreComments, bio, rank, ...props
182182
? <BioItem item={item} {...props} />
183183
: <TopLevelItem item={item} {...props} />}
184184
</div>)}
185-
<div className={styles.comments}>
186-
<Comments
187-
parentId={item.id} parentCreatedAt={item.createdAt}
188-
pinned={item.position} bio={bio} commentSats={item.commentSats}
189-
ncomments={item.ncomments}
190-
comments={item.comments.comments}
191-
commentsCursor={item.comments.cursor}
192-
fetchMoreComments={fetchMoreComments}
193-
newComments={item.newComments}
194-
lastCommentAt={item.lastCommentAt}
195-
/>
196-
</div>
185+
{item.comments &&
186+
<div className={styles.comments}>
187+
<Comments
188+
parentId={item.id} parentCreatedAt={item.createdAt}
189+
pinned={item.position} bio={bio} commentSats={item.commentSats}
190+
ncomments={item.ncomments}
191+
comments={item.comments.comments}
192+
commentsCursor={item.comments.cursor}
193+
fetchMoreComments={fetchMoreComments}
194+
newComments={item.newComments}
195+
lastCommentAt={item.lastCommentAt}
196+
/>
197+
</div>}
197198
</CarouselProvider>
198199
</RootProvider>
199200
</>

components/reply.js

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import { useRoot } from './root'
1313
import { CREATE_COMMENT } from '@/fragments/paidAction'
1414
import useItemSubmit from './use-item-submit'
1515
import gql from 'graphql-tag'
16+
import { updateAncestorsCommentCount } from '@/lib/comments'
1617

1718
export default forwardRef(function Reply ({
1819
item,
@@ -82,17 +83,7 @@ export default forwardRef(function Reply ({
8283
const ancestors = item.path.split('.')
8384

8485
// update all ancestors
85-
ancestors.forEach(id => {
86-
cache.modify({
87-
id: `Item:${id}`,
88-
fields: {
89-
ncomments (existingNComments = 0) {
90-
return existingNComments + 1
91-
}
92-
},
93-
optimistic: true
94-
})
95-
})
86+
updateAncestorsCommentCount(cache, ancestors, 1)
9687

9788
// so that we don't see indicator for our own comments, we record this comments as the latest time
9889
// but we also have record num comments, in case someone else commented when we did

components/show-new-comments.js

Lines changed: 40 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,60 +1,79 @@
1-
import { useCallback } from 'react'
1+
import { useCallback, useMemo } from 'react'
22
import { useApolloClient } from '@apollo/client'
3-
import { COMMENT_WITH_NEW } from '../fragments/comments'
3+
import { COMMENT_WITH_NEW_RECURSIVE } from '../fragments/comments'
44
import styles from './comment.module.css'
55
import { itemUpdateQuery, commentUpdateFragment } from './use-live-comments'
6+
import { updateAncestorsCommentCount } from '@/lib/comments'
67

78
function prepareComments (client, newComments) {
89
return (data) => {
9-
// TODO: it might be sane to pass the cache ref to the ShowNewComments component
10-
// TODO: and use it to read the latest newComments from the cache
11-
// newComments can have themselves new comments between the time the button is clicked and the query is executed
12-
// so we need to read the latest newComments from the cache
13-
const freshNewComments = newComments.map(c => {
10+
// newComments is an array of comment ids that allows us
11+
// to read the latest newComments from the cache, guaranteeing that we're not reading stale data
12+
const freshNewComments = newComments.map(id => {
1413
const fragment = client.cache.readFragment({
15-
id: `Item:${c.id}`,
16-
fragment: COMMENT_WITH_NEW,
17-
fragmentName: 'CommentWithNew'
14+
id: `Item:${id}`,
15+
fragment: COMMENT_WITH_NEW_RECURSIVE,
16+
fragmentName: 'CommentWithNewRecursive'
1817
})
19-
// if the comment is not in the cache, return the original comment
20-
return fragment || c
21-
})
2218

23-
// count the total number of comments including nested comments
24-
let ncomments = data.ncomments + freshNewComments.length
19+
if (!fragment) {
20+
return null
21+
}
22+
23+
return fragment
24+
}).filter(Boolean)
25+
26+
// count the total number of new comments including its nested new comments
27+
let totalNComments = freshNewComments.length
2528
for (const comment of freshNewComments) {
26-
ncomments += (comment.ncomments || 0)
29+
totalNComments += (comment.ncomments || 0)
2730
}
2831

32+
// update all ancestors, but not the item itself
33+
const ancestors = data.path.split('.').slice(0, -1)
34+
updateAncestorsCommentCount(client.cache, ancestors, totalNComments)
35+
2936
return {
3037
...data,
3138
comments: { ...data.comments, comments: [...freshNewComments, ...data.comments.comments] },
32-
ncomments,
39+
ncomments: data.ncomments + totalNComments,
3340
newComments: []
3441
}
3542
}
3643
}
3744

3845
// ShowNewComments is a component that dedupes, refreshes and injects newComments into the comments field
39-
export function ShowNewComments ({ newComments = [], itemId, topLevel = false, sort }) {
46+
export function ShowNewComments ({ topLevel = false, comments, newComments = [], itemId, sort }) {
4047
const client = useApolloClient()
4148

49+
const dedupedNewComments = useMemo(() => {
50+
const existingIds = new Set(comments.map(c => c.id))
51+
return newComments.filter(id => !existingIds.has(id))
52+
}, [newComments, comments])
53+
4254
const showNewComments = useCallback(() => {
43-
const payload = prepareComments(client, newComments)
55+
// fetch the latest version of the comments from the cache by their ids
56+
const payload = prepareComments(client, dedupedNewComments)
4457

4558
if (topLevel) {
4659
itemUpdateQuery(client, itemId, sort, payload)
4760
} else {
4861
commentUpdateFragment(client, itemId, payload)
4962
}
50-
}, [client, itemId, newComments, topLevel, sort])
63+
}, [client, itemId, dedupedNewComments, topLevel, sort])
64+
65+
if (dedupedNewComments.length === 0) {
66+
return null
67+
}
5168

5269
return (
5370
<div
5471
onClick={showNewComments}
5572
className={`${topLevel && `d-block fw-bold ${styles.comment} pb-2`} d-flex align-items-center gap-2 px-3 pointer`}
5673
>
57-
{newComments.length > 1 ? `${newComments.length} new comments` : 'show new comment'}
74+
{dedupedNewComments.length > 1
75+
? `${dedupedNewComments.length} new comments`
76+
: 'show new comment'}
5877
<div className={styles.newCommentDot} />
5978
</div>
6079
)

components/use-live-comments.js

Lines changed: 24 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { useQuery, useApolloClient } from '@apollo/client'
22
import { SSR } from '../lib/constants'
3-
import { GET_NEW_COMMENTS, COMMENT_WITH_NEW } from '../fragments/comments'
3+
import { GET_NEW_COMMENTS, COMMENT_WITH_NEW_LIMITED, COMMENT_WITH_NEW_RECURSIVE } from '../fragments/comments'
44
import { ITEM_FULL } from '../fragments/items'
55
import { useEffect, useRef, useState } from 'react'
66

@@ -60,14 +60,29 @@ export function itemUpdateQuery (client, id, sort, fn) {
6060

6161
// update the newComments field of a nested comment fragment
6262
export function commentUpdateFragment (client, id, fn) {
63-
client.cache.updateFragment({
63+
let result = client.cache.updateFragment({
6464
id: `Item:${id}`,
65-
fragment: COMMENT_WITH_NEW,
66-
fragmentName: 'CommentWithNew'
65+
fragment: COMMENT_WITH_NEW_RECURSIVE,
66+
fragmentName: 'CommentWithNewRecursive'
6767
}, (data) => {
6868
if (!data) return data
6969
return fn(data)
7070
})
71+
72+
// sometimes comments can reach their depth limit, and lack adherence to the CommentsRecursive fragment
73+
// for this reason, we update the fragment with a limited version that only includes the CommentFields fragment
74+
if (!result) {
75+
result = client.cache.updateFragment({
76+
id: `Item:${id}`,
77+
fragment: COMMENT_WITH_NEW_LIMITED,
78+
fragmentName: 'CommentWithNewLimited'
79+
}, (data) => {
80+
if (!data) return data
81+
return fn(data)
82+
})
83+
}
84+
85+
return result
7186
}
7287

7388
function cacheNewComments (client, rootId, newComments, sort) {
@@ -83,17 +98,10 @@ function cacheNewComments (client, rootId, newComments, sort) {
8398
itemUpdateQuery(client, rootId, sort, (data) => mergeNewComment(data, newComment))
8499
} else {
85100
// if the comment is a reply, update the parent comment
86-
// but first check if parent exists in cache before attempting update
87-
const parentExists = client.cache.readFragment({
88-
id: `Item:${parentId}`,
89-
fragment: COMMENT_WITH_NEW,
90-
fragmentName: 'CommentWithNew'
91-
})
101+
// merge the new comment into the parent comment's newComments field, checking for duplicates
102+
const result = commentUpdateFragment(client, parentId, (data) => mergeNewComment(data, newComment))
92103

93-
if (parentExists) {
94-
// merge the new comment into the parent comment's newComments field, checking for duplicates
95-
commentUpdateFragment(client, parentId, (data) => mergeNewComment(data, newComment))
96-
} else {
104+
if (!result) {
97105
// parent not in cache, queue for retry
98106
queuedComments.push(newComment)
99107
}
@@ -110,11 +118,11 @@ function mergeNewComment (item, newComment) {
110118
const existingComments = item.comments?.comments || []
111119

112120
// is the incoming new comment already in item's new comments or existing comments?
113-
if (existingNewComments.some(c => c.id === newComment.id) || existingComments.some(c => c.id === newComment.id)) {
121+
if (existingNewComments.includes(newComment.id) || existingComments.some(c => c.id === newComment.id)) {
114122
return item
115123
}
116124

117-
return { ...item, newComments: [...existingNewComments, newComment] }
125+
return { ...item, newComments: [...existingNewComments, newComment.id] }
118126
}
119127

120128
function getLatestCommentCreatedAt (comments, latest) {

fragments/comments.js

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,11 +118,11 @@ export const COMMENTS = gql`
118118
}
119119
}`
120120

121-
export const COMMENT_WITH_NEW = gql`
121+
export const COMMENT_WITH_NEW_RECURSIVE = gql`
122122
${COMMENT_FIELDS}
123123
${COMMENTS}
124124
125-
fragment CommentWithNew on Item {
125+
fragment CommentWithNewRecursive on Item {
126126
...CommentFields
127127
comments {
128128
comments {
@@ -133,6 +133,31 @@ export const COMMENT_WITH_NEW = gql`
133133
}
134134
`
135135

136+
export const COMMENT_WITH_NEW_LIMITED = gql`
137+
${COMMENT_FIELDS}
138+
139+
fragment CommentWithNewLimited on Item {
140+
...CommentFields
141+
comments {
142+
comments {
143+
...CommentFields
144+
}
145+
}
146+
newComments @client
147+
}
148+
`
149+
150+
// TODO: fragment for comments without item.comments field
151+
// TODO: remove if useless to pursue
152+
export const COMMENT_WITH_NEW = gql`
153+
${COMMENT_FIELDS}
154+
155+
fragment CommentWithNew on Item {
156+
...CommentFields
157+
newComments @client
158+
}
159+
`
160+
136161
export const GET_NEW_COMMENTS = gql`
137162
${COMMENTS}
138163

lib/comments.js

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
export function updateAncestorsCommentCount (cache, ancestors, increment) {
2+
// update all ancestors
3+
ancestors.forEach(id => {
4+
cache.modify({
5+
id: `Item:${id}`,
6+
fields: {
7+
ncomments (existingNComments = 0) {
8+
return existingNComments + increment
9+
}
10+
},
11+
optimistic: true
12+
})
13+
})
14+
}

0 commit comments

Comments
 (0)