Skip to content

Fix console error when opening a newly created expense on the Spend page#95670

Merged
NicolasBonet merged 3 commits into
mainfrom
claude-fixSearchRenderPhaseSetStateWarning
Jul 21, 2026
Merged

Fix console error when opening a newly created expense on the Spend page#95670
NicolasBonet merged 3 commits into
mainfrom
claude-fixSearchRenderPhaseSetStateWarning

Conversation

@MelvinBot

@MelvinBot MelvinBot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Explanation of Change

Fixes the dev-only console error Cannot update a component (SearchPage) while rendering a different component (Search) that appears when opening a newly created expense on the Spend page.

Search bails out to an error/empty state via conditional returns during render (searchResults === undefined, hasErrors, and the empty-state branch). Each of those calls cancelNavigationSpans(), which — before this change — also called the parent-owned onContentReady?.() (SearchPage's setIsSearchReady(true) from useSearchOverlay). Updating the parent's state during Search's render is exactly what React's warning flags.

When a just-created expense is opened before its transaction lands in the search snapshot, Search transiently takes the empty-state bail-out, so the render-phase parent update fires and the warning is logged.

The fix moves the onContentReady?.() call out of cancelNavigationSpans and into the existing no-dependency post-commit useEffect (guarded by the didBailToFallbackState ref that the bail-out paths already set). This defers the parent update to the commit phase instead of during render, matching the flush pattern already used in the same file. setIsSearchReady(true) is idempotent, so firing it on every bail-out render is safe and does not loop. Behavior is otherwise unchanged.

Fixed Issues

$ #94318
PROPOSAL: #94318 (comment)

Tests

  1. Log in to a new account
  2. Complete the onboarding flow
  3. Go to the Spend tab
  4. Verify that the Cannot update a component (SearchPage) while rendering a different component (Search) console error does not appear
  • Verify that no errors appear in the JS console

Offline tests

Same as Tests.

QA Steps

// TODO: The human co-author must fill out the QA tests you ran before marking this PR as "ready for review".
// Please describe what QA needs to do to validate your changes and what areas do they need to test for regressions.

  • Verify that no errors appear in the JS console

PR Author Checklist

  • I linked the correct issue in the ### Fixed Issues section above
  • I wrote clear testing steps that cover the changes made in this PR
    • I added steps for local testing in the Tests section
    • I added steps for the expected offline behavior in the Offline steps section
    • I added steps for Staging and/or Production testing in the QA steps section
    • I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
    • I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
    • I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).
  • I included screenshots or videos for tests on all platforms
  • I ran the tests on all platforms & verified they passed on:
    • Android: Native
    • Android: mWeb Chrome
    • iOS: Native
    • iOS: mWeb Safari
    • MacOS: Chrome / Safari
  • I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
  • I followed proper code patterns (see Reviewing the code)
    • I verified that comments were added to code that is not self explanatory
    • I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
    • I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
  • If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
  • I followed the guidelines as stated in the Review Guidelines
  • I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
  • If a new CSS style is added I verified that:
    • A similar style doesn't already exist
    • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))
  • If new assets were added or existing ones were modified, I verified that:
    • The assets are optimized and compressed (for SVG files, run npm run compress-svg)
    • The assets load correctly across all supported platforms.
  • If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
  • If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
  • If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
  • If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
  • If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
    • I verified that all the inputs inside a form are aligned with each other.
    • I added Design label and/or tagged @Expensify/design so the design team can review the changes.
  • I added unit tests for any new feature or bug fix in this PR to help automatically prevent regressions in this user flow.
  • If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.

Co-authored-by: DylanDylann <DylanDylann@users.noreply.github.com>
@MelvinBot
MelvinBot requested a review from a team July 9, 2026 09:25
@MelvinBot

Copy link
Copy Markdown
Contributor Author

I created this PR from an upstream branch since I don't have push access to your fork.

To take ownership of this branch and be able to push updates, run:

git remote add upstream https://github.com/Expensify/App.git
git fetch upstream claude-fixSearchRenderPhaseSetStateWarning
git checkout -b claude-fixSearchRenderPhaseSetStateWarning upstream/claude-fixSearchRenderPhaseSetStateWarning
git push -u origin claude-fixSearchRenderPhaseSetStateWarning

