From 0459148e303dee9e9887c533abcff946cb35da6b Mon Sep 17 00:00:00 2001 From: infinite-persistence Date: Mon, 11 Oct 2021 14:02:17 +0800 Subject: [PATCH] Comment-selectors: fix memoization --- .../channelMentionSuggestions/index.js | 4 +- ui/component/commentsList/index.js | 18 +- ui/component/commentsReplies/index.js | 32 +- ui/page/ownComments/index.js | 4 +- ui/redux/selectors/comments.js | 321 +++++++++--------- 5 files changed, 207 insertions(+), 172 deletions(-) diff --git a/ui/component/channelMentionSuggestions/index.js b/ui/component/channelMentionSuggestions/index.js index d6b4af95b..c1f635011 100644 --- a/ui/component/channelMentionSuggestions/index.js +++ b/ui/component/channelMentionSuggestions/index.js @@ -4,12 +4,12 @@ import { selectSubscriptions } from 'redux/selectors/subscriptions'; import { withRouter } from 'react-router'; import { makeSelectClaimForUri } from 'redux/selectors/claims'; import { doResolveUris } from 'redux/actions/claims'; -import { makeSelectTopLevelCommentsForUri } from 'redux/selectors/comments'; +import { selectTopLevelCommentsForUri } from 'redux/selectors/comments'; import ChannelMentionSuggestions from './view'; const select = (state, props) => { const subscriptionUris = selectSubscriptions(state).map(({ uri }) => uri); - const topLevelComments = makeSelectTopLevelCommentsForUri(props.uri)(state); + const topLevelComments = selectTopLevelCommentsForUri(state, props.uri); const commentorUris = []; // Avoid repeated commentors diff --git a/ui/component/commentsList/index.js b/ui/component/commentsList/index.js index a8a3962d0..4b09b9b4f 100644 --- a/ui/component/commentsList/index.js +++ b/ui/component/commentsList/index.js @@ -1,4 +1,5 @@ import { connect } from 'react-redux'; +import { doResolveUris } from 'redux/actions/claims'; import { makeSelectClaimForUri, makeSelectClaimIsMine, @@ -6,7 +7,7 @@ import { selectMyChannelClaims, } from 'redux/selectors/claims'; import { - makeSelectTopLevelCommentsForUri, + selectTopLevelCommentsForUri, makeSelectTopLevelTotalPagesForUri, selectIsFetchingComments, selectIsFetchingCommentsById, @@ -16,7 +17,7 @@ import { selectMyReacts, makeSelectCommentIdsForUri, selectSettingsByChannelId, - makeSelectPinnedCommentsForUri, + selectPinnedCommentsForUri, } from 'redux/selectors/comments'; import { doCommentReset, doCommentList, doCommentById, doCommentReactList } from 'redux/actions/comments'; import { selectActiveChannelClaim } from 'redux/selectors/app'; @@ -24,11 +25,19 @@ import CommentsList from './view'; const select = (state, props) => { const activeChannelClaim = selectActiveChannelClaim(state); + const topLevelComments = selectTopLevelCommentsForUri(state, props.uri); + + const resolvedComments = + topLevelComments && topLevelComments.length > 0 + ? topLevelComments.filter(({ channel_url }) => makeSelectClaimForUri(channel_url)(state) !== undefined) + : []; + return { + topLevelComments, + resolvedComments, myChannels: selectMyChannelClaims(state), allCommentIds: makeSelectCommentIdsForUri(props.uri)(state), - pinnedComments: makeSelectPinnedCommentsForUri(props.uri)(state), - topLevelComments: makeSelectTopLevelCommentsForUri(props.uri)(state), + pinnedComments: selectPinnedCommentsForUri(state, props.uri), topLevelTotalPages: makeSelectTopLevelTotalPagesForUri(props.uri)(state), totalComments: makeSelectTotalCommentsCountForUri(props.uri)(state), claim: makeSelectClaimForUri(props.uri)(state), @@ -49,6 +58,7 @@ const perform = (dispatch) => ({ fetchComment: (commentId) => dispatch(doCommentById(commentId)), fetchReacts: (commentIds) => dispatch(doCommentReactList(commentIds)), resetComments: (claimId) => dispatch(doCommentReset(claimId)), + doResolveUris: (uris) => dispatch(doResolveUris(uris, true)), }); export default connect(select, perform)(CommentsList); diff --git a/ui/component/commentsReplies/index.js b/ui/component/commentsReplies/index.js index 2b4a38c20..d7ac34b2e 100644 --- a/ui/component/commentsReplies/index.js +++ b/ui/component/commentsReplies/index.js @@ -1,15 +1,27 @@ import { connect } from 'react-redux'; -import { makeSelectClaimIsMine, selectMyChannelClaims } from 'redux/selectors/claims'; -import { selectIsFetchingCommentsByParentId, makeSelectRepliesForParentId } from 'redux/selectors/comments'; +import { doResolveUris } from 'redux/actions/claims'; +import { makeSelectClaimIsMine, selectMyChannelClaims, makeSelectClaimForUri } from 'redux/selectors/claims'; +import { selectIsFetchingCommentsByParentId, selectRepliesForParentId } from 'redux/selectors/comments'; import { selectUserVerifiedEmail } from 'redux/selectors/user'; import CommentsReplies from './view'; -const select = (state, props) => ({ - fetchedReplies: makeSelectRepliesForParentId(props.parentId)(state), - claimIsMine: makeSelectClaimIsMine(props.uri)(state), - commentingEnabled: IS_WEB ? Boolean(selectUserVerifiedEmail(state)) : true, - myChannels: selectMyChannelClaims(state), - isFetchingByParentId: selectIsFetchingCommentsByParentId(state), -}); +const select = (state, props) => { + const fetchedReplies = selectRepliesForParentId(state, props.parentId); + const resolvedReplies = + fetchedReplies && fetchedReplies.length > 0 + ? fetchedReplies.filter(({ channel_url }) => makeSelectClaimForUri(channel_url)(state) !== undefined) + : []; -export default connect(select)(CommentsReplies); + return { + fetchedReplies, + resolvedReplies, + claimIsMine: makeSelectClaimIsMine(props.uri)(state), + userCanComment: IS_WEB ? Boolean(selectUserVerifiedEmail(state)) : true, + myChannels: selectMyChannelClaims(state), + isFetchingByParentId: selectIsFetchingCommentsByParentId(state), + }; +}; + +const perform = (dispatch) => ({ doResolveUris: (uris) => dispatch(doResolveUris(uris, true)) }); + +export default connect(select, perform)(CommentsReplies); diff --git a/ui/page/ownComments/index.js b/ui/page/ownComments/index.js index df53ecfa8..4c41717cb 100644 --- a/ui/page/ownComments/index.js +++ b/ui/page/ownComments/index.js @@ -3,7 +3,7 @@ import { doCommentListOwn, doCommentReset } from 'redux/actions/comments'; import { selectActiveChannelClaim } from 'redux/selectors/app'; import { selectIsFetchingComments, - makeSelectCommentsForUri, + selectCommentsForUri, makeSelectTotalCommentsCountForUri, makeSelectTopLevelTotalPagesForUri, } from 'redux/selectors/comments'; @@ -17,7 +17,7 @@ const select = (state) => { return { activeChannelClaim, - allComments: makeSelectCommentsForUri(uri)(state), + allComments: selectCommentsForUri(state, uri), totalComments: makeSelectTotalCommentsCountForUri(uri)(state), topLevelTotalPages: makeSelectTopLevelTotalPagesForUri(uri)(state), isFetchingComments: selectIsFetchingComments(state), diff --git a/ui/redux/selectors/comments.js b/ui/redux/selectors/comments.js index f71eecdb8..bc53eb785 100644 --- a/ui/redux/selectors/comments.js +++ b/ui/redux/selectors/comments.js @@ -1,5 +1,6 @@ // @flow import { createSelector } from 'reselect'; +import { createCachedSelector } from 're-reselect'; import { selectMutedChannels } from 'redux/selectors/blocked'; import { selectShowMatureContent } from 'redux/selectors/settings'; import { selectBlacklistedOutpointMap, selectFilteredOutpointMap } from 'lbryinc'; @@ -27,26 +28,46 @@ export const selectOthersReactsForComment = (state: State, id: string) => { return state.comments.othersReactsByCommentId && state.comments.othersReactsByCommentId[id]; }; -export const selectPinnedCommentsById = (state: State) => selectState(state).pinnedCommentsById; -export const makeSelectPinnedCommentsForUri = (uri: string) => - createSelector( - selectCommentsByUri, - selectCommentsById, - selectPinnedCommentsById, - (byUri, byId, pinnedCommentsById) => { - const claimId = byUri[uri]; - const pinnedCommentIds = pinnedCommentsById && pinnedCommentsById[claimId]; - const pinnedComments = []; - - if (pinnedCommentIds) { - pinnedCommentIds.forEach((commentId) => { - pinnedComments.push(byId[commentId]); - }); - } - - return pinnedComments; +// previously this used a mapping from claimId -> Array +/* export const selectCommentsById = createSelector( + selectState, + state => state.byId || {} + ); */ +export const selectCommentsByUri = createSelector(selectState, (state) => { + const byUri = state.commentsByUri || {}; + const comments = {}; + Object.keys(byUri).forEach((uri) => { + const claimId = byUri[uri]; + if (claimId === null) { + comments[uri] = null; + } else { + comments[uri] = claimId; } - ); + }); + + return comments; +}); + +export const selectPinnedCommentsById = (state: State) => selectState(state).pinnedCommentsById; +export const selectPinnedCommentsForUri = createCachedSelector( + selectCommentsByUri, + selectCommentsById, + selectPinnedCommentsById, + (state, uri) => uri, + (byUri, byId, pinnedCommentsById, uri) => { + const claimId = byUri[uri]; + const pinnedCommentIds = pinnedCommentsById && pinnedCommentsById[claimId]; + const pinnedComments = []; + + if (pinnedCommentIds) { + pinnedCommentIds.forEach((commentId) => { + pinnedComments.push(byId[commentId]); + }); + } + + return pinnedComments; + } +)((state, uri) => uri); export const selectModerationBlockList = createSelector( (state) => selectState(state).moderationBlockList, @@ -109,22 +130,26 @@ export const selectCommentsByClaimId = createSelector(selectState, selectComment // no superchats? export const selectSuperchatsByUri = (state: State) => selectState(state).superChatsByUri; -export const selectTopLevelCommentsByClaimId = createSelector(selectState, selectCommentsById, (state, byId) => { - const byClaimId = state.topLevelCommentsById || {}; - const comments = {}; +export const selectTopLevelCommentsByClaimId = createSelector( + (state) => selectState(state).topLevelCommentsById, + selectCommentsById, + (topLevelCommentsById, byId) => { + const byClaimId = topLevelCommentsById || {}; + const comments = {}; - // replace every comment_id in the list with the actual comment object - Object.keys(byClaimId).forEach((claimId) => { - const commentIds = byClaimId[claimId]; + // replace every comment_id in the list with the actual comment object + Object.keys(byClaimId).forEach((claimId) => { + const commentIds = byClaimId[claimId]; - comments[claimId] = Array(commentIds === null ? 0 : commentIds.length); - for (let i = 0; i < commentIds.length; i++) { - comments[claimId][i] = byId[commentIds[i]]; - } - }); + comments[claimId] = Array(commentIds === null ? 0 : commentIds.length); + for (let i = 0; i < commentIds.length; i++) { + comments[claimId][i] = byId[commentIds[i]]; + } + }); - return comments; -}); + return comments; + } +); export const makeSelectCommentForCommentId = (commentId: string) => createSelector(selectCommentsById, (comments) => comments[commentId]); @@ -146,26 +171,6 @@ export const selectRepliesByParentId = createSelector(selectState, selectComment return comments; }); -// previously this used a mapping from claimId -> Array -/* export const selectCommentsById = createSelector( - selectState, - state => state.byId || {} -); */ -export const selectCommentsByUri = createSelector(selectState, (state) => { - const byUri = state.commentsByUri || {}; - const comments = {}; - Object.keys(byUri).forEach((uri) => { - const claimId = byUri[uri]; - if (claimId === null) { - comments[uri] = null; - } else { - comments[uri] = claimId; - } - }); - - return comments; -}); - export const selectLinkedCommentAncestors = (state: State) => selectState(state).linkedCommentAncestors; export const makeSelectCommentIdsForUri = (uri: string) => @@ -174,34 +179,48 @@ export const makeSelectCommentIdsForUri = (uri: string) => return state.byId[claimId]; }); +const filterCommentsDepOnList = { + claimsById: selectClaimsById, + myClaims: selectMyActiveClaims, + mutedChannels: selectMutedChannels, + personalBlockList: selectModerationBlockList, + blacklistedMap: selectBlacklistedOutpointMap, + filteredMap: selectFilteredOutpointMap, + showMatureContent: selectShowMatureContent, +}; + +const filterCommentsPropKeys = Object.keys(filterCommentsDepOnList); + export const selectPendingCommentReacts = (state: State) => selectState(state).pendingCommentReactions; export const selectSettingsByChannelId = (state: State) => selectState(state).settingsByChannelId; export const selectFetchingCreatorSettings = (state: State) => selectState(state).fetchingSettings; export const selectFetchingBlockedWords = (state: State) => selectState(state).fetchingBlockedWords; -export const makeSelectCommentsForUri = (uri: string) => - createSelector( - (state) => state, - selectCommentsByClaimId, - selectCommentsByUri, - (state, byClaimId, byUri) => { - const claimId = byUri[uri]; - const comments = byClaimId && byClaimId[claimId]; - return makeSelectFilteredComments(comments, claimId)(state); - } - ); +export const selectCommentsForUri = createCachedSelector( + (state, uri) => uri, + selectCommentsByClaimId, + selectCommentsByUri, + ...Object.values(filterCommentsDepOnList), + (uri, byClaimId, byUri, ...filterInputs) => { + const claimId = byUri[uri]; + const comments = byClaimId && byClaimId[claimId]; + return filterComments(comments, claimId, filterInputs); + } +)((state, uri) => uri); -export const makeSelectTopLevelCommentsForUri = (uri: string) => - createSelector( - (state) => state, - selectTopLevelCommentsByClaimId, - selectCommentsByUri, - (state, byClaimId, byUri) => { - const claimId = byUri[uri]; - const comments = byClaimId && byClaimId[claimId]; - return makeSelectFilteredComments(comments, claimId)(state); - } - ); +export const selectTopLevelCommentsForUri = createCachedSelector( + (state, uri) => uri, + (state, uri, maxCount) => maxCount, + selectTopLevelCommentsByClaimId, + selectCommentsByUri, + ...Object.values(filterCommentsDepOnList), + (uri, maxCount = -1, byClaimId, byUri, ...filterInputs) => { + const claimId = byUri[uri]; + const comments = byClaimId && byClaimId[claimId]; + const filtered = filterComments(comments, claimId, filterInputs); + return maxCount > 0 ? filtered.slice(0, maxCount) : filtered; + } +)((state, uri, maxCount = -1) => `${uri}:${maxCount}`); export const makeSelectTopLevelTotalCommentsForUri = (uri: string) => createSelector(selectState, selectCommentsByUri, (state, byUri) => { @@ -215,99 +234,93 @@ export const makeSelectTopLevelTotalPagesForUri = (uri: string) => return state.topLevelTotalPagesById[claimId] || 0; }); -export const makeSelectRepliesForParentId = (id: string) => - createSelector( - (state) => state, - selectCommentsById, - (state, commentsById) => { - // const claimId = byUri[uri]; // just parentId (id) - const replyIdsByParentId = state.comments.repliesByParentId; - const replyIdsForParent = replyIdsByParentId[id] || []; - if (!replyIdsForParent.length) return null; +export const selectRepliesForParentId = createCachedSelector( + (state, id) => id, + (state) => selectState(state).repliesByParentId, + selectCommentsById, + ...Object.values(filterCommentsDepOnList), + (id, repliesByParentId, commentsById, ...filterInputs) => { + // const claimId = byUri[uri]; // just parentId (id) + const replyIdsForParent = repliesByParentId[id] || []; + if (!replyIdsForParent.length) return null; - const comments = []; - replyIdsForParent.forEach((cid) => { - comments.push(commentsById[cid]); - }); - // const comments = byParentId && byParentId[id]; + const comments = []; + replyIdsForParent.forEach((cid) => { + comments.push(commentsById[cid]); + }); + // const comments = byParentId && byParentId[id]; - return makeSelectFilteredComments(comments)(state); - } - ); + return filterComments(comments, undefined, filterInputs); + } +)((state, id: string) => id); /** - * makeSelectFilteredComments + * filterComments * * @param comments List of comments to filter. * @param claimId The claim that `comments` reside in. + * @oaram filterInputs Values returned by filterCommentsDepOnList. */ -const makeSelectFilteredComments = (comments: Array, claimId?: string) => - createSelector( - selectClaimsById, - selectMyActiveClaims, - selectMutedChannels, - selectModerationBlockList, - selectAdminBlockList, - selectModeratorBlockList, - selectBlacklistedOutpointMap, - selectFilteredOutpointMap, - selectShowMatureContent, - ( - claimsById, - myClaims, - mutedChannels, - personalBlockList, - adminBlockList, - moderatorBlockList, - blacklistedMap, - filteredMap, - showMatureContent - ) => { - return comments - ? comments.filter((comment) => { - if (!comment) { - // It may have been recently deleted after being blocked +const filterComments = (comments: Array, claimId?: string, filterInputs: any) => { + const filterProps = filterInputs.reduce(function (acc, cur, i) { + acc[filterCommentsPropKeys[i]] = cur; + return acc; + }, {}); + + const { + claimsById, + myClaims, + mutedChannels, + personalBlockList, + blacklistedMap, + filteredMap, + showMatureContent, + } = filterProps; + + return comments + ? comments.filter((comment) => { + if (!comment) { + // It may have been recently deleted after being blocked + return false; + } + + const channelClaim = claimsById[comment.channel_id]; + + // Return comment if `channelClaim` doesn't exist so the component knows to resolve the author + if (channelClaim) { + if (myClaims && myClaims.size > 0) { + const claimIsMine = channelClaim.is_my_output || myClaims.has(channelClaim.claim_id); + if (claimIsMine) { + return true; + } + } + + const outpoint = `${channelClaim.txid}:${channelClaim.nout}`; + if (blacklistedMap[outpoint] || filteredMap[outpoint]) { + return false; + } + + if (!showMatureContent) { + const claimIsMature = isClaimNsfw(channelClaim); + if (claimIsMature) { return false; } + } + } - const channelClaim = claimsById[comment.channel_id]; - - // Return comment if `channelClaim` doesn't exist so the component knows to resolve the author - if (channelClaim) { - if (myClaims && myClaims.size > 0) { - const claimIsMine = channelClaim.is_my_output || myClaims.has(channelClaim.claim_id); - if (claimIsMine) { - return true; - } - } - - const outpoint = `${channelClaim.txid}:${channelClaim.nout}`; - if (blacklistedMap[outpoint] || filteredMap[outpoint]) { - return false; - } - - if (!showMatureContent) { - const claimIsMature = isClaimNsfw(channelClaim); - if (claimIsMature) { - return false; - } - } + if (claimId) { + const claimIdIsMine = myClaims && myClaims.size > 0 && myClaims.has(claimId); + if (!claimIdIsMine) { + if (personalBlockList.includes(comment.channel_url)) { + return false; } + } + } - if (claimId) { - const claimIdIsMine = myClaims && myClaims.size > 0 && myClaims.has(claimId); - if (!claimIdIsMine) { - if (personalBlockList.includes(comment.channel_url) || adminBlockList.includes(comment.channel_url)) { - return false; - } - } - } - - return !mutedChannels.includes(comment.channel_url); - }) - : []; - } - ); + return !mutedChannels.includes(comment.channel_url); + }) + : []; +}; export const makeSelectTotalReplyPagesForParentId = (parentId: string) => createSelector(selectState, (state) => {