Skip to content

Perf: Prevent multiple openReport calls #80125

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
LukasMod wants to merge 2 commits into from
Closed

Perf: Prevent multiple openReport calls #80125

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
acbf2dd
prevent openReport to be called multiple times
LukasMod Jan 21, 2026
5233193
fix naming
LukasMod Jan 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions src/pages/Search/SearchMoneyRequestReportPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,9 @@ function SearchMoneyRequestReportPage({route}: SearchMoneyRequestPageProps) {
// Prevents creating duplicate transaction threads for legacy transactions
const hasCreatedLegacyThreadRef = useRef(false);

// Prevents from multiple openReport calls
const lastReportIDRef = useRef<string | undefined>(undefined);

// Get transaction from search snapshot if not available in main collections
const {snapshotTransaction, snapshotViolations} = useMemo(() => {
if (!snapshot?.data || Object.keys(allReportTransactions).length > 0) {
Expand All @@ -159,20 +162,26 @@ function SearchMoneyRequestReportPage({route}: SearchMoneyRequestPageProps) {

// If there is more than one transaction, display the report in Super Wide RHP, otherwise it will be shown in Wide RHP
const shouldShowSuperWideRHP = visibleTransactions.length > 1;
const isTransactionThreadMissing = transactionThreadReportID === CONST.FAKE_REPORT_ID;

useShowSuperWideRHPVersion(shouldShowSuperWideRHP);

@github-actions github-actions Bot Jan 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ PERF-6 (docs)

The dependency array [reportIDFromRoute, isTransactionThreadFake] may not capture all necessary state changes. If isTransactionThreadFake transitions from true to false after the transaction thread is created, the effect won't re-run to call openReport because the ref has already been set.

Issue: The ref-based deduplication assumes a one-time check per reportIDFromRoute, but doesn't account for the async nature of transaction thread creation which changes transactionThreadReportID from CONST.FAKE_REPORT_ID to a real ID.

Suggested fix:
Consider resetting the ref when isTransactionThreadFake changes, or restructure the logic to handle the state transition.\n\n---\n\nPlease rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.


useEffect(() => {
if (transactionThreadReportID === CONST.FAKE_REPORT_ID && oneTransactionID) {
if (lastReportIDRef.current === reportIDFromRoute) {
return;
}

Comment on lines 168 to +172

@chatgpt-codex-connector chatgpt-codex-connector Bot Jan 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Allow openReport after thread creation

When transactionThreadReportID starts as FAKE_REPORT_ID, this effect creates the transaction thread and sets lastReportIDRef. Once the real child report ID arrives, the effect re‑runs (dependency change) but now exits immediately because lastReportIDRef.current === reportIDFromRoute, so openReport(reportIDFromRoute) never executes for the parent report. That means the parent report’s actions/metadata are never fetched on first open in the “create thread” path, leaving the view dependent on stale/partial snapshot data. This is the same scenario where ReportScreen explicitly re‑fetches after thread creation; here the new guard prevents that re‑fetch.

Useful? React with 👍 / 👎.

if (isTransactionThreadMissing && !!oneTransactionID) {
const iouAction = getIOUActionForTransactionID(reportActions, oneTransactionID);

@github-actions github-actions Bot Jan 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ CONSISTENCY-5 (docs)

The ESLint disable comment "We don't want this hook to re-run on the every report change" doesn't accurately reflect the current logic. With the ref-based deduplication pattern, the hook behavior has changed significantly.

Suggested fix:
Update the comment to explain the actual deduplication strategy:

// Prevent duplicate openReport calls using ref-based tracking.
// Only reportIDFromRoute and isTransactionThreadFake changes should trigger this effect.
// eslint-disable-next-line react-hooks/exhaustive-deps
```\n\n---\n\nPlease rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.

createTransactionThreadReport(report, iouAction);
lastReportIDRef.current = reportIDFromRoute;
return;
}

lastReportIDRef.current = reportIDFromRoute;
openReport(reportIDFromRoute, '', [], undefined, undefined, false, [], undefined);
// We don't want this hook to re-run on the every report change
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [reportIDFromRoute, transactionThreadReportID]);
}, [reportIDFromRoute, isTransactionThreadMissing]);

useEffect(() => {
hasCreatedLegacyThreadRef.current = false;
Expand Down
Loading