Then you can close this PR and open a new one from your fork.

@codecov

codecov Bot commented Jul 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ Changes either increased or maintained existing code coverage, great job!

Files with missing lines Coverage Δ
src/components/Search/index.tsx 45.32% <75.00%> (+0.02%) ⬆️
... and 8 files with indirect coverage changes

@MelvinBot

Copy link
Copy Markdown
Contributor Author

DylanDylann — no code fix is needed. The only genuinely failing check is PR Author Checklist / checklist, and it's failing for a non-code reason: the PR body's author-checklist boxes are all unchecked and the Tests / QA Steps sections still contain the // TODO placeholders left for the human co-author.

Per MelvinBot policy I can't tick the author checklist or fill out the human Tests/QA sections on your behalf — so this check will stay red until you complete it. That's the "fix": edit the PR description to fill in the Tests/QA steps and check the author-checklist items.

Every code-relevant check is green. I verified the two most likely to catch this change locally against the changed file (src/components/Search/index.tsx):

  • ESLint: PASS (0 errors). The useEffect that now calls onContentReady?.() intentionally has no dependency array, so react-hooks/exhaustive-deps doesn't apply, and the cancelNavigationSpans useCallback correctly dropped onContentReady from its deps since it no longer references it.
  • typecheck: PASS (0 errors project-wide). onContentReady is an existing optional prop already invoked elsewhere in the file, so the moved call is type-safe.

All other CI jobs are passing. When I finished, ESLint check and build (Debug, true) were still queued/running (not failed) — both are expected to pass, as this is a one-line render-phase refactor with no native or lint impact.

The failing check log
You completed 0 out of 38 checklist items with 38 unfinished items
##[error]PR Author Checklist is not completely filled out. Please check every box to verify you've thought about the item.

@DylanDylann

Copy link
Copy Markdown
Contributor

@MelvinBot Ready to review

@MelvinBot

Copy link
Copy Markdown
Contributor Author

Review

Verdict: The code change is correct, minimal, and the root-cause reasoning holds up — the render-phase → commit-phase move is the right fix for this warning. No functional regression. Two non-code items still block merge (see below); I can't complete them myself.

Why the fix is correct

The warning fires because Search updates its parent (SearchPage) during its own render. The bail-out paths — searchResults === undefined, hasErrors, and the empty-state branch — all call cancelNavigationSpans() inside conditional returns (render phase). Moving onContentReady?.() out of that callback and into the post-commit dep-free effect defers the parent update to the commit phase, which is exactly what React wants.

I verified the supporting claims:

  • Idempotency: onContentReady resolves to onSearchContentReadysetIsSearchReady(true). Setting the same true value bails out of re-render, so firing it on every bail-out render is safe and won't loop. ✅
  • Behavior equivalence: before this PR, onContentReady was already called unconditionally on every bail-out (just synchronously, in render). The dep-free effect runs after every commit and the didBailToFallbackState ref is set during the same bail-out render, so it still fires once per bail-out — just one commit later. ✅
  • Dependency change: dropping onContentReady from the cancelNavigationSpans useCallback deps is correct — it no longer references it. react-hooks/exhaustive-deps doesn't flag the effect since it intentionally has no dep array (pre-existing pattern in this file). ✅
  • Skeleton/deferred bail-outs (isDeferringHeavyWork, shouldShowRowSkeleton) don't call cancelNavigationSpans, so they don't trigger onContentReady — unchanged from before, and correct since those paths flush via their own onLayout. ✅

