Skip to content

fix: disableOffers now suppresses product:price:amount in SSR#47

Merged
iago1501 merged 3 commits into
masterfrom
fix/disable-offers-ssr
Jun 22, 2026
Merged

fix: disableOffers now suppresses product:price:amount in SSR#47
iago1501 merged 3 commits into
masterfrom
fix/disable-offers-ssr

Conversation

@iago1501

@iago1501 iago1501 commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Problem

The disableOffers setting was being ignored during SSR: the product:price:amount meta tag still appeared in the server-rendered HTML even when the flag was set to true.

Root cause: The render-runtime SSR pipeline runs getDataFromTree (react-apollo) in multiple passes before the final renderToString. react-helmet accumulates all meta tags registered across every pass and never discards them — Helmet.rewind() at the end returns the union of all passes.

The settings query was running with ssr: false, so disableOffers was always false (default) during SSR passes. On the pass where the product was already loaded, the price meta tag was registered in the Helmet state. Subsequent passes with the correct disableOffers = true could not remove it.

This was not a problem for vtex.structured-data because it emits a <script> directly in the React tree — only the final renderToString output matters, intermediate getDataFromTree passes are discarded.

Fix

Two changes:

  1. useAppSettings.ts: changed ssr: falsessr: true and exposed loading from the query.
  2. ProductOpenGraph.tsx: gate the entire component on settingsLoading — return null while settings are in flight. This prevents any Helmet registration before disableOffers is known.

With ssr: true, getDataFromTree executes the settings query before the final render. On the first pass (settings loading), the component returns null — nothing is added to Helmet. On the resolved pass, disableOffers has the correct value and the price tag is either included or omitted accordingly.

How to test it:

Workspace

image

Default behavior is still working in this workspace with the changes:

image

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@iago1501 iago1501 requested a review from a team as a code owner June 15, 2026 20:31
@iago1501 iago1501 requested review from gabpaladino, leo-prange-vtex and vmourac-vtex and removed request for a team June 15, 2026 20:31
@vtex-io-ci-cd

vtex-io-ci-cd Bot commented Jun 15, 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 Jun 15, 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 🎉🎉

@iago1501 iago1501 self-assigned this Jun 15, 2026
@iago1501 iago1501 added the bug Something isn't working label Jun 15, 2026
@iago1501 iago1501 requested review from monteirogc and vsseixaso and removed request for gabpaladino and leo-prange-vtex June 16, 2026 18:30
@vsseixaso vsseixaso requested a review from Copilot June 22, 2026 14:46

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Fixes an SSR-only bug where disableOffers was ignored and product:price:amount could still appear in the final server-rendered HTML due to multi-pass getDataFromTree + react-helmet accumulation.

Changes:

  • Run the app settings query during SSR (ssr: true) and expose the query loading state from useAppSettings.
  • Gate ProductOpenGraph rendering while settings are loading to prevent early Helmet registration.
  • Document the fix in the changelog (plus a minor GraphQL formatting change).

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.

File Description
react/hooks/useAppSettings.ts Enables SSR execution of the settings query and returns loading to callers.
react/ProductOpenGraph.tsx Avoids rendering Helmet tags until settings are available, preventing stale SSR meta tags.
react/queries/getSettings.graphql Minor formatting change.
CHANGELOG.md Adds an Unreleased entry describing the SSR suppression fix.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread react/ProductOpenGraph.tsx Outdated
Comment thread react/hooks/useAppSettings.ts
@vmourac-vtex

Copy link
Copy Markdown
Contributor

🔴 This branch breaks all 6 existing unit tests (IO app test is red)

The changes cause every test in react/__tests__/ProductOpenGraph.tsx to fail. I reproduced it locally head-to-head — same installed node_modules, with only the source diff differing between the two runs:

Ref Commit Result
master (this PR's base) 6af9570 6 passed / 6
this branch 2495c312 6 failed / 6Unable to find an element by: [data-testid="…"]

So this is a regression introduced by the PR, not pre-existing breakage.

Root cause. ProductOpenGraph now early-returns while settings are loading:

if (settingsLoading || !productContext?.product) {
  return null
}

The existing tests render the component with only a ProductContext.Provider and no Apollo mock for the getSettings query. Now that useAppSettings exposes loading from useQuery, settingsLoading stays true for the whole test, the component renders null, and every getByTestId(...) assertion fails.

This is specifically the gate — not a missing Apollo client. The harness does supply Apollo via MockedProvider; on master useQuery resolves to loading with no data and the component still renders fine. It only becomes fatal once the component gates rendering on settingsLoading.

Suggested resolution (pick one):

  1. If ssr: true alone suppresses the tag, drop the settingsLoading gate — that restores the tests with no further change. Worth confirming on a freshly cache-busted / new workspace first, to rule out render-server SmartCache lag (cached SSR HTML can serve stale settings for up to ~30 min) as the reason an ssr: true-only build appeared to still leak.
  2. If the gate is genuinely needed, update the suite to wrap renders in a MockedProvider that resolves getSettings, and add a regression test asserting product:price:amount is omitted when disableOffers: true and present when false — that is the behavior this PR fixes and it is currently untested.

Either way, the IO app test job needs to be green before merge.

iago1501 and others added 2 commits June 22, 2026 15:06
- Gate settingsLoading only during SSR (!canUseDOM) to avoid blocking
  client renders and breaking tests that lack an Apollo settings mock
- Export canUseDOM=true from the render-runtime mock so the guard
  is correctly bypassed in the test environment
- Wrap JSON.parse in try/catch in useAppSettings to prevent a malformed
  settings payload from crashing SSR with a 500
- Add regression tests: disableOffers=true omits product:price:amount,
  disableOffers=false keeps it

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Fixes prettier lint error in the disableOffers regression test.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@iago1501 iago1501 merged commit 721b66b into master Jun 22, 2026
8 checks passed
@iago1501 iago1501 deleted the fix/disable-offers-ssr branch June 22, 2026 19:27
@vtex-io-ci-cd

vtex-io-ci-cd Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Your PR has been merged! App is being published. 🚀
Version 1.4.1 → 1.4.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.open-graph@1.4.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

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants