[$250] Expenses flash yellow highlight repeatedly instead of once

[mountiny]

r## Bug Description

Slack thread https://expensify.slack.com/archives/C05LX9D6E07/p1771814813477409

When creating expenses and viewing the Expenses page (Reports > Expenses), newly created expenses flash yellow (highlight animation) correctly on first appearance, but then flash again repeatedly on subsequent search result updates. The highlight should only play once per new expense.

Expected Behavior

Each expense should flash yellow exactly once when it first appears in the Expenses list.

Actual Behavior

The same expense flashes yellow repeatedly whenever search results are refreshed (e.g., due to Onyx updates, API responses, or other transaction mutations while on the page).

Root Cause

There is a race condition between two cleanup useEffects in src/hooks/useSearchHighlightAndScroll.ts, combined with a missing deduplication guard.

Race condition between Effect A and Effect B

Two effects fire when newSearchResultKeys transitions from null to a non-null Set:

• Effect A (lines 234-258): requestAnimationFrame → setTimeout(300ms) → clears transactionIDsToHighlight in Onyx
• Effect B (lines 268-279): setTimeout(300ms) → setNewSearchResultKeys(null)

Effect B fires first (~300ms) because Effect A has an extra requestAnimationFrame delay (~16ms). When Effect B fires:

1. setNewSearchResultKeys(null) triggers a React re-render
2. Effect A's cleanup function runs, calling clearTimeout(timer) and cancelAnimationFrame(animation) — this cancels the pending Onyx clear
3. Effect A re-runs with newSearchResultKeys === null and hits the early return at line 236
4. Result: transactionIDsToHighlight stays {transactionID: true} in Onyx permanently (until the user navigates away from the search type)

Missing deduplication for manual highlights

After the first (correct) highlight, transactionIDsToHighlight is never cleared. Then any subsequent change to searchResults.data re-fires the "detect new items" effect (line 182). The filter at lines 210-213 unconditionally returns true for IDs found in manualHighlightTransactionIDs, bypassing the highlightedIDs deduplication check:

const newTransactionIDs = currentTransactionIDs.filter((id) => {
    if (manualHighlightTransactionIDs.has(id)) {
        return true;  // <-- Always true, bypasses highlightedIDs check
    }
    // ...
});

This causes newSearchResultKeys to be set again, and the highlight animation replays. The same cleanup race repeats indefinitely.

Proposed Fix

Fix 1 — Eliminate the race condition: Ensure transactionIDsToHighlight is cleared reliably regardless of when newSearchResultKeys is reset. For example, clear transactionIDsToHighlight immediately when the highlight is consumed in the "detect new items" effect (line 230), rather than in a separate effect with a timer. Alternatively, decouple the two cleanups so Effect A doesn't depend on newSearchResultKeys in its dependency array.

Fix 2 — Add deduplication for manual highlights: At line 211, also check highlightedIDs.current.has(id) before returning true:

if (manualHighlightTransactionIDs.has(id)) {
    return !highlightedIDs.current.has(id);
}

Reproduction Steps

1. Open the Expenses page (Reports > Expenses)
2. Create a new expense using the global create button (e.g., Scan, Manual, Distance)
3. Observe the expense flashes yellow once (correct)
4. Wait for any background data refresh or create another expense
5. Observe the previously created expense flashes yellow again (incorrect)

Key Files

• src/hooks/useSearchHighlightAndScroll.ts — the highlight tracking and cleanup logic
• src/hooks/useAnimatedHighlightStyle/index.ts — the yellow flash animation
• src/libs/actions/IOU/index.ts (line 1142) — where transactionIDsToHighlight is set on expense creation
• src/libs/actions/Transaction.ts (line 1545) — mergeTransactionIdsHighlightOnSearchRoute Onyx merge function
• src/components/Search/index.tsx (lines 1038-1063) — where shouldAnimateInHighlight is derived from newSearchResultKeys

Platform

Web, iOS, Android (all platforms using the Expenses search page)

Issue Owner

Current Issue Owner: @FitseTLT

[melvin-bot]

Triggered auto assignment to @johncschuster (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @FitseTLT (External)

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~022025923120731812559

[nkdengineer]

@mountiny I'd like to work on this if it's open for contributor since the solution is detailed in the OP

[MelvinBot]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Expenses on the Expenses search page flash yellow (highlight animation) repeatedly instead of once. After creating an expense, the yellow highlight replays on every subsequent search result update rather than playing a single time.

What is the root cause of that problem?

There are two interacting bugs in src/hooks/useSearchHighlightAndScroll.ts:

Bug 1 — Race condition between two cleanup effects prevents transactionIDsToHighlight from being cleared:

Effect A (line 235-258) clears transactionIDsToHighlight in Onyx via requestAnimationFrame → setTimeout(300ms) (~316ms total). Effect B (line 269-279) resets newSearchResultKeys to null via setTimeout(300ms) (300ms total). Effect B fires ~16ms earlier because it skips the requestAnimationFrame. When Effect B fires:

1. setNewSearchResultKeys(null) triggers a re-render
2. Since newSearchResultKeys is in Effect A's dependency array (line 258), React runs Effect A's cleanup — canceling the pending Onyx clear
3. Effect A re-runs with newSearchResultKeys === null and hits the early return at line 236
4. Result: transactionIDsToHighlight stays {transactionID: true} in Onyx permanently

Bug 2 — Missing deduplication for manual highlights:

The filter at lines 210-213 unconditionally returns true for IDs found in manualHighlightTransactionIDs, bypassing the highlightedIDs deduplication check. Since transactionIDsToHighlight is never cleared (Bug 1), every subsequent change to searchResults.data re-triggers the detect-new-items effect (line 182), and the same IDs pass the filter again, replaying the animation.

What changes do you think we should make in order to solve the problem?

Fix 1 — Eliminate the race condition in src/hooks/useSearchHighlightAndScroll.ts: Remove newSearchResultKeys from Effect A's dependency array (line 258) so that Effect B's state update doesn't cancel Effect A's pending Onyx clear. Alternatively, clear transactionIDsToHighlight synchronously when the highlight is consumed in the detect-new-items effect (around line 230), rather than in a separate timer-based effect.

Fix 2 — Add deduplication for manual highlights at line 211: Check highlightedIDs.current before returning true for manual highlight IDs:

if (manualHighlightTransactionIDs.has(id)) {
    return !highlightedIDs.current.has(`${ONYXKEYS.COLLECTION.TRANSACTION}${id}`);
}

Note: highlightedIDs stores prefixed keys (e.g., transactions_123) at line 227 but the initialization at line 177 stores raw IDs — the dedup check at line 211 should use the prefixed format to match what's added at line 227.

What alternative solutions did you explore?

An alternative for Fix 1 would be to make both effects use the same total delay (both use requestAnimationFrame + setTimeout, or neither). However, decoupling Effect A from newSearchResultKeys is cleaner since Effect A's job (clearing Onyx) shouldn't depend on whether newSearchResultKeys has been reset yet.

Detailed Investigation

Evidence Chain

1. IOU action sets highlight: src/libs/actions/IOU/index.ts:1142 calls mergeTransactionIdsHighlightOnSearchRoute(type, {[transactionID]: true})
2. Hook reads highlight from Onyx: useSearchHighlightAndScroll.ts:61-64 subscribes to TRANSACTION_IDS_HIGHLIGHT_ON_SEARCH_ROUTE
3. Detect-new-items effect builds manualHighlightTransactionIDs: Line 207 creates a Set from truthy entries in transactionIDsToHighlight
4. Filter bypasses dedup for manual IDs: Lines 210-213 return true without checking highlightedIDs
5. setNewSearchResultKeys(newKeys) triggers highlight: Line 230
6. sortedData derives shouldAnimateInHighlight: Search/index.tsx:1038-1063 checks newSearchResultKeys?.has(baseKey)
7. TransactionListItem passes to animation hook: TransactionListItem.tsx:111-113 — shouldHighlight: item?.shouldAnimateInHighlight ?? false
8. Animation replays on shouldHighlight cycle: useAnimatedHighlightStyle/index.ts:89-98 fires when shouldHighlight transitions false → true
9. Race condition prevents cleanup: Effect A (rAF + 300ms) is canceled by Effect B (300ms) resetting newSearchResultKeys to null

Relevant Code

• src/hooks/useSearchHighlightAndScroll.ts:210-213 — Missing dedup guard for manual highlights
• src/hooks/useSearchHighlightAndScroll.ts:235-258 — Effect A: cleanup with rAF + setTimeout, canceled by race
• src/hooks/useSearchHighlightAndScroll.ts:269-279 — Effect B: resets newSearchResultKeys, fires ~16ms earlier
• src/hooks/useAnimatedHighlightStyle/index.ts:89-98 — Animation trigger, replays on shouldHighlight cycle
• src/libs/actions/IOU/index.ts:1142 — Where transactionIDsToHighlight is set
• src/libs/actions/Transaction.ts:1545-1547 — mergeTransactionIdsHighlightOnSearchRoute Onyx merge

---

Next Steps for Contributor+ team: Reply with @MelvinBot implement this to create a draft PR, or @MelvinBot <your feedback> to refine this analysis.

[mountiny]

I dont know if the solution is the best one so I think you should still make a proposal

[nkdengineer]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Expenses flash yellow repeatedly whenever search results are refreshed (e.g., due to Onyx updates, API responses, or other transaction mutations). Each expense should only flash once when it first appears in the Expenses list.

What is the root cause of that problem?

There are two issues in src/hooks/useSearchHighlightAndScroll.ts:

1. Race condition between two cleanup effects

Two effects fire when newSearchResultKeys transitions from null to a non-null Set:

• Effect A (lines 234-258): requestAnimationFrame → setTimeout(300ms) → clears transactionIDsToHighlight in Onyx
• Effect B (lines 268-279): setTimeout(300ms) → setNewSearchResultKeys(null)

App/src/hooks/useSearchHighlightAndScroll.ts

Lines 234 to 258 in d58572f

	useEffect(() => {
	if (isEmptyObject(transactionIDsToHighlight) || newSearchResultKeys === null) {
	return;
	}
	const highlightedTransactionIDs = Object.keys(transactionIDsToHighlight).filter(
	(id) => transactionIDsToHighlight[id] && newSearchResultKeys?.has(`${ONYXKEYS.COLLECTION.TRANSACTION}${id}`),
	);
	// We need to use requestAnimationFrame here to ensure that setTimeout actually starts
	// only after the user has navigated to the "Reports > Expenses" page.
	// Otherwise, there is still a chance we might miss the timing because setTimeout runs too early,
	// causing the highlight not to appear.
	let timer: NodeJS.Timeout;
	const animation = requestAnimationFrame(() => {
	timer = setTimeout(() => {
	mergeTransactionIdsHighlightOnSearchRoute(queryJSON.type, Object.fromEntries(highlightedTransactionIDs.map((id) => [id, false])));
	}, CONST.ANIMATED_HIGHLIGHT_START_DURATION);
	});
	return () => {
	clearTimeout(timer);
	cancelAnimationFrame(animation);
	};
	}, [transactionIDsToHighlight, queryJSON.type, newSearchResultKeys]);

App/src/hooks/useSearchHighlightAndScroll.ts

Lines 268 to 279 in d58572f

	useEffect(() => {
	if (newSearchResultKeys === null) {
	return;
	}
	const timer = setTimeout(() => {
	setNewSearchResultKeys(null);
	}, CONST.ANIMATED_HIGHLIGHT_START_DURATION);
	return () => clearTimeout(timer);
	}, [newSearchResultKeys]);

Effect B fires first (~300ms) because Effect A has an extra requestAnimationFrame delay (~16ms). When Effect B fires, setNewSearchResultKeys(null) triggers a re-render, which runs Effect A's cleanup (clearTimeout + cancelAnimationFrame), canceling the pending Onyx clear. Effect A then re-runs with newSearchResultKeys === null and hits the early return at line 235. Result: transactionIDsToHighlight stays {transactionID: true} in Onyx permanently.

2. Missing deduplication for manual highlights

After the first (correct) highlight, transactionIDsToHighlight is never cleared (due to issue 1). Any subsequent change to searchResults.data re-fires the detection effect (line 181). The filter at lines 209-218 unconditionally returns true for IDs found in manualHighlightTransactionIDs, bypassing the highlightedIDs deduplication:

App/src/hooks/useSearchHighlightAndScroll.ts

Lines 209 to 218 in d58572f

	const newTransactionIDs = currentTransactionIDs.filter((id) => {
	if (manualHighlightTransactionIDs.has(id)) {
	return true;
	}
	if (!triggeredByHookRef.current || !hasNewItemsRef.current) {
	return false;
	}
	return !previousTransactionIDs.includes(id) && !highlightedIDs.current.has(id);
	});

This causes newSearchResultKeys to be set again, the highlight animation replays, and the same cleanup race repeats indefinitely.

What changes do you think we should make in order to solve the problem?

Fix 1: Add deduplication for manual highlights

In the detection effect, check highlightedIDs even for manually highlighted transactions so each ID is only highlighted once:

// Before
const newTransactionIDs = currentTransactionIDs.filter((id) => {
    if (manualHighlightTransactionIDs.has(id)) {
        return true;
    }
    // ...
});

// After
const newTransactionIDs = currentTransactionIDs.filter((id) => {
    if (manualHighlightTransactionIDs.has(id) && !highlightedIDs.current.has(id)) {
        return true;
    }
    // ...
});

App/src/hooks/useSearchHighlightAndScroll.ts

Lines 210 to 212 in d58572f

	if (manualHighlightTransactionIDs.has(id)) {
	return true;
	}

Fix 2: Fix the race condition by clearing Onyx synchronously before resetting state

Merge the two cleanup effects into one so that transactionIDsToHighlight is cleared in Onyx before newSearchResultKeys is set to null, eliminating the race:

useEffect(() => {
    if (newSearchResultKeys === null) {
        return;
    }

    const highlightedTransactionIDs = Object.keys(transactionIDsToHighlight ?? {}).filter(
        (id) => transactionIDsToHighlight?.[id] && newSearchResultKeys.has(`${ONYXKEYS.COLLECTION.TRANSACTION}${id}`),
    );

    let timer: NodeJS.Timeout;
    const animation = requestAnimationFrame(() => {
        timer = setTimeout(() => {
            if (highlightedTransactionIDs.length > 0) {
                mergeTransactionIdsHighlightOnSearchRoute(
                    queryJSON.type,
                    Object.fromEntries(highlightedTransactionIDs.map((id) => [id, false])),
                );
            }
            setNewSearchResultKeys(null);
        }, CONST.ANIMATED_HIGHLIGHT_START_DURATION);
    });

    return () => {
        clearTimeout(timer);
        cancelAnimationFrame(animation);
    };
}, [transactionIDsToHighlight, queryJSON.type, newSearchResultKeys]);

App/src/hooks/useSearchHighlightAndScroll.ts

Lines 234 to 279 in d58572f

	useEffect(() => {
	if (isEmptyObject(transactionIDsToHighlight) || newSearchResultKeys === null) {
	return;
	}
	const highlightedTransactionIDs = Object.keys(transactionIDsToHighlight).filter(
	(id) => transactionIDsToHighlight[id] && newSearchResultKeys?.has(`${ONYXKEYS.COLLECTION.TRANSACTION}${id}`),
	);
	// We need to use requestAnimationFrame here to ensure that setTimeout actually starts
	// only after the user has navigated to the "Reports > Expenses" page.
	// Otherwise, there is still a chance we might miss the timing because setTimeout runs too early,
	// causing the highlight not to appear.
	let timer: NodeJS.Timeout;
	const animation = requestAnimationFrame(() => {
	timer = setTimeout(() => {
	mergeTransactionIdsHighlightOnSearchRoute(queryJSON.type, Object.fromEntries(highlightedTransactionIDs.map((id) => [id, false])));
	}, CONST.ANIMATED_HIGHLIGHT_START_DURATION);
	});
	return () => {
	clearTimeout(timer);
	cancelAnimationFrame(animation);
	};
	}, [transactionIDsToHighlight, queryJSON.type, newSearchResultKeys]);
	// Remove transactionIDsToHighlight when the user leaves the current search type
	useEffect(
	() => () => {
	mergeTransactionIdsHighlightOnSearchRoute(queryJSON.type, null);
	},
	[queryJSON.type],
	);
	// Reset newSearchResultKey after it's been used
	useEffect(() => {
	if (newSearchResultKeys === null) {
	return;
	}
	const timer = setTimeout(() => {
	setNewSearchResultKeys(null);
	}, CONST.ANIMATED_HIGHLIGHT_START_DURATION);
	return () => clearTimeout(timer);
	}, [newSearchResultKeys]);

This ensures the Onyx clear and the state reset happen atomically within the same setTimeout callback, preventing the race condition.