Blockers before this can merge

  1. PR Author Checklist is failing because the checklist boxes are all unchecked and the Tests / QA Steps sections still contain the // TODO placeholders. Per policy I can't tick the author checklist or fill the human Tests/QA sections — a human co-author must do this. Since the repro is a dev-only console error, the natural QA step is the issue's own flow: create a workspace expense → immediately open it on the Spend page → confirm no Cannot update a component (SearchPage) while rendering a different component (Search) error in the console.

  2. No regression test. This is genuinely hard to unit-test (it's a dev-mode console.error), so I'd treat it as optional rather than a hard blocker — but if feasible, a test that renders Search in the transient empty-state and asserts console.error was not called with the render-phase warning would guard against re-introduction. Worth a note in the checklist either way.

Every code-relevant check is green (ESLint + typecheck verified locally earlier). Once the checklist/Tests/QA are filled in, this looks good to go from a code standpoint.

@DylanDylann

Copy link
Copy Markdown
Contributor

@MelvinBot Mark the PR ready

@MelvinBot
MelvinBot marked this pull request as ready for review July 9, 2026 09:49
@MelvinBot
MelvinBot requested a review from a team as a code owner July 9, 2026 09:49
@melvin-bot
melvin-bot Bot requested review from DylanDylann and heyjennahay and removed request for a team July 9, 2026 09:49
@melvin-bot

melvin-bot Bot commented Jul 9, 2026

Copy link
Copy Markdown

@DylanDylann Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

@melvin-bot
melvin-bot Bot removed the request for review from a team July 9, 2026 09:50
@MelvinBot

Copy link
Copy Markdown
Contributor Author

Done — PR #95670 is now marked Ready for review. ✅

One heads-up: the PR Author Checklist / checklist check will stay red until the author-checklist boxes are checked and the Tests / QA Steps sections (still // TODO placeholders) are filled in. Per policy I can't tick the checklist or complete the human Tests/QA sections on your behalf, so that last step is on the human co-author before this can merge.

@MelvinBot

Copy link
Copy Markdown
Contributor Author

The failing ESLint check is not related to this PR — it's a pre-existing eslint-seatbelt baseline drift on main, so I have not pushed a change to this branch.

Why it's unrelated: This PR only touches src/components/Search/index.tsx, which lints clean. All 31 reported errors are in two files this PR never touches:

  • src/libs/actions/Report/index.tsno-restricted-syntax (namespace imports): 10 errors, baseline allows 9
  • tests/actions/IOUTest/SplitTest.ts@typescript-eslint/no-unsafe-type-assertion: 21 errors, baseline allows 20

Both files have the exact same violation counts on main (origin/main = 7f29834) as on this branch, and the committed baseline in config/eslint/eslint.seatbelt.tsv is byte-identical between the two. In other words, main itself is currently over its lint baseline — a PR merged an extra namespace import to Report/index.ts and an extra unsafe assertion to SplitTest.ts without regenerating the seatbelt baseline, and CI enforces it repo-wide, so every open PR is failing this check, not just this one.

Merging latest main won't help — main's tip is still drifted.

Recommended fix (belongs on main, not this PR): either resolve the two actual violations, or regenerate the seatbelt baseline (npm run lint locally updates config/eslint/eslint.seatbelt.tsv to the real counts) and merge that. I deliberately did not bundle that unrelated baseline edit into this focused Search bugfix.

Evidence
# Files changed by this PR (vs origin/main)
src/components/Search/index.tsx   ← only real code change
package.json / package-lock.json / *.plist / build.gradle / Mobile-Expensify  ← version bumps from main merge

# origin/main Report/index.ts namespace imports: 10  (baseline in seatbelt.tsv: 9)
# origin/main SplitTest.ts unsafe assertions:    21  (baseline in seatbelt.tsv: 20)
# seatbelt.tsv lines 930 & 1772 identical on branch and origin/main

The other failing check, PR Author Checklist / checklist, is the standard author-checklist gate (needs a human to complete the checklist) — nothing to fix in code.

@DylanDylann

Copy link
Copy Markdown
Contributor

@MelvinBot Got a failed Eslint check

@DylanDylann

Copy link
Copy Markdown
Contributor

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Can't wait for the next one!

Reviewed commit: 0aa1d5923a

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@MelvinBot

Copy link
Copy Markdown
Contributor Author

The failing ESLint check is not caused by this PR — it's a broken-main issue affecting every PR built right now. No action is needed on this branch.

What's failing: the seatbelt rule in src/libs/actions/Report/index.tsno-restricted-syntax reports 10 import * as … from '@libs/…' namespace imports, but the seatbelt baseline only allows 9. Since seatbelt runs frozen in CI, actual(10) > allowed(9) is a hard failure. This PR only touches src/components/Search/index.tsx and never goes near Report/index.ts.

Root cause: a recent revert on main reintroduced the 10th namespace import in Report/index.ts without bumping the seatbelt baseline. main is currently red on ESLint, so the failure is inherited by this branch.

Already being fixed: #95671"Fix ESLint seatbelt budget violations reintroduced by revert." Once that lands on main, merge main into this branch and the ESLint check will go green.

I'm not pushing anything here: the fix belongs on main (via #95671), not smuggled into this Search PR, and editing an unrelated file wouldn't unblock everyone else.

Evidence

@DylanDylann

Copy link
Copy Markdown
Contributor

@MelvinBot Merge latest main

…rPhaseSetStateWarning

Co-authored-by: DylanDylann <DylanDylann@users.noreply.github.com>
@MelvinBot
MelvinBot requested a review from a team as a code owner July 10, 2026 07:12
@MelvinBot

Copy link
Copy Markdown
Contributor Author

🤖 Merged latest main into this branch and pushed the merge commit (33568f8). No conflicts — the merge was clean.

@melvin-bot
melvin-bot Bot requested review from NicolasBonet and removed request for a team July 10, 2026 07:12
@melvin-bot

melvin-bot Bot commented Jul 10, 2026

Copy link
Copy Markdown

@NicolasBonet Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

@DylanDylann

Copy link
Copy Markdown
Contributor

@MelvinBot Please reopen this PR

@MelvinBot MelvinBot reopened this Jul 21, 2026
@MelvinBot

Copy link
Copy Markdown
Contributor Author

🤖 Done — I've reopened this PR. The head branch claude-fixSearchRenderPhaseSetStateWarning still existed, so it was reopened cleanly and is back in the OPEN state.

Note: I couldn't verify anything in the browser in this run (web verification wasn't available), but reopening required no browser interaction.

@DylanDylann

Copy link
Copy Markdown
Contributor

@MelvinBot Please merge the latest main and mark this PR ready

…rPhaseSetStateWarning

Co-authored-by: DylanDylann <DylanDylann@users.noreply.github.com>
@MelvinBot
MelvinBot marked this pull request as ready for review July 21, 2026 03:40
@MelvinBot

Copy link
Copy Markdown
Contributor Author

🤖 Done!

  • Merged latest main into claude-fixSearchRenderPhaseSetStateWarning — no conflicts. Merge commit aeb16299e67 is pushed.
  • Marked the PR ready for review — it's no longer a draft and is currently MERGEABLE.

Note: I couldn't verify anything in a browser this run (web verification wasn't available), but merging and marking ready required no browser interaction. CI checks will run on the new merge commit — please keep an eye on them.

