Skip to content

Commit 907c71d

Browse files
committed
cleanup: better naming; fix: usecallback on live comments component; fix leak on useEffect because of missing sort
atomic apollo cache manipulations; manage top sort not being present in item query cache queue nested comments without a parent, retry on the next poll fix commit messages
1 parent 274927d commit 907c71d

File tree

2 files changed

+70
-96
lines changed

2 files changed

+70
-96
lines changed

components/comments.js

Lines changed: 3 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,8 @@ import { useRouter } from 'next/router'
99
import MoreFooter from './more-footer'
1010
import { FULL_COMMENTS_THRESHOLD } from '@/lib/constants'
1111
import useLiveComments, { ShowNewComments } from './use-live-comments'
12-
import ActionTooltip from './action-tooltip'
13-
import classNames from 'classnames'
1412

15-
export function CommentsHeader ({ handleSort, pinned, bio, parentCreatedAt, commentSats, livePolling, setLivePolling }) {
13+
export function CommentsHeader ({ handleSort, pinned, bio, parentCreatedAt, commentSats }) {
1614
const router = useRouter()
1715
const sort = router.query.sort || defaultCommentSort(pinned, bio, parentCreatedAt)
1816

@@ -31,25 +29,6 @@ export function CommentsHeader ({ handleSort, pinned, bio, parentCreatedAt, comm
3129
<Nav.Item className='text-muted'>
3230
{numWithUnits(commentSats)}
3331
</Nav.Item>
34-
{livePolling
35-
? (
36-
<Nav.Item className='ps-2'>
37-
<ActionTooltip notForm overlayText='comments are live'>
38-
<div className={styles.newCommentDot} />
39-
</ActionTooltip>
40-
</Nav.Item>
41-
)
42-
: (
43-
<Nav.Item className='ps-2'>
44-
<ActionTooltip notForm overlayText='click to resume live comments'>
45-
<div
46-
className={classNames(styles.newCommentDot, styles.paused)}
47-
onClick={() => setLivePolling(true)}
48-
style={{ cursor: 'pointer' }}
49-
/>
50-
</ActionTooltip>
51-
</Nav.Item>
52-
)}
5332
<div className='ms-auto d-flex'>
5433
<Nav.Item>
5534
<Nav.Link
@@ -92,7 +71,7 @@ export default function Comments ({
9271
// TODO-LIVE: ok now item updates thanks to sort awareness, but please CLEAN THIS UP
9372
const sort = router.query.sort || defaultCommentSort(pinned, bio, parentCreatedAt)
9473
// update item.newComments in cache
95-
const { polling: livePolling, setPolling: setLivePolling } = useLiveComments(parentId, lastCommentAt || parentCreatedAt, sort)
74+
useLiveComments(parentId, lastCommentAt || parentCreatedAt, sort)
9675

9776
const pins = useMemo(() => comments?.filter(({ position }) => !!position).sort((a, b) => a.position - b.position), [comments])
9877

@@ -101,7 +80,7 @@ export default function Comments ({
10180
{comments?.length > 0
10281
? <CommentsHeader
10382
commentSats={commentSats} parentCreatedAt={parentCreatedAt}
104-
pinned={pinned} bio={bio} livePolling={livePolling} setLivePolling={setLivePolling} handleSort={sort => {
83+
pinned={pinned} bio={bio} handleSort={sort => {
10584
const { commentsViewedAt, commentId, ...query } = router.query
10685
delete query.nodata
10786
router.push({

components/use-live-comments.js

Lines changed: 67 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -2,101 +2,97 @@ import { useQuery, useApolloClient } from '@apollo/client'
22
import { SSR } from '../lib/constants'
33
import { GET_NEW_COMMENTS, COMMENT_WITH_NEW } from '../fragments/comments'
44
import { ITEM_FULL } from '../fragments/items'
5-
import { useEffect, useState } from 'react'
5+
import { useCallback, useEffect, useState, useRef } from 'react'
66
import styles from './comment.module.css'
77

88
const POLL_INTERVAL = 1000 * 10 // 10 seconds
9-
const ACTIVITY_TIMEOUT = 1000 * 60 * 30 // 30 minutes
10-
const ACTIVITY_CHECK_INTERVAL = 1000 * 60 // 1 minute
9+
10+
function itemUpdateQuery (client, id, sort, fn) {
11+
client.cache.updateQuery({
12+
query: ITEM_FULL,
13+
variables: sort === 'top' ? { id } : { id, sort }
14+
}, (data) => fn(data))
15+
}
16+
17+
function commentUpdateFragment (client, id, fn) {
18+
client.cache.updateFragment({
19+
id: `Item:${id}`,
20+
fragment: COMMENT_WITH_NEW,
21+
fragmentName: 'CommentWithNew'
22+
}, (data) => fn(data))
23+
}
1124

1225
export default function useLiveComments (rootId, after, sort) {
1326
const client = useApolloClient()
1427
const [latest, setLatest] = useState(after)
15-
const [polling, setPolling] = useState(true)
16-
const [engagedAt, setEngagedAt] = useState(new Date())
17-
18-
// reset engagedAt when polling is toggled
19-
useEffect(() => {
20-
if (polling) {
21-
setEngagedAt(new Date())
22-
}
23-
}, [polling])
24-
25-
useEffect(() => {
26-
const checkActivity = () => {
27-
const now = new Date()
28-
const timeSinceEngaged = now.getTime() - engagedAt.getTime()
29-
const isActive = document.visibilityState === 'visible'
30-
31-
// poll only if the user is active and has been active in the last 30 minutes
32-
if (timeSinceEngaged < ACTIVITY_TIMEOUT) {
33-
setPolling(isActive)
34-
} else {
35-
setPolling(false)
36-
}
37-
}
38-
39-
// check activity every minute
40-
const interval = setInterval(checkActivity, ACTIVITY_CHECK_INTERVAL)
41-
// check activity also on visibility change
42-
document.addEventListener('visibilitychange', checkActivity)
43-
44-
return () => {
45-
// cleanup
46-
document.removeEventListener('visibilitychange', checkActivity)
47-
clearInterval(interval)
48-
}
49-
}, [engagedAt])
28+
const queuedCommentsRef = useRef([])
5029

5130
const { data } = useQuery(GET_NEW_COMMENTS, SSR
5231
? {}
5332
: {
54-
pollInterval: polling ? POLL_INTERVAL : null,
33+
pollInterval: POLL_INTERVAL,
5534
variables: { rootId, after: latest }
5635
})
5736

5837
useEffect(() => {
5938
if (!data?.newComments) return
6039

61-
cacheNewComments(client, rootId, data.newComments.comments, sort)
62-
// check new comments created after the latest new comment
63-
setLatest(prevLatest => getLatestCommentCreatedAt(data.newComments.comments, prevLatest))
64-
}, [data, client, rootId])
40+
// live comments can be orphans if the parent comment is not in the cache
41+
// queue them up and retry later, when the parent decides they want the children.
42+
const allComments = [...queuedCommentsRef.current, ...data.newComments.comments]
43+
const { queuedComments } = cacheNewComments(client, rootId, allComments, sort)
6544

66-
return { polling, setPolling }
45+
// keep the queued comments in the ref for the next poll
46+
queuedCommentsRef.current = queuedComments
47+
48+
// update latest timestamp to the latest comment created at
49+
setLatest(prevLatest => getLatestCommentCreatedAt(data.newComments.comments, prevLatest))
50+
}, [data, client, rootId, sort])
6751
}
6852

6953
function cacheNewComments (client, rootId, newComments, sort) {
54+
const queuedComments = []
55+
7056
for (const newComment of newComments) {
57+
console.log('newComment', newComment)
7158
const { parentId } = newComment
7259
const topLevel = Number(parentId) === Number(rootId)
7360

7461
// if the comment is a top level comment, update the item
7562
if (topLevel) {
76-
client.cache.updateQuery({
77-
query: ITEM_FULL,
78-
variables: { id: rootId, sort } // TODO-LIVE: ok now item updates thanks to sort awareness, but please CLEAN THIS UP
79-
}, (data) => {
63+
console.log('topLevel', topLevel)
64+
itemUpdateQuery(client, rootId, sort, (data) => {
8065
if (!data) return data
81-
// we return the entire item, not just the newComments
82-
return { item: mergeNewComments(data?.item, newComment) }
66+
return { item: mergeNewComment(data?.item, newComment) }
8367
})
8468
} else {
85-
// if the comment is a reply, update the parent comment
86-
client.cache.updateFragment({
69+
// check if parent exists in cache before attempting update
70+
const parentExists = client.cache.readFragment({
8771
id: `Item:${parentId}`,
8872
fragment: COMMENT_WITH_NEW,
8973
fragmentName: 'CommentWithNew'
90-
}, (data) => {
91-
if (!data) return data
92-
// here we return the parent comment with the new comment added
93-
return mergeNewComments(data, newComment)
9474
})
75+
76+
if (parentExists) {
77+
// if the comment is a reply, update the parent comment
78+
console.log('reply', parentId)
79+
commentUpdateFragment(client, parentId, (data) => {
80+
if (!data) return data
81+
return mergeNewComment(data, newComment)
82+
})
83+
} else {
84+
// parent not in cache, queue for retry
85+
queuedComments.push(newComment)
86+
}
9587
}
9688
}
89+
90+
return { queuedComments }
9791
}
9892

99-
function mergeNewComments (item, newComment) {
93+
// merge new comment into item's newComments
94+
// if the new comment is already in item's newComments or existing comments, do nothing
95+
function mergeNewComment (item, newComment) {
10096
const existingNewComments = item.newComments || []
10197
const existingComments = item.comments?.comments || []
10298

@@ -122,42 +118,41 @@ function getLatestCommentCreatedAt (comments, latest) {
122118
export function ShowNewComments ({ newComments = [], itemId, topLevel = false, sort }) {
123119
const client = useApolloClient()
124120

125-
const showNewComments = () => {
121+
const showNewComments = useCallback(() => {
126122
if (topLevel) {
127-
client.cache.updateQuery({
128-
query: ITEM_FULL,
129-
variables: { id: itemId, sort } // TODO-LIVE: ok now item updates thanks to sort awareness, but please CLEAN THIS UP
130-
}, (data) => {
123+
console.log('topLevel', topLevel)
124+
itemUpdateQuery(client, itemId, sort, (data) => {
125+
console.log('data', data)
131126
if (!data) return data
132127
const { item } = data
133128

134129
return {
135130
item: {
136131
...item,
137-
comments: dedupeComments(item.comments, newComments),
132+
comments: injectComments(item.comments, newComments),
138133
newComments: []
139134
}
140135
}
141136
})
142137
} else {
143-
client.cache.updateFragment({
144-
id: `Item:${itemId}`,
145-
fragment: COMMENT_WITH_NEW,
146-
fragmentName: 'CommentWithNew'
147-
}, (data) => {
138+
console.log('reply', itemId)
139+
commentUpdateFragment(client, itemId, (data) => {
140+
console.log('data', data)
148141
if (!data) return data
149142

150143
return {
151144
...data,
152-
comments: dedupeComments(data.comments, newComments),
145+
comments: injectComments(data.comments, newComments),
153146
newComments: []
154147
}
155148
})
156149
}
157-
}
150+
}, [client, itemId, newComments, topLevel, sort])
158151

159-
const dedupeComments = (existingComments = [], newComments = []) => {
160-
const existingIds = new Set(existingComments.comments?.map(c => c.id))
152+
// inject new comments into existing comments
153+
// if the new comment is already in existing comments, do nothing
154+
const injectComments = (existingComments = [], newComments = []) => {
155+
const existingIds = new Set(existingComments.comments.map(c => c.id))
161156
const filteredNew = newComments.filter(c => !existingIds.has(c.id))
162157
return {
163158
...existingComments,

0 commit comments

Comments
 (0)