Merge pull request Expensify#90065 from Expensify/revert-51366-@janic/open-at-new

Revert "Open reports at first unread action"

Author: arosiclair

jest/setup.ts

@@ -11,7 +11,6 @@ import mockStorage from 'react-native-onyx/dist/storage/__mocks__';
 import type Animated from 'react-native-reanimated';
 import 'setimmediate';
 import {TextDecoder, TextEncoder} from 'util';
-import type {RenderInfo} from '@components/FlatList/RenderTaskQueue';
 import '@src/polyfills/PromiseWithResolvers';
 import '@src/polyfills/requestIdleCallback';
 import mockFSLibrary from './setupMockFullstoryLib';
@@ -266,7 +265,7 @@ jest.mock(
         class SyncRenderTaskQueue {
             private handler: (info: unknown) => void = () => {};
 
-            add(info: RenderInfo) {
+            add(info: unknown) {
                 this.handler(info);
             }
 

src/components/FlashList/InvertedFlashList/index.tsx

@@ -22,34 +22,15 @@ type InvertedFlashListProps<T> = FlashListProps<T> & {
     shouldMaintainVisibleContentPosition?: boolean;
 };
 
-function InvertedFlashList<T>({
-    data,
-    keyExtractor,
-    initialScrollKey,
-    onStartReached: onStartReachedProp,
-    maintainVisibleContentPosition: maintainVisibleContentPositionProp,
-    shouldMaintainVisibleContentPosition,
-    ...restProps
-}: InvertedFlashListProps<T>) {
-    const {
-        displayedData,
-        onStartReached,
-        maintainVisibleContentPosition: maintainVisibleContentPositionForScrollKey,
-    } = useFlashListScrollKey<T>({
+function InvertedFlashList<T>({data, keyExtractor, initialScrollKey, onStartReached: onStartReachedProp, shouldMaintainVisibleContentPosition, ...restProps}: InvertedFlashListProps<T>) {
+    const {displayedData, onStartReached, maintainVisibleContentPosition} = useFlashListScrollKey<T>({
         data,
         keyExtractor,
         initialScrollKey,
         onStartReached: onStartReachedProp,
         shouldMaintainVisibleContentPosition,
     });
 
-    const maintainVisibleContentPosition = maintainVisibleContentPositionProp
-        ? {
-              ...maintainVisibleContentPositionForScrollKey,
-              ...maintainVisibleContentPositionProp,
-          }
-        : maintainVisibleContentPositionForScrollKey;
-
     return (
         <FlashList<T>
             // eslint-disable-next-line react/jsx-props-no-spreading

src/components/FlashList/useFlashListScrollKey.ts

@@ -19,28 +19,17 @@ type FlashListScrollKeyProps<T> = {
 };
 
 export default function useFlashListScrollKey<T>({data, keyExtractor, initialScrollKey, onStartReached, shouldMaintainVisibleContentPosition}: FlashListScrollKeyProps<T>) {
-    const [isInitialRender, setIsInitialRender] = useState(!!initialScrollKey);
+    const [isInitialRender, setIsInitialRender] = useState(true);
     const [hasLinkingSettled, setHasLinkingSettled] = useState(!initialScrollKey);
 
     // Two-frame handoff for deep-link:
     // RAF 1: switch from sliced data to the full array — FlashList's default MVCP pins the
     //        linked item through the data swap.
     // RAF 2: pinning has happened, disable MVCP so it doesn't cause later jumps.
     useEffect(() => {
-        if (!isInitialRender) {
+        if (!isInitialRender || !initialScrollKey) {
             return;
         }
-
-        // Without an anchor on this frame, we are not doing the deep-link slice handoff; clear the flag so a key that
-        // appears later (e.g. marking a message unread) cannot reuse the "first paint" slice path.
-        if (!initialScrollKey) {
-            // If the initial scroll key gets unset, we need to disable the initial render flag,
-            // otherwise the list will not render..
-            // eslint-disable-next-line react-hooks/set-state-in-effect
-            setIsInitialRender(false);
-            return;
-        }
-
         requestAnimationFrame(() => {
             setIsInitialRender(false);
             requestAnimationFrame(() => setHasLinkingSettled(true));

src/components/FlatList/FlatListWithScrollKey/BaseFlatListWithScrollKey.tsx

@@ -19,7 +19,7 @@ function BaseFlatListWithScrollKey<T>({ref, ...props}: BaseFlatListWithScrollKey
         onScrollBeginDrag,
         onWheel,
         onTouchStartCapture,
-        ...restProps
+        ...rest
     } = props;
     const {displayedData, maintainVisibleContentPosition, handleStartReached, isInitialData, handleRenderItem, listRef} = useFlatListScrollKey<T>({
         data,
@@ -52,7 +52,7 @@ function BaseFlatListWithScrollKey<T>({ref, ...props}: BaseFlatListWithScrollKey
         <FlatList
             ref={listRef}
             // eslint-disable-next-line react/jsx-props-no-spreading
-            {...restProps}
+            {...rest}
             data={displayedData}
             maintainVisibleContentPosition={maintainVisibleContentPosition}
             onStartReached={handleStartReached}
@@ -61,8 +61,8 @@ function BaseFlatListWithScrollKey<T>({ref, ...props}: BaseFlatListWithScrollKey
             // Since ListHeaderComponent is always prioritized for rendering before the data,
             // it will be rendered once the data has finished loading.
             // This prevents an unnecessary empty space above the highlighted item.
-            ListHeaderComponent={!isInitialData ? restProps.ListHeaderComponent : undefined}
-            contentContainerStyle={!isInitialData ? restProps.contentContainerStyle : undefined}
+            ListHeaderComponent={!isInitialData ? rest.ListHeaderComponent : undefined}
+            contentContainerStyle={!isInitialData ? rest.contentContainerStyle : undefined}
             onContentSizeChange={(width, height) => onContentSizeChange?.(width, height, isInitialData)}
             onViewableItemsChanged={(info) => {
                 onViewableItemsChanged?.(info);

src/components/FlatList/RenderTaskQueue.tsx

@@ -58,4 +58,3 @@ class RenderTaskQueue {
 }
 
 export default RenderTaskQueue;
-export type {RenderInfo};

src/components/MoneyRequestReportView/MoneyRequestReportActionsList.tsx

@@ -434,7 +434,6 @@ function MoneyRequestReportActionsList({onLayout}: MoneyRequestReportListProps)
         readActionSkippedRef: readActionSkipped,
         unreadMarkerReportActionIndex,
         isInverted: false,
-        hasNewerActions,
         onTrackScrolling: (event: NativeSyntheticEvent<NativeScrollEvent>) => {
             const {layoutMeasurement, contentSize, contentOffset} = event.nativeEvent;
             const fullContentHeight = contentSize.height;
@@ -461,7 +460,6 @@ function MoneyRequestReportActionsList({onLayout}: MoneyRequestReportListProps)
         hasNewestReportAction,
         setIsFloatingMessageCounterVisible,
         scrollToEnd: reportScrollManager.scrollToEnd,
-        resetKey: report.reportID,
     });
 
     /**
@@ -610,15 +608,15 @@ function MoneyRequestReportActionsList({onLayout}: MoneyRequestReportListProps)
         setIsFloatingMessageCounterVisible(false);
 
         if (!hasNewestReportAction) {
-            openReport({reportID, introSelected, betas});
+            openReport({reportID: report?.reportID, introSelected, betas});
             reportScrollManager.scrollToEnd();
             return;
         }
 
         reportScrollManager.scrollToEnd();
         readActionSkipped.current = false;
-        readNewestAction(reportID, !!reportLoadingState?.hasOnceLoadedReportActions);
-    }, [setIsFloatingMessageCounterVisible, hasNewestReportAction, reportScrollManager, reportID, reportLoadingState?.hasOnceLoadedReportActions, introSelected, betas]);
+        readNewestAction(report?.reportID, !!reportLoadingState?.hasOnceLoadedReportActions);
+    }, [setIsFloatingMessageCounterVisible, hasNewestReportAction, reportScrollManager, report?.reportID, reportLoadingState?.hasOnceLoadedReportActions, introSelected, betas]);
 
     const scrollToNewTransaction = useCallback(
         (pageY: number) => {

src/hooks/usePaginatedReportActions.ts

@@ -12,16 +12,13 @@ import useReportIsArchived from './useReportIsArchived';
 type UsePaginatedReportActionsOptions = {
     /** Whether to link to the oldest unread report action, if no other report action id is provided. */
     shouldLinkToOldestUnreadReportAction?: boolean;
-
-    /** When true, pagination anchors to the newest window only (ignores route and unread-derived anchors). */
-    treatAsNoPaginationAnchor?: boolean;
 };
 
 /**
  * Get the longest continuous chunk of reportActions including the linked reportAction. If not linking to a specific action, returns the continuous chunk of newest reportActions.
  */
 function usePaginatedReportActions(reportID: string | undefined, reportActionID?: string, options?: UsePaginatedReportActionsOptions) {
-    const {shouldLinkToOldestUnreadReportAction = false, treatAsNoPaginationAnchor = false} = options ?? {};
+    const {shouldLinkToOldestUnreadReportAction = false} = options ?? {};
 
     const nonEmptyStringReportID = getNonEmptyStringOnyxID(reportID);
     const [report] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${nonEmptyStringReportID}`);
@@ -47,11 +44,6 @@ function usePaginatedReportActions(reportID: string | undefined, reportActionID?
     const initialReportLastReadTime = useRef(report?.lastReadTime);
 
     const id = useMemo(() => {
-        /* eslint-disable react-hooks/refs -- initialReportLastReadTime snapshots lastRead at first render for stable unread deep-link anchor */
-        if (treatAsNoPaginationAnchor) {
-            return undefined;
-        }
-
         if (reportActionID) {
             return reportActionID;
         }
@@ -60,14 +52,14 @@ function usePaginatedReportActions(reportID: string | undefined, reportActionID?
             return undefined;
         }
 
-        const initialLastReadTime = initialReportLastReadTime.current;
-        if (!initialLastReadTime || !sortedAllReportActions?.length) {
-            return undefined;
-        }
+        return sortedAllReportActions?.findLast((reportAction) => {
+            if (!initialReportLastReadTime.current) {
+                return false;
+            }
 
-        return sortedAllReportActions.findLast((reportAction) => reportAction.created > initialLastReadTime)?.reportActionID;
-        /* eslint-enable react-hooks/refs */
-    }, [treatAsNoPaginationAnchor, reportActionID, shouldLinkToOldestUnreadReportAction, sortedAllReportActions]);
+            return reportAction.created > initialReportLastReadTime.current;
+        })?.reportActionID;
+    }, [reportActionID, shouldLinkToOldestUnreadReportAction, sortedAllReportActions]);
 
     const {
         data: reportActions,
@@ -82,27 +74,14 @@ function usePaginatedReportActions(reportID: string | undefined, reportActionID?
         return PaginationUtils.getContinuousChain(sortedAllReportActions, reportActionPages ?? [], (reportAction) => reportAction.reportActionID, id);
     }, [id, reportActionPages, sortedAllReportActions]);
 
-    // When `treatAsNoPaginationAnchor` is set, we intentionally ignore `reportActionID` for pagination
-    // (same as `id` above), so we must not surface a "linked" action from that id either.
-    const linkedAction = useMemo(() => {
-        if (treatAsNoPaginationAnchor) {
-            return undefined;
-        }
-        if (!reportActionID) {
-            return undefined;
-        }
-        return resourceItem?.item;
-    }, [resourceItem?.item, reportActionID, treatAsNoPaginationAnchor]);
+    const linkedAction = useMemo(() => (reportActionID ? resourceItem?.item : undefined), [resourceItem?.item, reportActionID]);
 
     const oldestUnreadReportAction = useMemo(() => {
-        if (treatAsNoPaginationAnchor) {
-            return undefined;
-        }
         if (shouldLinkToOldestUnreadReportAction && resourceItem && !reportActionID) {
             return resourceItem.item;
         }
         return undefined;
-    }, [resourceItem, shouldLinkToOldestUnreadReportAction, reportActionID, treatAsNoPaginationAnchor]);
+    }, [resourceItem, shouldLinkToOldestUnreadReportAction, reportActionID]);
 
     return {
         reportActions,

src/hooks/useScrollToEndOnNewMessageReceived.ts

@@ -1,4 +1,4 @@
-import {useEffect, useLayoutEffect, useRef} from 'react';
+import {useEffect, useRef} from 'react';
 import type React from 'react';
 import {AUTOSCROLL_TO_TOP_THRESHOLD} from '@components/FlatList/hooks/useFlatListScrollKey';
 import usePrevious from './usePrevious';
@@ -43,22 +43,8 @@ function useScrollToEndOnNewMessageReceived({
 }: UseScrollToEndOnPaginationMergeParams) {
     const previousLastIndex = useRef(lastActionID);
     const reportActionSize = useRef(visibleActionsLength);
-    const previousResetKeyRef = useRef<unknown>(undefined);
     const prevHasNewestReportAction = usePrevious(hasNewestReportAction);
 
-    // When the hook is used across report navigations, baselines from the previous report must not drive scroll logic.
-    useLayoutEffect(() => {
-        if (resetKey === undefined) {
-            return;
-        }
-        if (previousResetKeyRef.current === resetKey) {
-            return;
-        }
-        previousResetKeyRef.current = resetKey;
-        previousLastIndex.current = lastActionID;
-        reportActionSize.current = visibleActionsLength;
-    }, [resetKey, lastActionID, visibleActionsLength]);
-
     useEffect(() => {
         const didListSizeChange = sizeChangeType === 'grewFromReportActions' ? reportActionSize.current > (reportActionsLength ?? 0) : reportActionSize.current !== visibleActionsLength;
 

src/libs/API/index.ts

@@ -291,7 +291,9 @@ function paginate<TRequestType extends ApiRequestType, TCommand extends CommandO
     const request: PaginatedRequest<TKey> = {
         ...prepareRequest(command, type, apiCommandParameters, onyxData, conflictResolver),
         ...config,
-        isPaginated: true,
+        ...{
+            isPaginated: true,
+        },
     };
 
     switch (type) {

src/libs/Middleware/Pagination.ts

@@ -3,12 +3,11 @@ import type {OnyxCollection, OnyxKey} from 'react-native-onyx';
 import Onyx from 'react-native-onyx';
 import type {ApiCommand} from '@libs/API/types';
 import Log from '@libs/Log';
-import {mergeAndSortContinuousPages, mergePagesByIDOverlap} from '@libs/PaginationUtils';
+import PaginationUtils from '@libs/PaginationUtils';
 import CONST from '@src/CONST';
 import type {OnyxCollectionKey, OnyxPagesKey, OnyxValues} from '@src/ONYXKEYS';
 import ONYXKEYS from '@src/ONYXKEYS';
 import type {Request} from '@src/types/onyx';
-import type Pages from '@src/types/onyx/Pages';
 import type {AnyOnyxUpdate, PaginatedRequest} from '@src/types/onyx/Request';
 import type Middleware from './types';
 
@@ -125,18 +124,14 @@ const Pagination: Middleware = (requestResponse, request) => {
         const sortedAllItems = sortItems(allItems, resourceID);
 
         const pagesCollections = pages.get(pageCollectionKey) ?? {};
-        const existingPages: Pages = pagesCollections[pageKey] ?? [];
-
-        const isMiddleInitialSlice = type === 'initial' && !cursorID && response.hasNewerActions === true && response.hasOlderActions === true;
+        const existingPages = pagesCollections[pageKey] ?? [];
 
         // Only strip PAGINATION_START_ID from cached pages when the server explicitly confirms newer actions exist.
         // Some commands (e.g. GetOlderActions) don't return hasNewerActions at all — in that case, preserve the existing boundary.
         const shouldStripStartMarker = response.hasNewerActions === true;
         const sanitizedExistingPages = shouldStripStartMarker ? existingPages.map((page) => page.filter((id) => id !== CONST.PAGINATION_START_ID)) : existingPages;
 
-        const mergedPages: Pages = isMiddleInitialSlice
-            ? mergePagesByIDOverlap(sortedAllItems, [...sanitizedExistingPages, newPage], getItemID)
-            : mergeAndSortContinuousPages(sortedAllItems, [...sanitizedExistingPages, newPage], getItemID);
+        const mergedPages = PaginationUtils.mergeAndSortContinuousPages(sortedAllItems, [...sanitizedExistingPages, newPage], getItemID);
 
         (response.onyxData as AnyOnyxUpdate[]).push({
             key: pageKey,