@MelvinBot

Copy link
Copy Markdown
Contributor Author

The failing ESLint check is now passing ✅ — it was stale, not caused by this PR's changes.

What was wrong

The ESLint run that failed (job) reported 6 problems, and all of them were in .github/scripts/createDocsRoutes.ts — a file this PR never touches (the only changed file is src/components/Search/index.tsx):

.github/scripts/createDocsRoutes.ts
  6 problems (6 errors, 0 warnings) — @typescript-eslint/no-unsafe-type-assertion
  [eslint-seatbelt]: There are 6 errors of this type, but only 3 are allowed.

That failure ran on 2026-07-10 against an old snapshot of main. Since then main refactored createDocsRoutes.ts to use type guards (isRecord / getOptionalString / getOptionalNumber) instead of as assertions, and tightened the eslint-seatbelt baseline to match — so on current main that file no longer trips the seatbelt.

What fixed it

The branch was simply behind main. A merge main brought in the refactored file (now only a single as DocsRoutes assertion, well under the seatbelt's allowance) plus the matching seatbelt baseline. ESLint has since re-run on the current head aeb16299e67 and passes (1m24s).

Remaining check

The only check still red is PR Author Checklist / checklist, which is a human-only checklist gate (the QA/testing checkboxes and the QA Steps section still need to be filled out by the human co-author). It is not something a code change can fix.

Note on verification

I couldn't drive the app in a browser in this run, but that isn't needed here — this was a CI/lint issue, and I confirmed the ESLint check passes directly from the PR check status.

@MelvinBot

Copy link
Copy Markdown
Contributor Author

I looked into the checks on this PR. The only failing check is PR Author Checklist / checklist, and it's not a code failure — every other check (tests, ESLint, typecheck, builds, etc.) is passing or pending.

The checklist job failed with:

You completed 0 out of 38 checklist items with 38 unfinished items
PR Author Checklist is not completely filled out. Please check every box to verify you've thought about the item.

This is the standard human author checklist from the PR template. It can't be fixed with a code change — the boxes in the PR description just need to be checked off by the author/assignee. Once every item is ticked, the check will pass on the next run.

No push was needed, so I haven't made any changes.

@DylanDylann

Copy link
Copy Markdown
Contributor

@MelvinBot Please update the test steps

  1. Log in to a new account
  2. Complete the onboarding flow
  3. Going to Spend tab
  4. Verify that the Cannot update a component (SearchPage) while rendering a different component (Search) console error does not appear

@MelvinBot

Copy link
Copy Markdown
Contributor Author

🤖 Done — I've updated the Tests section of the PR body to:

  1. Log in to a new account
  2. Complete the onboarding flow
  3. Go to the Spend tab
  4. Verify that the Cannot update a component (SearchPage) while rendering a different component (Search) console error does not appear

(Offline tests still point to "Same as Tests.") Let me know if you'd like any further tweaks.

@DylanDylann

Copy link
Copy Markdown
Contributor

Reviewer Checklist

  • I have verified the author checklist is complete (all boxes are checked off).
  • I verified the correct issue is linked in the ### Fixed Issues section above
  • I verified testing steps are clear and they cover the changes made in this PR
    • I verified the steps for local testing are in the Tests section
    • I verified the steps for Staging and/or Production testing are in the QA steps section
    • I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
    • I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
  • I checked that screenshots or videos are included for tests on all platforms
  • I included screenshots or videos for tests on all platforms
  • I verified that the composer does not automatically focus or open the keyboard on mobile unless explicitly intended. This includes checking that returning the app from the background does not unexpectedly open the keyboard.
  • I verified tests pass on all platforms & I tested again on:
    • Android: HybridApp
    • Android: mWeb Chrome
    • iOS: HybridApp
    • iOS: mWeb Safari
    • MacOS: Chrome / Safari
  • If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
  • I verified proper code patterns were followed (see Reviewing the code)
    • I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
    • I verified that comments were added to code that is not self explanatory
    • I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
    • I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
  • If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
  • I verified that this PR follows the guidelines as stated in the Review Guidelines
  • I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
  • If a new component is created I verified that:
    • A similar component doesn't exist in the codebase
    • All props are defined accurately and each prop has a /** comment above it */
    • The file is named correctly
    • The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
    • The only data being stored in the state is data necessary for rendering and nothing else
    • For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
    • Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
    • All JSX used for rendering exists in the render method
    • The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
  • If any new file was added I verified that:
    • The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
  • If a new CSS style is added I verified that:
    • A similar style doesn't already exist
    • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
  • If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
  • If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
  • If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
  • If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
  • If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
    • I verified that all the inputs inside a form are aligned with each other.
    • I added Design label and/or tagged @Expensify/design so the design team can review the changes.
  • For any bug fix or new feature in this PR, I verified that sufficient unit tests are included to prevent regressions in this flow.
  • If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
  • I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Android: HybridApp
Screen.Recording.2026-07-21.at.11.07.01.mov
Android: mWeb Chrome
iOS: HybridApp
Screen.Recording.2026-07-21.at.11.04.48.mov
iOS: mWeb Safari
MacOS: Chrome / Safari

@DylanDylann

Copy link
Copy Markdown
Contributor

@NicolasBonet This PR is ready for your review. It was created by Melvin, and I'm unable to update the PR description to satisfy the author checklist

@MelvinBot

Copy link
Copy Markdown
Contributor Author

I checked the current status of all checks on this PR and none of them are failing. 🟢

Every check reports pass (Bun tests, ESLint, Oxfmt, typecheck, test jobs 1–8, Storybook, perf tests, review, spellcheck, etc.), and the remaining entries are either skipping or the PR Author Checklist / checklist, which is currently in_progress and completes on its own — it isn't a failure.

Since there's nothing failing, I haven't pushed any changes. If a check was red earlier, it may have been a transient/flaky run that has since re-run green. If you're still seeing a specific red check, let me know which one (or link the failing job) and I'll dig into it.

@DylanDylann

Copy link
Copy Markdown
Contributor

@codex review

@MelvinBot

Copy link
Copy Markdown
Contributor Author

Re-checked — all checks on this PR are green. ✅

Every check now reports pass (or skipping), including the PR Author Checklist that was still in_progress during my previous check. There are no failing checks, so there's nothing for me to fix or push.

If you're still seeing a red check on your end, point me at the specific failing job (link the run) and I'll investigate that one directly.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep them coming!

Reviewed commit: aeb16299e6

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@NicolasBonet
NicolasBonet merged commit 53efbbd into main Jul 21, 2026
47 of 91 checks passed
@NicolasBonet
NicolasBonet deleted the claude-fixSearchRenderPhaseSetStateWarning branch July 21, 2026 14:24
@DylanDylann

Copy link
Copy Markdown
Contributor

@NicolasBonet All yours

@github-actions

Copy link
Copy Markdown
Contributor

🚧 NicolasBonet has triggered a test Expensify/App build. You can view the workflow run here.

@OSBotify

Copy link
Copy Markdown
Contributor

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

@github-actions

Copy link
Copy Markdown
Contributor

🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
Built from App PR #95670.

Android 🤖 iOS 🍎
https://ad-hoc-expensify-cash.s3.us-east-1.amazonaws.com/rock-artifacts/ad-hoc/rock-android-Adhoc-53efbbdbc2a-a8a121b-0340623b65751865c4e7a353576f49a17c770397/index.html ❌ FAILED ❌
Android The QR code can't be generated, because the iOS build failed
Web 🕸️
https://95670.pr-testing.expensify.com
Web

👀 View the workflow run that generated this build 👀

@OSBotify

Copy link
Copy Markdown
Contributor

🚀 Deployed to staging by https://github.com/NicolasBonet in version: 9.4.42-0 🚀

platform result
🕸 web 🕸 success ✅
🤖 android 🤖 success ✅
🍎 iOS 🍎 success ✅

@MelvinBot

Copy link
Copy Markdown
Contributor Author

🤖 No help site changes required.

This PR is a dev-only fix scoped entirely to internal React rendering. It moves a parent-state update (onContentReady?.()) out of Search's render phase and into a post-commit useEffect to silence the console warning Cannot update a component (SearchPage) while rendering a different component (Search). Per the PR description, behavior is otherwise unchanged.

There's no user-facing feature, UI label, setting, tab, or workflow change here — nothing that any article under docs/articles documents. As a result, no draft docs PR was created.

What I reviewed
  • Files changed: only src/components/Search/index.tsx (+12/-3).
  • Nature of change: defers onContentReady?.() (which calls setIsSearchReady(true) in the parent) from the render-phase cancelNavigationSpans to the existing no-dependency post-commit effect, guarded by the didBailToFallbackState ref.
  • User impact: none — this only removes a development console warning; the Spend/Search page renders and behaves identically for end users.

DylanDylann, since this change has no customer-facing impact, there's nothing to reflect on the help site. If you believe a behavior change here should be documented, let me know and I'll take another look.

@OSBotify

Copy link
Copy Markdown
Contributor

🚀 Deployed to production by https://github.com/yuwenmemon in version: 9.4.42-1 🚀

platform result
🕸 web 🕸 success ✅
🤖 android 🤖 success ✅
🍎 iOS 🍎 success ✅

Bundle Size Analysis (Sentry):

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants