Skip to content

Commit 6abd09c

Browse files
authored
fix(feedback): Fix a case where duplicate items can be rendered in the list (#95623)
The robots made a comment: #95398 (review) It points out that we're not de-duplicating result items inside an infinite list when we go to render things. How could we get duplicate items? If the pagination params are based on offsets (which ours mostly are) then: - You fetch the first 10 results (items with `id:1`, `id:2`, ... `id:10`) in the first page - In the 2nd page we'll ask for 10 items, starting at the item 10th from the start - So we'd expect to get `id:11`, `id:12` and so on But "start" depends on the sort order, and usually we're showing the most recent stuff first. - When sorting newest->oldest our first page of data would actually be something like `id: 89`, `id:88`, `id:87`, etc. - The second page we'd expect to be `id:79`, id:78`, etc. - But if a new item appears, and becomes `id:90` then everything else shifts down one. - So our 2nd page of data actually returns `id: 80`, `id:79`, etc. The end result is that `id:80` is double-fetched. This is what we need to filter out of the data.
1 parent 3206e5b commit 6abd09c

File tree

2 files changed

+9
-3
lines changed

2 files changed

+9
-3
lines changed

static/app/components/feedback/list/feedbackList.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,9 @@ export default function FeedbackList() {
3535
enabled: Boolean(listQueryKey),
3636
});
3737

38+
// Deduplicated issues. In case one page overlaps with another.
3839
const issues = useMemo(
39-
() => uniqBy(queryResult.data?.pages.flatMap(([pageData]) => pageData) ?? [], 'id'),
40+
() => uniqBy(queryResult.data?.pages.flatMap(result => result[0]) ?? [], 'id'),
4041
[queryResult.data?.pages]
4142
);
4243
const checkboxState = useListItemCheckboxContext({
@@ -57,6 +58,7 @@ export default function FeedbackList() {
5758
loadingMessage={() => <LoadingIndicator />}
5859
>
5960
<InfiniteListItems<FeedbackIssueListItem>
61+
deduplicateItems={items => uniqBy(items, 'id')}
6062
estimateSize={() => 24}
6163
queryResult={queryResult}
6264
itemRenderer={({item}) => (

static/app/components/infiniteList/infiniteListItems.tsx

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ interface Props<Data> {
2424
>,
2525
{fetchNextPage: () => Promise<unknown>}
2626
>;
27+
deduplicateItems?: (items: Data[]) => Data[];
2728
emptyMessage?: () => React.ReactNode;
2829
estimateSize?: () => number;
2930
loadingCompleteMessage?: () => React.ReactNode;
@@ -32,16 +33,19 @@ interface Props<Data> {
3233
}
3334

3435
export default function InfiniteListItems<Data>({
36+
deduplicateItems = _ => _,
37+
emptyMessage = EmptyMessage,
3538
estimateSize,
3639
itemRenderer,
37-
emptyMessage = EmptyMessage,
3840
loadingCompleteMessage = LoadingCompleteMessage,
3941
loadingMoreMessage = LoadingMoreMessage,
4042
overscan,
4143
queryResult,
4244
}: Props<Data>) {
4345
const {data, hasNextPage, isFetchingNextPage, fetchNextPage} = queryResult;
44-
const loadedRows = data ? data.pages.flatMap(d => d[0]) : [];
46+
const loadedRows = deduplicateItems(
47+
data ? data.pages.flatMap(result => result[0]) : []
48+
);
4549
const parentRef = useRef<HTMLDivElement>(null);
4650

4751
const rowVirtualizer = useVirtualizer({

0 commit comments

Comments
 (0)