Skip to content

fix: keep the ads wait window tied to the search loading state - #779

Merged
amandascm merged 3 commits into
masterfrom
fix/ads-wait-window
Aug 11, 2026
Merged

fix: keep the ads wait window tied to the search loading state#779
amandascm merged 3 commits into
masterfrom
fix/ads-wait-window

Conversation

@amandascm

@amandascm amandascm commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

What problem is this solving?

Since v3.143.0 the PLP flickers: the result loads partially, the loader shows up again and only then the page renders. Reported on a store with no active campaigns (so ads always come back empty), where rolling back to v3.141.2 makes it go away (reported by telhanorte, but noticed in other stores such as drogal).

  • Screenshots in this workspace, but check also in Drogal or Telha Norte production stores
    • First load
      image
    • Shows some result
      image
    • Second load
      image

The wait for sponsored products introduced in v3.143.0 was armed from the ads loading state alone:

useEffect(() => {
  if (!isFirstPage) { setHoldOrganic(false); return }
  if (!isAdsLoading) { setHoldOrganic(false); return }
  setHoldOrganic(true) // ← only depends on ads
  ...
}, [isAdsLoading, isFirstPage])

Nothing tied it to the organic search state, and isAdsLoading goes back to true after the first paint, because useAds refires its request whenever term, selectedFacets or the ads identity change (@vtex/ads-react deps). On this store selectedFacets changes when facetsFromSession resolves asynchronously (SearchQuery.js:352). So: search resolves → gallery renders → ads become pending again → loading: true + products: []loader is back → ads return empty → gallery renders again.

The intended behavior, per Ads Frictionless Integration on Store Framework, is kept: the organic result does wait for ads (bounded by a timeout) so sponsored and organic products land in the same paint.

What changed

useMergeResults now arms the wait only while the organic search is itself loading, so waiting can extend a loader that is already on screen but can never turn a rendered result back into one. It is released on the first of:

  • the ads request settling (with or without ads) and the search having resolved;
  • ADS_MAX_WAIT_MS (2s).

Ads pending while the search has already resolved keeps the loader — that is the intended wait. Ads pending after the result is rendered does nothing, structurally.

Also guarded searchQuery.data before rewriting data.productSearch: data is undefined while Apollo is loading, which is exactly the state the wait now holds on purpose.

How to test it?

Search for coral (or any term) on this workspace without active campaigns - Workspace: the loader must appear once and stay until the ads request settles, then render the result. No second loader, no flicker.

Also worth checking: facet navigation, pagination (the wait is first-page only), and a store with active sponsored campaigns, such as eletroangeloni in the new ads workspace (keeps working and showing ads for term sanduicheira).

yarn test: 16 suites, 96 tests passing. react/__tests__/useMergeResults.test.js covers both directions — the wait happens, the 2s cap releases, ads becoming pending after the render never brings the loader back, and a new search re-arms the wait.

Describe alternatives you've considered, if any.

Dropping the wait altogether (pure merge hook) removes the flicker but reintroduces the layout shift of sponsored products popping into a rendered gallery — rejected, since waiting is the behavior the RFC asks for.

Related to / Depends on

Fixes the regression introduced in v3.143.0 (refactor: timeout to wait ads before rendering). Affects every version in the 3.143.x line.

One trade-off worth reviewing: stores with no campaigns still wait for the (empty) ads response. ADS_MAX_WAIT_MS bounds only the artificial hold this hook applies — it never cancels, truncates or short-circuits the search request. When the limit expires with the search still loading, the hold is dropped and searchQuery.loading goes back to being whatever Apollo reports, so the loader stays until the search actually responds. The limit is also counted from the moment the search starts, not from when it resolves: a search that takes 1.5s leaves ads only 500ms of wait. The RFC covers this trade-off — simultaneous requests, low ads response time, and a timeout so ads don't delay the final result — but note that the 2s here is a UI-side ceiling, not a timeout on the ads request itself. A request-level timeout would have to come from @vtex/ads-react/ads-core, which does not expose one today.

How does this PR make you feel? 🔗

@amandascm
amandascm requested review from a team as code owners August 5, 2026 19:01
@amandascm
amandascm requested review from RodrigoTadeuF, gabpaladino and vsseixaso and removed request for a team August 5, 2026 19:01
@vtex-io-ci-cd

vtex-io-ci-cd Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Hi! I'm VTEX IO CI/CD Bot and I'll be helping you to publish your app! 🤖

Please select which version do you want to release:

  • Patch (backwards-compatible bug fixes)

  • Minor (backwards-compatible functionality)

  • Major (incompatible API changes)

And then you just need to merge your PR when you are ready! There is no need to create a release commit/tag.

  • No thanks, I would rather do it manually 😞

@vtex-io-docs-bot

vtex-io-docs-bot Bot commented Aug 5, 2026

Copy link
Copy Markdown

Beep boop 🤖

I noticed you didn't make any changes at the docs/ folder

  • There's nothing new to document 🤔
  • I'll do it later 😞

In order to keep track, I'll create an issue if you decide now is not a good time

  • I just updated 🎉🎉

@amandascm
amandascm force-pushed the fix/ads-wait-window branch from e35b99c to e812ed0 Compare August 5, 2026 19:39
@amandascm
amandascm requested a review from hiagolcm August 5, 2026 19:47
@amandascm
amandascm force-pushed the fix/ads-wait-window branch from e812ed0 to e3e5b0b Compare August 5, 2026 20:01

@hiagolcm hiagolcm left a comment

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.

LGTM

amandascm and others added 3 commits August 10, 2026 14:45
The wait for sponsored products was armed from the ads loading state alone,
so a refired ads request (term, selected facets or identity change) sent an
already rendered PLP back to the loading state, making the page flicker.

The wait is now armed only while the organic search is loading, and released
when the ads request settles or after ADS_MAX_WAIT_MS.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@amandascm
amandascm force-pushed the fix/ads-wait-window branch from e3e5b0b to 50efcf9 Compare August 10, 2026 18:14
@amandascm
amandascm merged commit e046edf into master Aug 11, 2026
7 checks passed
@amandascm
amandascm deleted the fix/ads-wait-window branch August 11, 2026 12:08
@vtex-io-ci-cd

vtex-io-ci-cd Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Your PR has been merged! App is being published. 🚀
Version 3.150.1 → 3.150.2

After the publishing process has been completed (check #vtex-io-releases) and doing A/B tests with the new version, you can deploy your release by running:

vtex deploy vtex.search-result@3.150.2

After that your app will be updated on all accounts.

For more information on the deployment process check the docs. 📖

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.

2 participants