diff --git a/cspell.json b/cspell.json index 3fa504ae25a9..01a04820149f 100644 --- a/cspell.json +++ b/cspell.json @@ -776,6 +776,7 @@ "Uncapitalize", "uncategorized", "Undelete", + "unflushed", "unheld", "unhold", "uninstallations", diff --git a/src/CONST/index.ts b/src/CONST/index.ts index 998a439a6fdc..00779fe4062d 100644 --- a/src/CONST/index.ts +++ b/src/CONST/index.ts @@ -1839,6 +1839,9 @@ const CONST = { ACTIVITY_INDICATOR_TIMEOUT: 10000, MIN_SMOOTH_SCROLL_EVENT_THROTTLE: 16, }, + DEFERRED_LAYOUT_WRITE_KEYS: { + SEARCH: 'search', + }, TELEMETRY: { CONTEXT_FULLSTORY: 'Fullstory', CONTEXT_MEMORY: 'Memory', @@ -1951,6 +1954,7 @@ const CONST = { ATTRIBUTE_MIN_DURATION: 'min_duration', ATTRIBUTE_FINISHED_MANUALLY: 'finished_manually', ATTRIBUTE_IS_WARM: 'is_warm', + ATTRIBUTE_WAS_LIST_EMPTY: 'was_list_empty', ATTRIBUTE_SKELETON_PREFIX: 'skeleton.', ATTRIBUTE_SCENARIO: 'scenario', ATTRIBUTE_HAS_RECEIPT: 'has_receipt', diff --git a/src/components/Search/DeferredSearch/index.native.tsx b/src/components/Search/DeferredSearch/index.native.tsx deleted file mode 100644 index c3297804a819..000000000000 --- a/src/components/Search/DeferredSearch/index.native.tsx +++ /dev/null @@ -1,52 +0,0 @@ -import React, {useEffect, useMemo, useState} from 'react'; -import SearchRowSkeleton from '@components/Skeletons/SearchRowSkeleton'; -import Search from '..'; -import type {SearchProps} from '..'; - -/** - * Wrapper that defers mounting Search by one frame when the query hash changes. - * Search is expensive to mount (many hooks, Onyx subscriptions, memos). When key={hash} - * changes, React remounts it and can show a blank frame. This shows a lightweight - * skeleton for one frame first so the transition is skeleton -> Search (no blank gap). - */ -function DeferredSearch(props: SearchProps) { - const {queryJSON, contentContainerStyle} = props; - const queryHash = queryJSON?.hash; - - const [preparedHash, setPreparedHash] = useState(undefined); - - useEffect(() => { - if (queryHash === undefined || preparedHash === queryHash) { - return; - } - const id = requestAnimationFrame(() => { - setPreparedHash(queryHash); - }); - return () => cancelAnimationFrame(id); - }, [queryHash, preparedHash]); - - const isSearchReady = preparedHash === queryHash; - const skeletonReasonAttributes = useMemo(() => ({context: 'DeferredSearch'}), []); - - if (!isSearchReady) { - return ( - - ); - } - - return ( - - ); -} - -DeferredSearch.displayName = 'DeferredSearch'; - -export default DeferredSearch; diff --git a/src/components/Search/DeferredSearch/index.tsx b/src/components/Search/DeferredSearch/index.tsx deleted file mode 100644 index 02d598d4b5de..000000000000 --- a/src/components/Search/DeferredSearch/index.tsx +++ /dev/null @@ -1,21 +0,0 @@ -/** - * Web renders Search directly; no deferral needed. See index.native.tsx for the native wrapper. - * We still wrap it to apply key={hash} which was moved here from the call site. - */ -import React from 'react'; -import Search from '..'; -import type {SearchProps} from '..'; - -function DeferredSearch(props: SearchProps) { - return ( - - ); -} - -DeferredSearch.displayName = 'DeferredSearch'; - -export default DeferredSearch; diff --git a/src/components/Search/SearchContext.tsx b/src/components/Search/SearchContext.tsx index 293ec58d5aaf..e4da17cea809 100644 --- a/src/components/Search/SearchContext.tsx +++ b/src/components/Search/SearchContext.tsx @@ -1,5 +1,5 @@ import {useNavigationState} from '@react-navigation/native'; -import React, {useCallback, useContext, useMemo, useRef, useState} from 'react'; +import React, {useCallback, useContext, useEffect, useMemo, useRef, useState} from 'react'; // We need direct access to useOnyx from react-native-onyx to avoid circular dependencies in SearchContext // eslint-disable-next-line no-restricted-imports import {useOnyx} from 'react-native-onyx'; @@ -8,8 +8,6 @@ import useCurrentUserPersonalDetails from '@hooks/useCurrentUserPersonalDetails' import usePreviousDefined from '@hooks/usePreviousDefined'; import useTodos from '@hooks/useTodos'; import {getDeepestFocusedScreen} from '@libs/Navigation/Navigation'; -import type {PlatformStackScreenProps} from '@libs/Navigation/PlatformStackNavigation/types'; -import type {SearchFullscreenNavigatorParamList} from '@libs/Navigation/types'; import {isMoneyRequestReport} from '@libs/ReportUtils'; import {buildSearchQueryJSON, buildSearchQueryString} from '@libs/SearchQueryUtils'; import type {SearchKey, SearchTypeMenuItem} from '@libs/SearchUIUtils'; @@ -81,22 +79,21 @@ const defaultSearchActionsContext: SearchActionsContextValue = { const SearchStateContext = React.createContext(defaultSearchStateContext); const SearchActionsContext = React.createContext(defaultSearchActionsContext); -function SearchContextProvider({children}: SearchContextProps) { - const focusedScreen = useNavigationState((state) => getDeepestFocusedScreen(state)); - const focusedScreenName = focusedScreen?.name; - const focusedScreenParams = focusedScreen?.params; - - // Get the params for the search page so that we can derive the search query JSON from it - const params = useMemo(() => { - if (focusedScreenName !== SCREENS.SEARCH.ROOT) { - return undefined; - } +function selectSearchQueryParam(state: Parameters[0]>[0]) { + const focused = getDeepestFocusedScreen(state); + return focused?.name === SCREENS.SEARCH.ROOT ? (focused.params?.q as string | undefined) : undefined; +} - return focusedScreenParams as PlatformStackScreenProps['route']['params']; - }, [focusedScreenName, focusedScreenParams]); +function selectSearchRawQueryParam(state: Parameters[0]>[0]) { + const focused = getDeepestFocusedScreen(state); + return focused?.name === SCREENS.SEARCH.ROOT ? (focused.params?.rawQuery as string | undefined) : undefined; +} - const queryParam = params?.q; - const rawQueryParam = params?.rawQuery; +function SearchContextProvider({children}: SearchContextProps) { + // Extract only the primitive values we need from the focused screen to avoid + // re-renders from new object references returned by getDeepestFocusedScreen. + const queryParam = useNavigationState(selectSearchQueryParam); + const rawQueryParam = useNavigationState(selectSearchRawQueryParam); const definedQueryParam = usePreviousDefined(queryParam) ?? buildSearchQueryString(); const currentSearchQueryJSON = useMemo(() => buildSearchQueryJSON(definedQueryParam, rawQueryParam), [definedQueryParam, rawQueryParam]); @@ -107,9 +104,6 @@ function SearchContextProvider({children}: SearchContextProps) { const [shouldShowSelectAllMatchingItems, setShouldShowSelectAllMatchingItems] = useState(false); const [searchContextData, setSearchContextData] = useState({...defaultSearchContextData}); - const selectedReports = searchContextData.selectedReports; - const selectedTransactions = searchContextData.selectedTransactions; - const selectedTransactionIDs = searchContextData.selectedTransactionIDs; const currentSearchHash = currentSearchQueryJSON?.hash ?? -1; const currentRecentSearchHash = currentSearchQueryJSON?.recentSearchHash ?? -1; const currentSimilarSearchHash = currentSearchQueryJSON?.similarSearchHash ?? -1; @@ -119,7 +113,8 @@ function SearchContextProvider({children}: SearchContextProps) { const {defaultCardFeed} = useCardFeedsForDisplay(); const {accountID} = useCurrentUserPersonalDetails(); - const suggestedSearches = getSuggestedSearches(accountID, defaultCardFeed?.id); + const defaultCardFeedID = defaultCardFeed?.id; + const suggestedSearches = useMemo(() => getSuggestedSearches(accountID, defaultCardFeedID), [accountID, defaultCardFeedID]); const currentSearchKey = useMemo(() => { return Object.values(suggestedSearches).find((search) => search.similarSearchHash === currentSimilarSearchHash)?.key; @@ -150,7 +145,7 @@ function SearchContextProvider({children}: SearchContextProps) { return snapshotSearchResults ?? undefined; }, [currentSearchKey, shouldUseLiveData, snapshotSearchResults, todoSearchResultsData]); - const setSelectedTransactions: SearchActionsContextValue['setSelectedTransactions'] = (transactionIDs, data = []) => { + const setSelectedTransactions: SearchActionsContextValue['setSelectedTransactions'] = useCallback((transactionIDs, data = []) => { if (transactionIDs instanceof Array) { if (!transactionIDs.length && areTransactionsEmpty.current) { areTransactionsEmpty.current = true; @@ -211,7 +206,12 @@ function SearchContextProvider({children}: SearchContextProps) { selectedTransactions: transactionIDs, shouldTurnOffSelectionMode: false, })); - }; + }, []); + + const currentSearchHashRef = useRef(currentSearchHash); + useEffect(() => { + currentSearchHashRef.current = currentSearchHash; + }, [currentSearchHash]); const setCurrentSelectedTransactionReportID: SearchActionsContextValue['setCurrentSelectedTransactionReportID'] = (reportID) => { setSearchContextData((prevState) => { @@ -233,88 +233,116 @@ function SearchContextProvider({children}: SearchContextProps) { return; } - if (searchHashOrClearIDsFlag === currentSearchHash) { + if (searchHashOrClearIDsFlag === currentSearchHashRef.current) { return; } - if (selectedReports.length === 0 && isEmptyObject(selectedTransactions) && !searchContextData.shouldTurnOffSelectionMode) { - return; - } - setSearchContextData((prevState) => ({ - ...prevState, - shouldTurnOffSelectionMode, - selectedTransactions: {}, - selectedReports: [], - })); + setSearchContextData((prevState) => { + if (prevState.selectedReports.length === 0 && isEmptyObject(prevState.selectedTransactions) && !prevState.shouldTurnOffSelectionMode) { + return prevState; + } + return { + ...prevState, + shouldTurnOffSelectionMode, + selectedTransactions: {}, + selectedReports: [], + }; + }); - // Unselect all transactions and hide the "select all matching items" option setShouldShowSelectAllMatchingItems(false); selectAllMatchingItems(false); }, - [currentSearchHash, searchContextData.shouldTurnOffSelectionMode, selectedReports.length, selectedTransactions], + // currentSearchHash is read via currentSearchHashRef to keep this callback stable. + // setShouldShowSelectAllMatchingItems and selectAllMatchingItems are stable useState setters. + [setSelectedTransactions], ); - const removeTransaction: SearchActionsContextValue['removeTransaction'] = (transactionID) => { + const removeTransaction: SearchActionsContextValue['removeTransaction'] = useCallback((transactionID) => { if (!transactionID) { return; } - if (!isEmptyObject(selectedTransactions)) { - const newSelectedTransactions = Object.entries(selectedTransactions).reduce((acc, [key, value]) => { - if (key === transactionID) { - return acc; - } - acc[key] = value; - return acc; - }, {} as SelectedTransactions); + setSearchContextData((prevState) => { + const hasSelectedTransactions = !isEmptyObject(prevState.selectedTransactions); + const hasSelectedIDs = prevState.selectedTransactionIDs.length > 0; - setSearchContextData((prevState) => ({ - ...prevState, - selectedTransactions: newSelectedTransactions, - })); - } + if (!hasSelectedTransactions && !hasSelectedIDs) { + return prevState; + } - if (selectedTransactionIDs.length > 0) { - setSearchContextData((prevState) => ({ - ...prevState, - selectedTransactionIDs: selectedTransactionIDs.filter((ID) => transactionID !== ID), - })); - } - }; + const newState = {...prevState}; + if (hasSelectedTransactions) { + const newSelectedTransactions = Object.entries(prevState.selectedTransactions).reduce((acc, [key, value]) => { + if (key === transactionID) { + return acc; + } + acc[key] = value; + return acc; + }, {} as SelectedTransactions); + newState.selectedTransactions = newSelectedTransactions; + } + if (hasSelectedIDs) { + newState.selectedTransactionIDs = prevState.selectedTransactionIDs.filter((ID) => transactionID !== ID); + } + return newState; + }); + }, []); - const setShouldResetSearchQuery = (shouldReset: boolean) => { + const setShouldResetSearchQuery = useCallback((shouldReset: boolean) => { setSearchContextData((prevState) => ({ ...prevState, shouldResetSearchQuery: shouldReset, })); - }; - - const searchStateContextValue: SearchStateContextValue = { - ...searchContextData, - suggestedSearches, - currentSearchKey, - currentSearchHash, - currentSimilarSearchHash, - currentSearchResults, - shouldUseLiveData, - shouldShowActionsBarLoading, - lastSearchType, - shouldShowSelectAllMatchingItems, - areAllMatchingItemsSelected, - currentSearchQueryJSON, - }; + }, []); + + const searchStateContextValue: SearchStateContextValue = useMemo( + () => ({ + ...searchContextData, + suggestedSearches, + currentSearchKey, + currentSearchHash, + currentSimilarSearchHash, + currentSearchResults, + shouldUseLiveData, + shouldShowActionsBarLoading, + lastSearchType, + shouldShowSelectAllMatchingItems, + areAllMatchingItemsSelected, + currentSearchQueryJSON, + }), + [ + searchContextData, + suggestedSearches, + currentSearchKey, + currentSearchHash, + currentSimilarSearchHash, + currentSearchResults, + shouldUseLiveData, + shouldShowActionsBarLoading, + lastSearchType, + shouldShowSelectAllMatchingItems, + areAllMatchingItemsSelected, + currentSearchQueryJSON, + ], + ); - const searchActionsContextValue: SearchActionsContextValue = { - removeTransaction, - setSelectedTransactions, - setCurrentSelectedTransactionReportID, - clearSelectedTransactions, - setShouldShowActionsBarLoading, - setLastSearchType, - setShouldShowSelectAllMatchingItems, - selectAllMatchingItems, - setShouldResetSearchQuery, - }; + const searchActionsContextValue: SearchActionsContextValue = useMemo( + () => ({ + removeTransaction, + setSelectedTransactions, + setCurrentSelectedTransactionReportID, + clearSelectedTransactions, + setShouldShowActionsBarLoading, + setLastSearchType, + setShouldShowSelectAllMatchingItems, + selectAllMatchingItems, + setShouldResetSearchQuery, + }), + // setShouldShowActionsBarLoading, setLastSearchType, setShouldShowSelectAllMatchingItems, + // and selectAllMatchingItems are stable useState setters — excluded from deps intentionally. + // setCurrentSelectedTransactionReportID only uses setSearchContextData (stable setter). + [removeTransaction, setSelectedTransactions, clearSelectedTransactions, setShouldResetSearchQuery], + ); return ( diff --git a/src/components/Search/index.tsx b/src/components/Search/index.tsx index bb4bc45cde16..d4abada11ecb 100644 --- a/src/components/Search/index.tsx +++ b/src/components/Search/index.tsx @@ -25,6 +25,7 @@ import useThemeStyles from '@hooks/useThemeStyles'; import {turnOffMobileSelectionMode, turnOnMobileSelectionMode} from '@libs/actions/MobileSelectionMode'; import type {TransactionPreviewData} from '@libs/actions/Search'; import {setOptimisticDataForTransactionThreadPreview} from '@libs/actions/Search'; +import {flushDeferredWrite, getOptimisticWatchKey, hasDeferredWrite} from '@libs/deferredLayoutWrite'; import {canUseTouchScreen} from '@libs/DeviceCapabilities'; import Log from '@libs/Log'; import isSearchTopmostFullScreenRoute from '@libs/Navigation/helpers/isSearchTopmostFullScreenRoute'; @@ -91,6 +92,11 @@ type SearchProps = { searchRequestResponseStatusCode?: number | null; }; +// Max time (ms) to keep the optimistic item cache/skeleton alive before +// clearing all tracking state. Must be longer than deferredLayoutWrite's +// 5s safety timeout so the API.write() has time to apply optimistic data. +const OPTIMISTIC_TRACKING_TIMEOUT_MS = 10_000; + const hashToString = (queryHash?: number) => (queryHash || queryHash === 0 ? String(queryHash) : undefined); function mapTransactionItemToSelectedEntry( @@ -205,7 +211,52 @@ function Search({ searchRequestResponseStatusCode, }: SearchProps) { const {type, status, sortBy, sortOrder, hash, similarSearchHash, groupBy, view} = queryJSON; - const [shouldDeferHeavySearchWork, setShouldDeferHeavySearchWork] = useState(() => !isSearchDataLoaded(searchResults, queryJSON)); + // Deferred write: API.write() is postponed so the skeleton renders instantly. + // Once flushed, we cache the optimistic item from sortedData and re-inject it + // via stableSortedData if a stale snapshot briefly removes it. The cache is + // cleared when the server-confirmed version arrives (pendingAction !== ADD). + const hasPendingWriteOnMountRef = useRef(hasDeferredWrite(CONST.DEFERRED_LAYOUT_WRITE_KEYS.SEARCH)); + const optimisticWatchKeyRef = useRef(getOptimisticWatchKey(CONST.DEFERRED_LAYOUT_WRITE_KEYS.SEARCH)); + const skipDeferralOnFocusRef = useRef(isSearchDataLoaded(searchResults, queryJSON) && !hasPendingWriteOnMountRef.current); + + const [shouldDeferHeavySearchWork, setShouldDeferHeavySearchWork] = useState(() => !isSearchDataLoaded(searchResults, queryJSON) || hasPendingWriteOnMountRef.current); + const [showPendingExpensePlaceholder, setShowPendingExpensePlaceholder] = useState(() => hasPendingWriteOnMountRef.current && optimisticWatchKeyRef.current != null); + // Caches the optimistic list item once it first appears in sortedData. + // Used by stableSortedData to re-inject the row if a stale snapshot temporarily removes it. + // Cleared once the server-confirmed (non-optimistic) version arrives. + const cachedOptimisticItemRef = useRef(null); + const optimisticTrackingCleanedUpRef = useRef(false); + + const clearOptimisticTracking = useCallback(() => { + if (optimisticTrackingCleanedUpRef.current) { + return; + } + optimisticTrackingCleanedUpRef.current = true; + cachedOptimisticItemRef.current = null; + optimisticWatchKeyRef.current = undefined; + setShowPendingExpensePlaceholder(false); + }, []); + + // Safety timeout: if the optimistic lifecycle hasn't resolved within 10s + // (e.g. API failure, offline, item never reaches sortedData), clear all + // tracking state so the UI doesn't get stuck on a skeleton or ghost row. + useEffect(() => { + if (!hasPendingWriteOnMountRef.current || !optimisticWatchKeyRef.current) { + return; + } + const id = setTimeout(clearOptimisticTracking, OPTIMISTIC_TRACKING_TIMEOUT_MS); + return () => clearTimeout(id); + }, [clearOptimisticTracking]); + + // Flush (not cancel) on unmount so the API.write() still executes if the + // user navigates away before onLayout fires. This also clears the channel, + // preventing a stale hasDeferredWrite() on the next mount. + useEffect( + () => () => { + flushDeferredWrite(CONST.DEFERRED_LAYOUT_WRITE_KEYS.SEARCH); + }, + [], + ); const {isOffline} = useNetwork(); const prevIsOffline = usePrevious(isOffline); @@ -279,6 +330,8 @@ function Search({ const validGroupBy = getValidGroupBy(groupBy); const prevValidGroupBy = usePrevious(validGroupBy); const isSearchResultsEmpty = !searchResults?.data || isSearchResultsEmptyUtil(searchResults, validGroupBy); + const isSearchResultsEmptyRef = useRef(isSearchResultsEmpty); + isSearchResultsEmptyRef.current = isSearchResultsEmpty; // When grouping by card, we need cardFeeds to display feed names const isCardFeedsLoading = validGroupBy === CONST.SEARCH.GROUP_BY.CARD && cardFeedsResult?.status === 'loading'; @@ -371,13 +424,13 @@ function Search({ // Skipping the defer for live data (to-dos) and cached results avoids a // flash of the empty state or a blank page caused by a redundant defer cycle. useEffect(() => { - if (shouldUseLiveData || isDataLoaded) { + if (isDataLoaded) { setShouldDeferHeavySearchWork(false); return; } return deferHeavySearchWork(); // eslint-disable-next-line react-hooks/exhaustive-deps - }, [hash, deferHeavySearchWork, shouldUseLiveData, isDataLoaded]); + }, [hash, deferHeavySearchWork, isDataLoaded]); useFocusEffect( useCallback(() => { @@ -389,22 +442,30 @@ function Search({ return; } + if (skipDeferralOnFocusRef.current) { + skipDeferralOnFocusRef.current = false; + return; + } + // Re-applying the defer only on the submit-return path keeps the optimization scoped to // the transition we care about instead of slowing every search refocus. return deferHeavySearchWork(true); }, [deferHeavySearchWork]), ); + const [skeletonWasDisplayed, setSkeletonWasDisplayed] = useState(false); + const onSkeletonLayout = useCallback(() => setSkeletonWasDisplayed(true), []); + const deferredWorkReasonAttributes = useMemo(() => ({context: 'Search.DeferredWork'}) as const, []); + const pendingExpenseReasonAttributes = useMemo(() => ({context: 'Search.PendingExpensePlaceholder'}) as const, []); + // Show a skeleton whenever heavy work is deferred, even for live-data (to-do) searches, // so we never fall through to the empty-state check with stale zero-length data. - const shouldShowLoadingState = - (!isOffline && shouldDeferHeavySearchWork) || - (!shouldUseLiveData && - !isOffline && - (!isDataLoaded || - (!!searchResults?.search.isLoading && Array.isArray(searchResults?.data) && searchResults?.data.length === 0) || - (hasErrors && searchRequestResponseStatusCode === null) || - isCardFeedsLoading)); + const isDeferringHeavyWork = !isOffline && shouldDeferHeavySearchWork; + const isSearchLoadingWithNoResults = !!searchResults?.search.isLoading && Array.isArray(searchResults?.data) && searchResults?.data.length === 0; + const hasUnresolvedErrors = hasErrors && searchRequestResponseStatusCode === null; + const isWaitingForInitialData = !shouldUseLiveData && !isOffline && (!isDataLoaded || isSearchLoadingWithNoResults || hasUnresolvedErrors || isCardFeedsLoading); + const shouldShowLoadingState = isDeferringHeavyWork || isWaitingForInitialData; + const shouldShowRowSkeleton = (!skeletonWasDisplayed || shouldShowLoadingState) && hasPendingWriteOnMountRef.current && !hasErrors; const shouldShowLoadingMoreItems = !shouldShowLoadingState && searchResults?.search?.isLoading && searchResults?.search?.offset > 0; @@ -1158,6 +1219,45 @@ function Search({ [type, status, filteredData, localeCompare, translate, sortBy, sortOrder, validGroupBy, isChat, newSearchResultKeys, hash], ); + // Track the optimistic item through its lifecycle in sortedData. + // First appearance -> cache it & hide the skeleton. + // Server confirmed (pendingAction !== ADD) -> clear all tracking. + useEffect(() => { + if (!optimisticWatchKeyRef.current) { + return; + } + const optimisticItem = sortedData.find( + (item): item is TransactionListItemType => 'transactionID' in item && `${ONYXKEYS.COLLECTION.TRANSACTION}${item.transactionID}` === optimisticWatchKeyRef.current, + ); + if (optimisticItem) { + if (!cachedOptimisticItemRef.current) { + setShowPendingExpensePlaceholder(false); + } + cachedOptimisticItemRef.current = optimisticItem; + + if (optimisticItem.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD) { + clearOptimisticTracking(); + } + } + }, [sortedData, clearOptimisticTracking]); + + // Re-inject the cached optimistic item when a stale snapshot temporarily removes it + // from sortedData. Once the item is back (real data), this is a no-op. + // Refs are intentionally excluded from deps. The memo doesn't need the cached + // value during the render where the item IS in sortedData (it passes through). + // When sortedData later changes (item removed by snapshot), the preceding tracking + // effect has already populated the ref, so the memo picks up the cached value. + const stableSortedData = useMemo(() => { + if (!cachedOptimisticItemRef.current || !optimisticWatchKeyRef.current) { + return sortedData; + } + const isStillInList = sortedData.some((item) => 'transactionID' in item && `${ONYXKEYS.COLLECTION.TRANSACTION}${item.transactionID}` === optimisticWatchKeyRef.current); + if (isStillInList) { + return sortedData; + } + return [...sortedData, cachedOptimisticItemRef.current]; + }, [sortedData]); + useEffect(() => { const currentRoute = Navigation.getActiveRouteWithoutParams(); if (hasErrors && (currentRoute === '/' || (shouldResetSearchQuery && currentRoute === '/search'))) { @@ -1247,12 +1347,16 @@ function Search({ markNavigateAfterExpenseCreateEnd(); const pending = getPendingSubmitFollowUpAction(); if (pending && pending.followUpAction !== CONST.TELEMETRY.SUBMIT_FOLLOW_UP_ACTION.DISMISS_MODAL_AND_OPEN_REPORT) { - endSubmitFollowUpActionSpan(pending.followUpAction); + endSubmitFollowUpActionSpan(pending.followUpAction, undefined, { + [CONST.TELEMETRY.ATTRIBUTE_IS_WARM]: true, + [CONST.TELEMETRY.ATTRIBUTE_WAS_LIST_EMPTY]: isSearchResultsEmptyRef.current, + }); } // Reset the ref after the span is ended so future render-time cancelSpan calls are no longer guarded. spanExistedOnMount.current = false; - handleSelectionListScroll(sortedData, searchListRef.current); - }, [handleSelectionListScroll, sortedData]); + handleSelectionListScroll(stableSortedData, searchListRef.current); + flushDeferredWrite(CONST.DEFERRED_LAYOUT_WRITE_KEYS.SEARCH); + }, [handleSelectionListScroll, stableSortedData]); useEffect(() => { const spanExisted = spanExistedOnMount.current; @@ -1265,6 +1369,10 @@ function Search({ // eslint-disable-next-line react-hooks/exhaustive-deps }, []); + // Must be a ref, not state: cancelNavigationSpans is called during render + // (inside conditional returns), so using setState would trigger infinite re-renders. + const didBailToFallbackState = useRef(false); + const cancelNavigationSpans = useCallback(() => { cancelSpan(CONST.TELEMETRY.SPAN_NAVIGATE_TO_REPORTS); cancelSpan(CONST.TELEMETRY.SPAN_NAVIGATE_AFTER_EXPENSE_CREATE); @@ -1273,8 +1381,22 @@ function Search({ cancelSubmitFollowUpActionSpan(); } spanExistedOnMount.current = false; + didBailToFallbackState.current = true; }, []); + // When the render bails to an error/empty state, the SelectionList never mounts + // so its onLayout callback (the primary flush site) never fires. This effect + // catches that case and flushes immediately after commit. No dependency array + // is intentional — we need to check after every render since bail-outs happen + // in conditional returns that can't trigger state-based effects. + useEffect(() => { + if (!didBailToFallbackState.current || !hasDeferredWrite(CONST.DEFERRED_LAYOUT_WRITE_KEYS.SEARCH)) { + return; + } + didBailToFallbackState.current = false; + flushDeferredWrite(CONST.DEFERRED_LAYOUT_WRITE_KEYS.SEARCH); + }); + const onLayoutChart = useCallback(() => { hasHadFirstLayout.current = true; endSpanWithAttributes(CONST.TELEMETRY.SPAN_NAVIGATE_TO_REPORTS, {[CONST.TELEMETRY.ATTRIBUTE_IS_WARM]: true}); @@ -1289,16 +1411,40 @@ function Search({ } const pending = getPendingSubmitFollowUpAction(); if (pending && pending.followUpAction !== CONST.TELEMETRY.SUBMIT_FOLLOW_UP_ACTION.DISMISS_MODAL_AND_OPEN_REPORT) { - endSubmitFollowUpActionSpan(pending.followUpAction); + endSubmitFollowUpActionSpan(pending.followUpAction, undefined, { + [CONST.TELEMETRY.ATTRIBUTE_IS_WARM]: !shouldShowLoadingState, + [CONST.TELEMETRY.ATTRIBUTE_WAS_LIST_EMPTY]: isSearchResultsEmptyRef.current, + }); } endSpanWithAttributes(CONST.TELEMETRY.SPAN_NAVIGATE_TO_REPORTS, { [CONST.TELEMETRY.ATTRIBUTE_IS_WARM]: !shouldShowLoadingState, }); markNavigateAfterExpenseCreateEnd(); spanExistedOnMount.current = false; + // On re-focus (e.g. DISMISS_MODAL_ONLY) onLayout won't re-fire — flush here. + flushDeferredWrite(CONST.DEFERRED_LAYOUT_WRITE_KEYS.SEARCH); }, [shouldShowLoadingState]), ); + // Reset before conditional returns. Only cancelNavigationSpans (error/empty paths) + // sets it to true. Must happen during render since it coordinates with the + // dep-free useEffect above — see comment on didBailToFallbackState. + didBailToFallbackState.current = false; + + // This is a performance optimization for the submit-expense->search path only. + // The SearchPage skeleton (useSearchLoadingState) doesn't cover this case because + // Search must mount for its onLayout to flush the deferred CreateMoneyRequest API write, which would block the JS thread causing a slowdown on post expense creation navigation + if (shouldShowRowSkeleton) { + return ( + + ); + } + if (searchResults === undefined) { Log.alert('[Search] Undefined search type'); cancelNavigationSpans(); @@ -1324,8 +1470,18 @@ function Search({ } const isAnyVisibleActionLoading = filteredData.some((item) => 'reportID' in item && item.reportID && isActionLoadingSet.has(`${ONYXKEYS.COLLECTION.REPORT_METADATA}${item.reportID}`)); const visibleDataLength = filteredData.filter((item) => item.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE || isOffline).length; - // Guard: don't render the empty view while heavy work is still deferred - the data is temporarily [] and not truly empty. - if (!shouldDeferHeavySearchWork && shouldShowEmptyState(isDataLoaded, visibleDataLength, searchDataType) && !isAnyVisibleActionLoading) { + // Guard: don't render the empty view while the data is transiently empty. + // - shouldDeferHeavySearchWork: skeleton is still showing, data hasn't been computed yet. + // - showPendingExpensePlaceholder: deferred write hasn't produced the optimistic item yet. + // - cachedOptimisticItemRef: an optimistic item was seen but a stale snapshot briefly removed it; + // stableSortedData will re-inject it, so the list isn't truly empty. + if ( + !shouldDeferHeavySearchWork && + !showPendingExpensePlaceholder && + !cachedOptimisticItemRef.current && + shouldShowEmptyState(isDataLoaded, visibleDataLength, searchDataType) && + !isAnyVisibleActionLoading + ) { cancelNavigationSpans(); return ( @@ -1412,7 +1568,7 @@ function Search({ ) : undefined } diff --git a/src/components/Skeletons/SearchRowSkeleton.tsx b/src/components/Skeletons/SearchRowSkeleton.tsx index 7c0d92e42915..b26f5230c8a0 100644 --- a/src/components/Skeletons/SearchRowSkeleton.tsx +++ b/src/components/Skeletons/SearchRowSkeleton.tsx @@ -1,5 +1,5 @@ import React from 'react'; -import type {StyleProp, ViewStyle} from 'react-native'; +import type {LayoutChangeEvent, StyleProp, ViewStyle} from 'react-native'; import {View} from 'react-native'; import {Circle} from 'react-native-svg'; import SkeletonRect from '@components/SkeletonRect'; @@ -18,6 +18,7 @@ type SearchRowSkeletonProps = { gradientOpacityEnabled?: boolean; containerStyle?: StyleProp; reasonAttributes: SkeletonSpanReasonAttributes; + onLayout?: (event: LayoutChangeEvent) => void; }; const barHeight = 8; @@ -36,7 +37,7 @@ const centralPanePadding = 40; // 80 is the width of the button on the right side const rightButtonWidth = 80; -function SearchRowSkeleton({shouldAnimate = true, fixedNumItems, gradientOpacityEnabled = false, containerStyle, reasonAttributes}: SearchRowSkeletonProps) { +function SearchRowSkeleton({shouldAnimate = true, fixedNumItems, gradientOpacityEnabled = false, containerStyle, reasonAttributes, onLayout}: SearchRowSkeletonProps) { const styles = useThemeStyles(); const {windowWidth} = useWindowDimensions(); const {shouldUseNarrowLayout, isLargeScreenWidth} = useResponsiveLayout(); @@ -50,6 +51,7 @@ function SearchRowSkeleton({shouldAnimate = true, fixedNumItems, gradientOpacity itemViewStyle={[styles.highlightBG, styles.mb2, styles.br3, styles.ml5]} gradientOpacityEnabled={gradientOpacityEnabled} shouldAnimate={shouldAnimate} + onLayout={onLayout} fixedNumItems={fixedNumItems} renderSkeletonItem={() => ( <> @@ -120,6 +122,7 @@ function SearchRowSkeleton({shouldAnimate = true, fixedNumItems, gradientOpacity shouldAnimate={shouldAnimate} fixedNumItems={fixedNumItems} gradientOpacityEnabled={gradientOpacityEnabled} + onLayout={onLayout} itemViewStyle={[styles.highlightBG, styles.mb2, styles.br3, styles.ml5]} renderSkeletonItem={() => ( <> diff --git a/src/libs/actions/IOU/PerDiem.ts b/src/libs/actions/IOU/PerDiem.ts index 178106a7c8cc..9b06917c8c76 100644 --- a/src/libs/actions/IOU/PerDiem.ts +++ b/src/libs/actions/IOU/PerDiem.ts @@ -6,10 +6,12 @@ import type {CreatePerDiemRequestParams} from '@libs/API/parameters'; import {WRITE_COMMANDS} from '@libs/API/types'; import {convertAmountToDisplayString, convertToFrontendAmountAsString} from '@libs/CurrencyUtils'; import DateUtils from '@libs/DateUtils'; +import {registerDeferredWrite} from '@libs/deferredLayoutWrite'; import {getMicroSecondOnyxErrorWithTranslationKey} from '@libs/ErrorUtils'; import {updateIOUOwnerAndTotal} from '@libs/IOUUtils'; import {formatPhoneNumber} from '@libs/LocalePhoneNumber'; import {validateAmount} from '@libs/MoneyRequestUtils'; +import isReportTopmostSplitNavigator from '@libs/Navigation/helpers/isReportTopmostSplitNavigator'; import Navigation from '@libs/Navigation/Navigation'; // eslint-disable-next-line @typescript-eslint/no-deprecated import {buildNextStepNew, buildOptimisticNextStep} from '@libs/NextStepUtils'; @@ -999,7 +1001,19 @@ function submitPerDiemExpense(submitPerDiemExpenseInformation: PerDiemExpenseInf if (shouldPlaySoundParam) { playSound(SOUNDS.DONE); } - API.write(WRITE_COMMANDS.CREATE_PER_DIEM_REQUEST, parameters, onyxData); + + const shouldDeferWrite = shouldHandleNavigation && isFromGlobalCreate && !isReportTopmostSplitNavigator(); + const apiWrite = () => { + API.write(WRITE_COMMANDS.CREATE_PER_DIEM_REQUEST, parameters, onyxData); + }; + + if (shouldDeferWrite) { + registerDeferredWrite(CONST.DEFERRED_LAYOUT_WRITE_KEYS.SEARCH, apiWrite, { + optimisticWatchKey: `${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`, + }); + } else { + apiWrite(); + } // eslint-disable-next-line @typescript-eslint/no-deprecated InteractionManager.runAfterInteractions(() => removeDraftTransaction(CONST.IOU.OPTIMISTIC_TRANSACTION_ID)); diff --git a/src/libs/actions/IOU/SendInvoice.ts b/src/libs/actions/IOU/SendInvoice.ts index 1af58b437347..deffc971dfa7 100644 --- a/src/libs/actions/IOU/SendInvoice.ts +++ b/src/libs/actions/IOU/SendInvoice.ts @@ -5,9 +5,11 @@ import * as API from '@libs/API'; import type {SendInvoiceParams} from '@libs/API/parameters'; import {WRITE_COMMANDS} from '@libs/API/types'; import DateUtils from '@libs/DateUtils'; +import {registerDeferredWrite} from '@libs/deferredLayoutWrite'; import {getMicroSecondOnyxErrorWithTranslationKey} from '@libs/ErrorUtils'; import {formatPhoneNumber} from '@libs/LocalePhoneNumber'; import Log from '@libs/Log'; +import isReportTopmostSplitNavigator from '@libs/Navigation/helpers/isReportTopmostSplitNavigator'; import {getReportActionHtml, getReportActionText} from '@libs/ReportActionsUtils'; import type {OptimisticChatReport, OptimisticCreatedReportAction, OptimisticIOUReportAction} from '@libs/ReportUtils'; import { @@ -19,7 +21,6 @@ import { getPersonalDetailsForAccountID, } from '@libs/ReportUtils'; import playSound, {SOUNDS} from '@libs/Sound'; -import {startSpan} from '@libs/telemetry/activeSpans'; import {buildOptimisticTransaction} from '@libs/TransactionUtils'; import {buildOptimisticPolicyRecentlyUsedTags} from '@userActions/Policy/Tag'; import {notifyNewAction} from '@userActions/Report'; @@ -802,20 +803,21 @@ function sendInvoice({ ...(invoiceChatReport?.reportID ? {receiverInvoiceRoomID: invoiceChatReport.reportID} : {receiverEmail: receiver.login ?? ''}), }; - startSpan(CONST.TELEMETRY.SPAN_SUBMIT_TO_DESTINATION_VISIBLE, { - name: 'submit-to-destination-visible', - op: CONST.TELEMETRY.SPAN_SUBMIT_TO_DESTINATION_VISIBLE, - attributes: { - [CONST.TELEMETRY.ATTRIBUTE_SCENARIO]: CONST.TELEMETRY.SUBMIT_EXPENSE_SCENARIO.INVOICE, - [CONST.TELEMETRY.ATTRIBUTE_HAS_RECEIPT]: !!receiptFile, - [CONST.TELEMETRY.ATTRIBUTE_IS_FROM_GLOBAL_CREATE]: isFromGlobalCreate, - [CONST.TELEMETRY.ATTRIBUTE_IOU_TYPE]: CONST.IOU.TYPE.INVOICE, - [CONST.TELEMETRY.ATTRIBUTE_IOU_REQUEST_TYPE]: 'invoice', - }, - }); - playSound(SOUNDS.DONE); - API.write(WRITE_COMMANDS.SEND_INVOICE, parameters, onyxData); + + const shouldDeferWrite = isFromGlobalCreate && !isReportTopmostSplitNavigator(); + const apiWrite = () => { + API.write(WRITE_COMMANDS.SEND_INVOICE, parameters, onyxData); + }; + + if (shouldDeferWrite) { + registerDeferredWrite(CONST.DEFERRED_LAYOUT_WRITE_KEYS.SEARCH, apiWrite, { + optimisticWatchKey: `${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`, + }); + } else { + apiWrite(); + } + // eslint-disable-next-line @typescript-eslint/no-deprecated InteractionManager.runAfterInteractions(() => removeDraftTransaction(CONST.IOU.OPTIMISTIC_TRANSACTION_ID)); diff --git a/src/libs/actions/IOU/index.ts b/src/libs/actions/IOU/index.ts index 224ddb58cf49..46a6dc62e110 100644 --- a/src/libs/actions/IOU/index.ts +++ b/src/libs/actions/IOU/index.ts @@ -39,6 +39,7 @@ import type { import {WRITE_COMMANDS} from '@libs/API/types'; import {convertToDisplayString} from '@libs/CurrencyUtils'; import DateUtils from '@libs/DateUtils'; +import {registerDeferredWrite} from '@libs/deferredLayoutWrite'; import DistanceRequestUtils from '@libs/DistanceRequestUtils'; import {getMicroSecondOnyxErrorObject, getMicroSecondOnyxErrorWithTranslationKey} from '@libs/ErrorUtils'; import {readFileAsync} from '@libs/fileDownload/FileUtils'; @@ -6422,6 +6423,15 @@ function requestMoney(requestMoneyInformation: RequestMoneyInformation): {iouRep playSound(SOUNDS.DONE); } + // API.write is deferred until the Search screen's actual content (not skeleton) + // lays out, so that Onyx optimistic updates don't block the JS thread while + // the skeleton→content transition is in progress. The Search component flushes + // the registered write from its content onLayout callback. + // Only the SUBMIT and default (REQUEST_MONEY) branches wrap the write; the actual + // deferral only activates when we navigate to Search (shouldHandleNavigation && !isRetry). + // CATEGORIZE and SHARE navigate elsewhere and don't benefit from this deferral. + let deferredAPIWrite: (() => void) | undefined; + switch (action) { case CONST.IOU.ACTION.SUBMIT: { if (!linkedTrackedExpenseReportAction || !linkedTrackedExpenseReportID) { @@ -6449,42 +6459,44 @@ function requestMoney(requestMoneyInformation: RequestMoneyInformation): {iouRep } : undefined; const isDistanceRequest = isDistanceRequestTransactionUtils(transaction); - convertTrackedExpenseToRequest({ - payerParams: { - accountID: payerAccountID, - email: payerEmail, - }, - transactionParams: { - amount, - distance, - currency, - comment, - merchant, - created, - attendees, - transactionID: transaction.transactionID, - actionableWhisperReportActionID, - linkedTrackedExpenseReportAction, - linkedTrackedExpenseReportID, - transactionThreadReportID: transactionThreadReportID ?? iouAction?.childReportID, - isLinkedTrackedExpenseReportArchived, - isDistance: isDistanceRequest, - customUnitRateID: isDistanceRequest ? customUnitRateID : undefined, - waypoints: isDistanceRequest ? sanitizedWaypoints : undefined, - }, - chatParams: { - reportID: chatReport.reportID, - createdReportActionID: createdChatReportActionID, - reportPreviewReportActionID: reportPreviewAction.reportActionID, - }, - iouParams: { - reportID: iouReport.reportID, - createdReportActionID: createdIOUReportActionID, - reportActionID: iouAction.reportActionID, - }, - onyxData, - workspaceParams, - }); + deferredAPIWrite = () => { + convertTrackedExpenseToRequest({ + payerParams: { + accountID: payerAccountID, + email: payerEmail, + }, + transactionParams: { + amount, + distance, + currency, + comment, + merchant, + created, + attendees, + transactionID: transaction.transactionID, + actionableWhisperReportActionID, + linkedTrackedExpenseReportAction, + linkedTrackedExpenseReportID, + transactionThreadReportID: transactionThreadReportID ?? iouAction?.childReportID, + isLinkedTrackedExpenseReportArchived, + isDistance: isDistanceRequest, + customUnitRateID: isDistanceRequest ? customUnitRateID : undefined, + waypoints: isDistanceRequest ? sanitizedWaypoints : undefined, + }, + chatParams: { + reportID: chatReport.reportID, + createdReportActionID: createdChatReportActionID, + reportPreviewReportActionID: reportPreviewAction.reportActionID, + }, + iouParams: { + reportID: iouReport.reportID, + createdReportActionID: createdIOUReportActionID, + reportActionID: iouAction.reportActionID, + }, + onyxData, + workspaceParams, + }); + }; break; } default: { @@ -6543,7 +6555,9 @@ function requestMoney(requestMoneyInformation: RequestMoneyInformation): {iouRep shouldDeferAutoSubmit, }; // eslint-disable-next-line rulesdir/no-multiple-api-calls - API.write(WRITE_COMMANDS.REQUEST_MONEY, parameters, onyxData); + deferredAPIWrite = () => { + API.write(WRITE_COMMANDS.REQUEST_MONEY, parameters, onyxData); + }; } } @@ -6557,6 +6571,18 @@ function requestMoney(requestMoneyInformation: RequestMoneyInformation): {iouRep } } + // Register the deferred write BEFORE navigation so the Search component's + // hasDeferredWrite() check on mount always sees the pending channel. + if (deferredAPIWrite) { + if (shouldHandleNavigation && !requestMoneyInformation.isRetry && isFromGlobalCreate && !isReportTopmostSplitNavigator()) { + registerDeferredWrite(CONST.DEFERRED_LAYOUT_WRITE_KEYS.SEARCH, deferredAPIWrite, { + optimisticWatchKey: `${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`, + }); + } else { + deferredAPIWrite(); + } + } + if (!requestMoneyInformation.isRetry) { highlightTransactionOnSearchRouteIfNeeded(isFromGlobalCreate, transaction.transactionID, CONST.SEARCH.DATA_TYPES.EXPENSE); @@ -6934,7 +6960,18 @@ function trackExpense(params: CreateTrackExpenseParams) { parameters.actionableWhisperReportActionID = actionableWhisperReportActionIDParam; } - API.write(WRITE_COMMANDS.TRACK_EXPENSE, parameters, onyxData); + const shouldDeferWrite = shouldHandleNavigation && !params.isRetry && isFromGlobalCreate && !isReportTopmostSplitNavigator(); + const apiWrite = () => { + API.write(WRITE_COMMANDS.TRACK_EXPENSE, parameters, onyxData); + }; + + if (shouldDeferWrite) { + registerDeferredWrite(CONST.DEFERRED_LAYOUT_WRITE_KEYS.SEARCH, apiWrite, { + optimisticWatchKey: `${ONYXKEYS.COLLECTION.TRANSACTION}${transaction?.transactionID}`, + }); + } else { + apiWrite(); + } } } @@ -7802,7 +7839,19 @@ function createDistanceRequest(distanceRequestInformation: CreateDistanceRequest playSound(SOUNDS.DONE); } - API.write(WRITE_COMMANDS.CREATE_DISTANCE_REQUEST, parameters, onyxData); + const shouldDeferWrite = shouldHandleNavigation && isFromGlobalCreate && !isReportTopmostSplitNavigator(); + const apiWrite = () => { + API.write(WRITE_COMMANDS.CREATE_DISTANCE_REQUEST, parameters, onyxData); + }; + + if (shouldDeferWrite) { + registerDeferredWrite(CONST.DEFERRED_LAYOUT_WRITE_KEYS.SEARCH, apiWrite, { + optimisticWatchKey: `${ONYXKEYS.COLLECTION.TRANSACTION}${parameters.transactionID}`, + }); + } else { + apiWrite(); + } + // eslint-disable-next-line @typescript-eslint/no-deprecated InteractionManager.runAfterInteractions(() => removeDraftTransaction(CONST.IOU.OPTIMISTIC_TRANSACTION_ID)); const activeReportID = isMoneyRequestReport && report?.reportID ? report.reportID : parameters.chatReportID; diff --git a/src/libs/deferredLayoutWrite.ts b/src/libs/deferredLayoutWrite.ts new file mode 100644 index 000000000000..ec75f126eaef --- /dev/null +++ b/src/libs/deferredLayoutWrite.ts @@ -0,0 +1,125 @@ +/** + * Coordinates deferred API.write() calls with screen content layout transitions. + * + * Problem: API.write() applies optimistic Onyx data synchronously, which triggers + * expensive collection-level re-renders. When navigating to a screen after an action + * like expense creation, firing this during the skeleton->content transition blocks + * the JS thread and makes the skeleton hang. + * + * Solution: The action registers its write via `registerDeferredWrite(key, cb)`; + * the target component flushes it from its content onLayout callback via `flushDeferredWrite(key)`. + * A per-channel safety timeout (default 5s) ensures the write always fires even if + * the target screen never mounts or the user navigates elsewhere. + * + * Note: The Search component has its own 10s safety timeout (clearOptimisticTracking) + * for the UI-level optimistic item cache. The two timeouts serve different layers: + * - 5s (here): guarantees the API.write() executes. + * - 10s (Search): guarantees the skeleton/ghost-row UI clears if the optimistic + * item never reaches sortedData (e.g. empty list, API failure, offline). + */ +import {AppState} from 'react-native'; +import type {OnyxKey} from 'react-native-onyx'; +import Log from './Log'; + +const DEFAULT_SAFETY_TIMEOUT_MS = 5000; + +type DeferredChannel = { + write: () => void; + safetyTimeoutId: ReturnType; + + /** + * An Onyx key that the deferred write will create via optimistic data. + * Consumer components can subscribe to this key with useOnyx to know + * when the optimistic updates have been applied. + */ + optimisticWatchKey?: OnyxKey; +}; + +const channels = new Map(); + +function clearChannelTimeout(channel: DeferredChannel) { + clearTimeout(channel.safetyTimeoutId); +} + +type DeferredWriteOptions = { + safetyTimeoutMs?: number; + optimisticWatchKey?: OnyxKey; +}; + +/** + * Register a callback to be executed when the target component lays out. + * If a previous write for the same key is still pending it is flushed + * immediately before registering the new one. + */ +function registerDeferredWrite(key: string, callback: () => void, options: DeferredWriteOptions = {}) { + const {safetyTimeoutMs = DEFAULT_SAFETY_TIMEOUT_MS, optimisticWatchKey} = options; + + const existing = channels.get(key); + if (existing) { + Log.warn(`[DeferredLayoutWrite] Overwriting unflushed deferred write for key "${key}" - flushing the pending one first`); + flushDeferredWrite(key); + } + + const safetyTimeoutId = setTimeout(() => { + Log.warn(`[DeferredLayoutWrite] Safety timeout (${safetyTimeoutMs}ms) fired for key "${key}" - the target component likely never laid out`); + flushDeferredWrite(key); + }, safetyTimeoutMs); + + channels.set(key, {write: callback, safetyTimeoutId, optimisticWatchKey}); +} + +/** + * Execute and clear the pending deferred write for the given key. + * Called by the target component when actual content (not skeleton) lays out. + */ +function flushDeferredWrite(key: string) { + const channel = channels.get(key); + if (!channel) { + return; + } + + clearChannelTimeout(channel); + channels.delete(key); + channel.write(); +} + +/** + * Cancel a pending deferred write without executing the callback. + * Clears the safety timeout. No-op if no channel is registered for the key. + */ +function cancelDeferredWrite(key: string) { + const channel = channels.get(key); + if (!channel) { + return; + } + clearChannelTimeout(channel); + channels.delete(key); +} + +function hasDeferredWrite(key: string): boolean { + return channels.has(key); +} + +/** + * Returns the Onyx key that the deferred write for the given channel will + * create via optimistic data. Returns undefined when no channel is registered + * or the channel was registered without a watch key. + */ +function getOptimisticWatchKey(key: string): OnyxKey | undefined { + return channels.get(key)?.optimisticWatchKey; +} + +// Flush every pending deferred write when the app moves to background so +// that API.write() calls are persisted to the SequentialQueue before the OS +// can kill the process. +AppState.addEventListener('change', (nextState) => { + if (nextState === 'active' || channels.size === 0) { + return; + } + Log.info(`[DeferredLayoutWrite] App going to "${nextState}" - flushing ${channels.size} pending deferred write(s)`); + for (const key of [...channels.keys()]) { + flushDeferredWrite(key); + } +}); + +export {registerDeferredWrite, flushDeferredWrite, cancelDeferredWrite, hasDeferredWrite, getOptimisticWatchKey}; diff --git a/src/libs/telemetry/activeSpans.ts b/src/libs/telemetry/activeSpans.ts index e6c334140e10..6e6b09b2cf3b 100644 --- a/src/libs/telemetry/activeSpans.ts +++ b/src/libs/telemetry/activeSpans.ts @@ -49,7 +49,7 @@ function endSpan(spanId: string) { const {span, startTime} = entry; const now = performance.now(); const durationMs = Math.round(now - startTime); - console.debug(`[Sentry][${spanId}] Ending span (${durationMs}ms)`, {spanId, durationMs, timestamp: now}); + console.debug(`[Sentry][${spanId}] Ending span (${durationMs}ms)`, {spanId, durationMs, timestamp: now, attributes: Sentry.spanToJSON(span).data}); span.setStatus({code: 1}); span.setAttribute(CONST.TELEMETRY.ATTRIBUTE_FINISHED_MANUALLY, true); span.end(); diff --git a/src/libs/telemetry/submitFollowUpAction.ts b/src/libs/telemetry/submitFollowUpAction.ts index 83803efeab0f..2f324636e4ac 100644 --- a/src/libs/telemetry/submitFollowUpAction.ts +++ b/src/libs/telemetry/submitFollowUpAction.ts @@ -3,6 +3,7 @@ * (e.g. dismiss modal and open report, dismiss modal only, navigate to search) is complete and the target screen is visible. * Uses submit_follow_up_action attribute to record which action was taken. */ +import type {SpanAttributeValue} from '@sentry/core'; import type {ValueOf} from 'type-fest'; import CONST from '@src/CONST'; import {cancelSpan, endSpanWithAttributes, getSpan} from './activeSpans'; @@ -86,7 +87,7 @@ function getPendingSubmitFollowUpAction(): PendingSubmitFollowUpAction { * End the submit-to-visible span and clear the pending action. Call from each screen when its main content is visible (e.g. onLayout). * Only ends the span if the passed followUpAction matches the current pending action (avoids races and wrong attribution). */ -function endSubmitFollowUpActionSpan(followUpAction: SubmitFollowUpAction, reportID?: string) { +function endSubmitFollowUpActionSpan(followUpAction: SubmitFollowUpAction, reportID?: string, extraAttributes?: Record) { if (!getSpan(CONST.TELEMETRY.SPAN_SUBMIT_TO_DESTINATION_VISIBLE)) { return; } @@ -97,8 +98,9 @@ function endSubmitFollowUpActionSpan(followUpAction: SubmitFollowUpAction, repor if (pending.reportID !== undefined && pending.reportID !== reportID) { return; } - const attributes: Record = { + const attributes: Record = { [CONST.TELEMETRY.ATTRIBUTE_SUBMIT_FOLLOW_UP_ACTION]: followUpAction, + ...extraAttributes, }; if (reportID !== undefined) { attributes[CONST.TELEMETRY.ATTRIBUTE_REPORT_ID] = reportID; diff --git a/src/pages/iou/request/step/IOURequestStepCompanyInfo.tsx b/src/pages/iou/request/step/IOURequestStepCompanyInfo.tsx index 40a76974758e..3d995285fa55 100644 --- a/src/pages/iou/request/step/IOURequestStepCompanyInfo.tsx +++ b/src/pages/iou/request/step/IOURequestStepCompanyInfo.tsx @@ -13,6 +13,7 @@ import usePolicy from '@hooks/usePolicy'; import useThemeStyles from '@hooks/useThemeStyles'; import {getDefaultCompanyWebsite} from '@libs/BankAccountUtils'; import {convertToDisplayString} from '@libs/CurrencyUtils'; +import {startSpan} from '@libs/telemetry/activeSpans'; import {extractUrlDomain} from '@libs/Url'; import {getFieldRequiredErrors, isPublicDomain, isValidWebsite} from '@libs/ValidationUtils'; import Navigation from '@navigation/Navigation'; @@ -77,6 +78,18 @@ function IOURequestStepCompanyInfo({route, report, transaction}: IOURequestStepC const submit = (values: FormOnyxValues) => { const companyWebsite = Str.sanitizeURL(values.companyWebsite, CONST.COMPANY_WEBSITE_DEFAULT_SCHEME); + const isFromGlobalCreate = transaction?.isFromFloatingActionButton ?? transaction?.isFromGlobalCreate; + startSpan(CONST.TELEMETRY.SPAN_SUBMIT_TO_DESTINATION_VISIBLE, { + name: 'submit-to-destination-visible', + op: CONST.TELEMETRY.SPAN_SUBMIT_TO_DESTINATION_VISIBLE, + attributes: { + [CONST.TELEMETRY.ATTRIBUTE_SCENARIO]: CONST.TELEMETRY.SUBMIT_EXPENSE_SCENARIO.INVOICE, + [CONST.TELEMETRY.ATTRIBUTE_HAS_RECEIPT]: false, + [CONST.TELEMETRY.ATTRIBUTE_IS_FROM_GLOBAL_CREATE]: !!isFromGlobalCreate, + [CONST.TELEMETRY.ATTRIBUTE_IOU_TYPE]: CONST.IOU.TYPE.INVOICE, + [CONST.TELEMETRY.ATTRIBUTE_IOU_REQUEST_TYPE]: 'invoice', + }, + }); sendInvoice({ currentUserAccountID: currentUserPersonalDetails.accountID, transaction, diff --git a/tests/actions/IOUTest.ts b/tests/actions/IOUTest.ts index e17c5af77198..24cc3a4bad77 100644 --- a/tests/actions/IOUTest.ts +++ b/tests/actions/IOUTest.ts @@ -165,6 +165,16 @@ jest.mock('@src/libs/actions/Report', () => { }); jest.mock('@libs/Navigation/helpers/isSearchTopmostFullScreenRoute', () => jest.fn()); jest.mock('@libs/Navigation/helpers/isReportTopmostSplitNavigator', () => jest.fn()); +// In production, requestMoney defers its API.write() call until the target screen's +// content lays out (or a safety timeout fires). In tests there is no target component +// to flush the deferred write, so we bypass the deferral by executing the callback immediately. +jest.mock('@libs/deferredLayoutWrite', () => ({ + registerDeferredWrite: (_key: string, callback: () => void) => callback(), + flushDeferredWrite: jest.fn(), + cancelDeferredWrite: jest.fn(), + hasDeferredWrite: () => false, + getOptimisticWatchKey: () => undefined, +})); jest.mock('@hooks/useCardFeedsForDisplay', () => jest.fn(() => ({defaultCardFeed: null, cardFeedsByPolicy: {}}))); const unapprovedCashHash = 71801560; diff --git a/tests/unit/deferredLayoutWriteTest.ts b/tests/unit/deferredLayoutWriteTest.ts new file mode 100644 index 000000000000..6cd6096d8ac4 --- /dev/null +++ b/tests/unit/deferredLayoutWriteTest.ts @@ -0,0 +1,123 @@ +import {AppState} from 'react-native'; +import {cancelDeferredWrite, flushDeferredWrite, getOptimisticWatchKey, hasDeferredWrite, registerDeferredWrite} from '@libs/deferredLayoutWrite'; + +beforeEach(() => { + jest.useFakeTimers(); +}); + +afterEach(() => { + jest.useRealTimers(); +}); + +describe('deferredLayoutWrite', () => { + it('registers and flushes a deferred write', () => { + const callback = jest.fn(); + registerDeferredWrite('test', callback); + + expect(hasDeferredWrite('test')).toBe(true); + expect(callback).not.toHaveBeenCalled(); + + flushDeferredWrite('test'); + + expect(callback).toHaveBeenCalledTimes(1); + expect(hasDeferredWrite('test')).toBe(false); + }); + + it('fires the safety timeout when not flushed', () => { + const callback = jest.fn(); + registerDeferredWrite('test', callback, {safetyTimeoutMs: 3000}); + + expect(callback).not.toHaveBeenCalled(); + + jest.advanceTimersByTime(3000); + + expect(callback).toHaveBeenCalledTimes(1); + expect(hasDeferredWrite('test')).toBe(false); + }); + + it('does not double-fire after flush + timeout', () => { + const callback = jest.fn(); + registerDeferredWrite('test', callback, {safetyTimeoutMs: 3000}); + + flushDeferredWrite('test'); + jest.advanceTimersByTime(3000); + + expect(callback).toHaveBeenCalledTimes(1); + }); + + it('flushes the previous write when overwriting with a new one', () => { + const first = jest.fn(); + const second = jest.fn(); + + registerDeferredWrite('test', first); + registerDeferredWrite('test', second); + + expect(first).toHaveBeenCalledTimes(1); + expect(second).not.toHaveBeenCalled(); + + flushDeferredWrite('test'); + expect(second).toHaveBeenCalledTimes(1); + }); + + it('cancels a deferred write without executing it', () => { + const callback = jest.fn(); + registerDeferredWrite('test', callback, {safetyTimeoutMs: 3000}); + + cancelDeferredWrite('test'); + + expect(callback).not.toHaveBeenCalled(); + expect(hasDeferredWrite('test')).toBe(false); + + jest.advanceTimersByTime(3000); + expect(callback).not.toHaveBeenCalled(); + }); + + it('returns the optimisticWatchKey when registered', () => { + registerDeferredWrite('test', jest.fn(), {optimisticWatchKey: 'transactions_123'}); + + expect(getOptimisticWatchKey('test')).toBe('transactions_123'); + + flushDeferredWrite('test'); + expect(getOptimisticWatchKey('test')).toBeUndefined(); + }); + + it('returns undefined for hasDeferredWrite and getOptimisticWatchKey on unknown keys', () => { + expect(hasDeferredWrite('unknown')).toBe(false); + expect(getOptimisticWatchKey('unknown')).toBeUndefined(); + }); + + it('is a no-op when flushing or cancelling an unknown key', () => { + expect(() => flushDeferredWrite('unknown')).not.toThrow(); + expect(() => cancelDeferredWrite('unknown')).not.toThrow(); + }); + + it('flushes all pending writes when the app goes to background', () => { + const callbackA = jest.fn(); + const callbackB = jest.fn(); + + registerDeferredWrite('a', callbackA); + registerDeferredWrite('b', callbackB); + + expect(callbackA).not.toHaveBeenCalled(); + expect(callbackB).not.toHaveBeenCalled(); + + (AppState as unknown as {emitCurrentTestState: (state: string) => void}).emitCurrentTestState('background'); + + expect(callbackA).toHaveBeenCalledTimes(1); + expect(callbackB).toHaveBeenCalledTimes(1); + expect(hasDeferredWrite('a')).toBe(false); + expect(hasDeferredWrite('b')).toBe(false); + }); + + it('does not flush writes when the app returns to active state', () => { + const callback = jest.fn(); + registerDeferredWrite('test', callback); + + (AppState as unknown as {emitCurrentTestState: (state: string) => void}).emitCurrentTestState('active'); + + expect(callback).not.toHaveBeenCalled(); + expect(hasDeferredWrite('test')).toBe(true); + + flushDeferredWrite('test'); + }); +});