chore(test): combine VTE-14 PLP performance + VTE-18 partytown migration#3345
Closed
renatomaurovtex wants to merge 19 commits into
Closed
chore(test): combine VTE-14 PLP performance + VTE-18 partytown migration#3345renatomaurovtex wants to merge 19 commits into
renatomaurovtex wants to merge 19 commits into
Conversation
- Preload the hero image (when present) or the first product image in <head> with rel=preload + fetchPriority=high to address the ~5.5s LCP. The href is run through faststoreLoader so it matches the optimized URL Next/Image renders (avoids a double-download cache miss). - Respect the Rules of Hooks in useShouldFetchFirstPage: move both useRef calls before the early return so they are always called. - Replace generatedBuildTime (build-time constant, always >5 min in prod) with a per-mount Date.now() ref so the refetch window is measured from when the user actually loaded the page, not from the build. - Remove React.lazy() from ProductGalleryPage import so the above-fold gallery is included in the initial bundle instead of a deferred chunk. - Switch useScreenResize from useEffect to an isomorphic useLayoutEffect so window.innerWidth is read before the browser paints, eliminating the 24ms forced reflow observed in the PSI trace. - Eager-load the first 4 product images (was: only the first) to match the typical above-fold count on mobile. Co-Authored-By: Paperclip <noreply@paperclip.ing>
Section.data is typed as `unknown` in @vtex/client-cms. The inline
type annotation on the find() callback parameter doesn't affect the
inferred return type, so heroSection.data was `unknown` and TS rejected
the .image?.src access.
Extract heroData with an explicit cast to `{ image?: { src?: string } }
| undefined` so the access is type-safe without suppressing the error.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
…E-14)
The <link rel="preload" fetchPriority="high"> in <Head> tells the browser
to start fetching early, but Lighthouse's "LCP request discovery" check
looks for fetchpriority="high" on the <img> element itself.
Pass priority={true} (via imgProps.priority) for the first product on the
first page so that Next.js adds fetchpriority="high" directly to the <img>
and also emits its own <link rel="preload"> — fully satisfying the PSI
diagnostic without any additional fetch overhead.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
The FastStore Image component already sets priority={loading === 'eager'}
internally (Image.tsx:16). Since ProductGrid already passes
loading='eager' for idx < 4, the LCP image already gets priority
treatment without an explicit prop.
The explicit priority={imgProps?.priority} added in the previous commit
was redundant and triggered a SonarQube deprecation notice (Next.js 16
deprecated the priority prop). Remove it — and remove the corresponding
imgProps.priority in ProductGrid — to silence the warning without
changing any runtime behaviour.
Co-Authored-By: Paperclip <noreply@paperclip.ing>
PSI confirmed loading='eager' alone does not inject fetchpriority="high" on the <img> element in Next.js 16. Pass fetchPriority='high' explicitly via imgProps for the first product on the first page so the browser prioritises the LCP image fetch without using the deprecated priority prop. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… (VTE-14) ProductCard.tsx only forwarded specific imgProps fields (sizes, width, height, loading) to the Image component, silently dropping fetchPriority. Add the forwarding so the fetchPriority='high' set by ProductGrid for the first product on the first page actually reaches the rendered <img> element. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…zedFonts Introduces `experimental.optimizedFonts` flag in discovery.config. When enabled, FastStore self-hosts the Inter font via next/font/google, eliminating the render-blocking request to fonts.googleapis.com (~660ms FCP improvement on mobile). - Defaults to false — zero behavior change for existing stores - Uses require() gating so next/font/google is tree-shaken when flag is off - Restricts subsets to latin + latin-ext (down from 7) - Applies --font-inter CSS variable to <body> only when flag is on Co-authored-by: Cursor <cursoragent@cursor.com>
Webpack statically follows require('src/fonts/inter') in _document.tsx
into the bundle graph regardless of the runtime flag, causing Babel to
compile inter.ts (which imports next/font/google). Since next/font/google
is SWC-only, this produced a hard build error for stores that have a
custom .babelrc.js:
Syntax error: "next/font" requires SWC although Babel is being used
Fix: add a webpack resolve.alias in next.config.js that redirects
src/fonts/inter to a null stub (inter.stub.ts) whenever optimizedFonts
is false (the default). This way inter.ts is never added to the webpack
module graph for stores that haven't opted in, and the build succeeds
with Babel unchanged.
Co-authored-by: Cursor <cursoragent@cursor.com>
…ation
resolve.alias failed because Next.js resolves tsconfig paths ("src/*")
to absolute paths before webpack's alias lookup runs, so the key
'src/fonts/inter' never matched.
NormalModuleReplacementPlugin fires against the raw request string
before any path resolution, so it correctly intercepts the require()
in _document.tsx and redirects it to the null stub when
optimizedFonts is false.
Co-authored-by: Cursor <cursoragent@cursor.com>
Previous approach tried to redirect webpack away from inter.ts (which imports next/font/google) using NormalModuleReplacementPlugin. This did not work reliably because webpack follows require() statically before the plugin can intercept. New approach inverts the responsibility: - src/fonts/inter.ts → Babel-safe stub (exports null by default) - src/fonts/inter.optimized.ts → real next/font/google module Since inter.ts is now the default module webpack compiles, stores that use Babel never encounter the "next/font requires SWC" error. inter.optimized.ts is only pulled into the module graph when experimental.optimizedFonts === true via NormalModuleReplacementPlugin, at which point SWC is required and expected. Co-authored-by: Cursor <cursoragent@cursor.com>
Even though inter.optimized.ts was not reachable via the webpack module graph when optimizedFonts was false, the Next.js/TypeScript build pipeline was scanning all files under src/ (via tsconfig include: ["src/**/*.ts"]) and feeding them to Babel, which threw the "next/font requires SWC" error. Moving the real next/font/google module to fonts/inter.ts (package root, outside src/) keeps it out of every TypeScript scan and Next.js scanner. It is only added to the webpack module graph by NormalModuleReplacementPlugin when the store explicitly sets experimental.optimizedFonts: true, at which point SWC is required. Co-authored-by: Cursor <cursoragent@cursor.com>
…hosting next/font/google has a hard requirement on SWC. Stores that ship a custom .babelrc.js cause Next.js to disable SWC, which breaks any next/font/* import at build time — regardless of webpack aliases, conditional require()s, or where the file lives in the project. This commit swaps the implementation to @fontsource/inter (CSS-only, compiler-agnostic), which works with both Babel and SWC. The opt-in contract (experimental.optimizedFonts) is unchanged: - src/fonts/inter.ts is an empty stub by default (no .woff2 bundled when the flag is off → still meets the "zero cost when not opted in" acceptance criterion). - fonts/inter.ts (outside src/) side-effect-imports the @fontsource/inter weight CSS files. webpack's NormalModuleReplacementPlugin redirects src/fonts/inter to it only when optimizedFonts === true. - The CSS import lives in _app.tsx (the only location Next.js allows global CSS imports for pages router), and _document.tsx is reverted to its original body className="theme". - Adds @fontsource/inter@^5.2.6 as a runtime dependency in core. See CHANGELOG.md for the full dependency justification per AGENTS.md. No breaking change: stores that have not enabled optimizedFonts see the exact same HTML output, bundle contents, and font loading behavior as before this commit. Co-authored-by: Cursor <cursoragent@cursor.com>
PSI data from 3 runs on the deployed store confirms that the LCP element is always the first product grid item (fetchpriority=high on the <img> but no <link rel=preload> in <head>), even when a Hero section is present. The hero renders at a smaller visual area than the product card on mobile, so it never wins the LCP contest. The previous code preloaded the hero image when present, which caused a cache miss on the actual LCP image and left its resource-load-delay at ~1.6s. This fix removes the hero branch and always preloads the first product image at width=256 (appropriate for sizes="30vw" at 2× DPR). Co-Authored-By: Paperclip <noreply@paperclip.ing>
Replace @builder.io/partytown@^0.6.1 with @qwik.dev/partytown@^0.14.0 in packages/core/package.json and update the issue comment URL in lighthouserc.js from BuilderIO/partytown to QwikDev/partytown. Closes VTE-18 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The committed pnpm-lock.yaml was missing the top-level `overrides:` block declared in pnpm-workspace.yaml (graphql 16.11.0, typescript 5.9.3), causing CI to fail with ERR_PNPM_LOCKFILE_CONFIG_MISMATCH on frozen install. Co-authored-by: Cursor <cursoragent@cursor.com>
Combines PLP performance fixes (VTE-14) with the partytown migration (VTE-18) into a single branch so both changes can be tested together. Regenerated pnpm-lock.yaml to include @fontsource/inter introduced by the PLP branch. Co-authored-by: Cursor <cursoragent@cursor.com>
renatomaurovtex
requested review from
emersonlaurentino and
hellofanny
and removed request for
a team
May 22, 2026 18:30
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
Contributor
WalkthroughAdds an opt-in experimental flag to self-host Inter fonts via webpack module replacement, improves image LCP by preloading and prioritizing key images, refactors first-page fetch timing to use client page-load time, and includes related tests and small component/SSR fixes. ChangesPerformance Optimizations: Fonts, LCP, and Data Freshness
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core/src/sdk/product/useShouldFetchFirstPage.ts (1)
50-55:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftMove
lastFetchTimeupdate out of the render path
useShouldFetchFirstPagemutateslastFetchTime.current = Date.now()during render (lines 50-55). InusePageProductsQuery, the actual fetch can still be skipped becauseuseQueryusesdoNotRun: !shouldFetch || (isDeliveryPromiseEnabled && isSessionValidating, which returnsnulland prevents the request. So the timestamp can advance even when no fetch happened, incorrectly delaying the next refresh window by 5 minutes. Update the timestamp when the fetch is actually started/completed (e.g., via effect keyed on the request outcome), not inside the boolean computation.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/sdk/product/useShouldFetchFirstPage.ts` around lines 50 - 55, The hook useShouldFetchFirstPage currently mutates lastFetchTime.current = Date.now() during render, which can advance the timestamp even if the fetch is skipped by useQuery (doNotRun logic); remove that assignment from the boolean computation in useShouldFetchFirstPage and instead update lastFetchTime inside an effect or callback in usePageProductsQuery that runs only when a real fetch is initiated or successfully completed (e.g., track the request start/fulfillment via the useQuery lifecycle or an effect keyed on shouldFetch && !isDeliveryPromiseEnabled && !isSessionValidating and the query running state) so the timestamp only advances when an actual network request occurs; adjust references to lastFetchTime, useShouldFetchFirstPage, usePageProductsQuery, useQuery, doNotRun, isDeliveryPromiseEnabled, and isSessionValidating accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/core/src/sdk/product/useShouldFetchFirstPage.ts`:
- Around line 50-55: The hook useShouldFetchFirstPage currently mutates
lastFetchTime.current = Date.now() during render, which can advance the
timestamp even if the fetch is skipped by useQuery (doNotRun logic); remove that
assignment from the boolean computation in useShouldFetchFirstPage and instead
update lastFetchTime inside an effect or callback in usePageProductsQuery that
runs only when a real fetch is initiated or successfully completed (e.g., track
the request start/fulfillment via the useQuery lifecycle or an effect keyed on
shouldFetch && !isDeliveryPromiseEnabled && !isSessionValidating and the query
running state) so the timestamp only advances when an actual network request
occurs; adjust references to lastFetchTime, useShouldFetchFirstPage,
usePageProductsQuery, useQuery, doNotRun, isDeliveryPromiseEnabled, and
isSessionValidating accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: cd77588b-8e6e-46cd-afcd-081efa53c49b
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamland included by none
📒 Files selected for processing (17)
packages/core/CHANGELOG.mdpackages/core/discovery.config.default.jspackages/core/fonts/inter.tspackages/core/lighthouserc.jspackages/core/next.config.jspackages/core/package.jsonpackages/core/src/components/product/ProductCard/ProductCard.tsxpackages/core/src/components/product/ProductGrid/ProductGrid.tsxpackages/core/src/components/templates/ProductListingPage/ProductListingPage.tsxpackages/core/src/components/ui/ProductGallery/ProductGallery.tsxpackages/core/src/fonts/inter.tspackages/core/src/pages/_app.tsxpackages/core/src/sdk/product/usePageProductsQuery.tspackages/core/src/sdk/product/useShouldFetchFirstPage.tspackages/core/src/sdk/ui/useScreenResize.tspackages/core/test/fonts/inter.test.tspackages/core/test/fonts/optimizedFonts.test.ts
💤 Files with no reviewable changes (1)
- packages/core/src/sdk/product/usePageProductsQuery.ts
Root cause (found via PSI on deploy dwitlwb98): - resource-load-delay = 2,020 ms → LCP = 4.9 s Two independent bugs kept the LCP image arriving late: 1. ViewportObserver blocks SSR of product images RenderSections wraps every CMS section in a ViewportObserver (LazyLoadingSection) that initialises isVisible=false on the server. This means zero <img> tags for the ProductGallery appear in the SSR/ISR HTML; the browser can only discover product images after React hydrates (~2 s on Slow 4G + 4× CPU throttle). Fix: in ProductListing.tsx, set skipLazyLoadingSection=true on the ProductGallery section entry before passing it to RenderSections. This bypasses ViewportObserver and lets Next.js embed the actual <img> elements in the static HTML so the browser fetches them at parse time. 2. Preload URL width mismatch (256 ≠ 320) next.config.js defines imageSizes=[34,68,154,320]. With sizes="30vw" at Lighthouse mobile (412 px viewport, DPR≈2): 30%×412×2 = 247 px → browser picks 320 (first step ≥ 247). Our preload used width=256, which is not a configured image step, so the preload URL (...-256-auto/...) never matched the srcset entry the browser selected (...-320-auto/...). Both were fetched, wasting the preload. Fix: change the preload width from 256 to 320. Expected impact: resource-load-delay drops from ~2,020 ms to ~100 ms; LCP from 4.9 s to ~2.5–3.0 s on mobile Slow 4G. Co-Authored-By: Paperclip <noreply@paperclip.ing>
@faststore/api
@faststore/cli
@faststore/components
@faststore/core
@faststore/diagnostics
@faststore/lighthouse
@faststore/sdk
@faststore/ui
commit: |
Section.data from @vtex/client-cms is typed as `unknown`, which TS refuses to spread. Cast to Record<string, unknown> when overriding it with skipLazyLoadingSection so consumers with strict TS settings build. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/core/src/components/templates/ProductListingPage/ProductListing.tsx (1)
100-103: ⚡ Quick winAvoid assertion here; narrow
section.datawith a runtime object guard.This assertion suppresses type safety and can mask unexpected CMS payload shapes. Guard
section.dataas an object before spreading.Suggested fix
- data: { - ...(section.data as Record<string, unknown>), - skipLazyLoadingSection: true, - }, + data: { + ...( + section.data !== null && + typeof section.data === 'object' && + !Array.isArray(section.data) + ? section.data + : {} + ), + skipLazyLoadingSection: true, + },As per coding guidelines, "Ensure type safety and avoid type assertions when possible."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/src/components/templates/ProductListingPage/ProductListing.tsx` around lines 100 - 103, The code spreads section.data using a type assertion (section.data as Record<string, unknown>), which bypasses type safety; change this to a runtime object guard in ProductListing (the place where data: { ...(section.data as Record<string, unknown>), skipLazyLoadingSection: true } is built) — check that section.data is a non-null object (e.g., typeof section.data === 'object' && section.data !== null) and spread it only when true, otherwise spread an empty object, while still adding skipLazyLoadingSection: true; update the logic inside the data construction in the ProductListing component to use this guard instead of the type assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@packages/core/src/components/templates/ProductListingPage/ProductListing.tsx`:
- Around line 100-103: The code spreads section.data using a type assertion
(section.data as Record<string, unknown>), which bypasses type safety; change
this to a runtime object guard in ProductListing (the place where data: {
...(section.data as Record<string, unknown>), skipLazyLoadingSection: true } is
built) — check that section.data is a non-null object (e.g., typeof section.data
=== 'object' && section.data !== null) and spread it only when true, otherwise
spread an empty object, while still adding skipLazyLoadingSection: true; update
the logic inside the data construction in the ProductListing component to use
this guard instead of the type assertion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4dd148a7-73f6-4d07-948e-bc9084b9d806
📒 Files selected for processing (1)
packages/core/src/components/templates/ProductListingPage/ProductListing.tsx
…TE-9) PSI on deploy 7go5igoom (branch chore/test-combined-vte-14-vte-18) showed CLS jumping from 0 to 0.444 after the skipLazyLoadingSection change. Root cause: ProductGalleryPage cannot SSR product data because useGalleryPage relies on client-only React hooks (SWR/useSearch/useSession). With skipLazyLoadingSection=true, the ViewportObserver placeholder (height=823px) that reserved vertical space was removed. During SSR, ProductGallery rendered with 0-height (empty product list). After hydration, product cards appeared and pushed all content below downward → CLS = 0.444. The LCP improvement hypothesis was also wrong: element-render-delay remained ~1,200ms because the <img> still only appears after React hydrates (product data is client-side), making the skipLazyLoadingSection change a pure downside. Keeping the preload width fix (256→320, commit 89a0874): resource-load-delay confirmed improved from 2,020ms to 280ms in 7go5igoom PSI data. That benefit is preserved; only the CLS-breaking section change is reverted. Co-Authored-By: Paperclip <noreply@paperclip.ing>
|
4 tasks
Contributor
Author
renatomaurovtex
added a commit
that referenced
this pull request
Jun 1, 2026
## Summary Improves PLP LCP by making the browser fetch and prioritize the first product card image earlier. - **ProductListingPage**: render `<link rel="preload" as="image" fetchPriority="high">` in `<Head>` for the first product image. The preload URL is generated by `faststoreLoader` with `width=320` so it exactly matches the `<img>` srcset selection at Lighthouse mobile (412px viewport, DPR≈2, `sizes="30vw"` → 247px → next srcset step = 320). This avoids a double fetch. - **ProductGrid**: set `fetchPriority: 'high'` on the first card image, and switch eager loading from `idx === 0` to `idx < 4` so the first visible row is not blocked by lazy loading. - **ProductCard**: forward `fetchPriority` from `imgProps` to the underlying `Image` component. This is the first slice of #3345. ## Context PageSpeed Insights identifies the first product card image as the LCP element on PLP even when a Hero section is present — the hero renders at a smaller visual area than the first product card on mobile. ## Test plan - [ ] Lighthouse mobile PLP run shows LCP element = first product image and the preload reused (no `request not used` warning in DevTools). - [ ] PLP renders correctly when `server.search.products.edges[0]` is missing (no preload link rendered). - [ ] Other PLPs and PDP unaffected. - [ ] `pnpm test --filter=@faststore/core` passes. Made with [Cursor](https://cursor.com) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Performance Improvements** * Optimized image loading on product listing pages with intelligent priority handling to improve page load speed. * First product images are now prioritized for faster initial rendering on all devices. * Product cards now support configurable image loading priority settings for enhanced performance control. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/vtex/faststore/pull/3348?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Cursor <cursoragent@cursor.com>
renatomaurovtex
added a commit
that referenced
this pull request
Jun 1, 2026
…3350) ## Summary `useShouldFetchFirstPage` previously used `generatedBuildTime` as the reference for the 5-minute refetch window. For static PLP pages served hours or days after the build, the build-time check is always `true` on the first client-side render, so the hook would refetch on every initial navigation — defeating the cache injected by SSR. This PR switches to a per-mount `pageLoadTime` ref (set on first render via `useRef(Date.now())`) and only refetches once 5 minutes have elapsed since the user actually opened the page. This matches the documented intent ("don't refetch immediately after SSR injects the cache") and removes the dependency on the build timestamp from `usePageProductsQuery`. Also reorders the hook body so `useRef` is called before the `page !== 0` early return, complying with the Rules of Hooks. This is the third slice of #3345. ## Behavior changes - First PLP visit no longer triggers a redundant client-side fetch right after SSR. - Subsequent refetch logic (after 5min) is unchanged in spirit, but anchored to the user's session rather than the build. - Public API of `useShouldFetchFirstPage` changes: the `generatedBuildTime` parameter is removed. Only internal call site (`usePageProductsQuery`) is updated; the hook is not part of the documented SDK surface. ## Test plan - [ ] On a fresh PLP load, the network panel shows only the SSR-injected query, no immediate client refetch. - [ ] After waiting 5 minutes and triggering a refetch (filter/sort change), the first-page fetch happens. - [ ] `pnpm test --filter=@faststore/core` passes (existing tests for `useShouldFetchFirstPage` may need updates — flag if so). Made with [Cursor](https://cursor.com) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Refactor** * Updated product data refresh logic to use client-side page load timing instead of server-side build time for determining when to refetch the first page, while maintaining the 5-minute refresh window. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/vtex/faststore/pull/3350?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Cursor <cursoragent@cursor.com>
renatomaurovtex
added a commit
that referenced
this pull request
Jun 1, 2026
…VTE-9) (#3349) ## Summary Two related changes that reduce layout shift on PLP first paint. - **ProductGallery**: replace `React.lazy(() => import('./ProductGalleryPage'))` with a static import of `ProductGalleryPage`. The lazy chunk was loaded on the client after hydration, so the gallery first rendered the Suspense fallback and then swapped in the real content, producing a measurable CLS. Inlining the gallery into the parent bundle removes the swap. - **useScreenResize**: switch from `useEffect` to an isomorphic `useLayoutEffect` (`useLayoutEffect` in the browser, `useEffect` during SSR). The previous `useEffect` ran after the first paint, so mobile-only branches first rendered with `isMobile=undefined` and then shifted once the effect updated the state. `useLayoutEffect` runs synchronously before paint, eliminating the shift. This is the second slice of #3345. ## Notes for reviewers - SonarQube flagged `useIsomorphicLayoutEffect` definition with a `negated condition` warning and a `Prefer globalThis.window over window` warning. Both are intentional — the negated form is the canonical pattern from the React docs, and `window` (not `globalThis.window`) is the convention used throughout the rest of the file. Happy to revisit if reviewers prefer otherwise. ## Test plan - [ ] PLP first paint CWV: CLS does not regress (Lighthouse mobile). - [ ] PLP renders the full gallery on first paint (no Suspense fallback flash). - [ ] `useScreenResize` consumers continue to receive correct `isMobile`/`isTablet`/`isDesktop` after window resize. - [ ] `pnpm test --filter=@faststore/core` passes. Made with [Cursor](https://cursor.com) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Refactor** * Improved component loading and rendering architecture for better performance * Enhanced server-side rendering compatibility for layout operations <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/vtex/faststore/pull/3349?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Cursor <cursoragent@cursor.com>
renatomaurovtex
added a commit
that referenced
this pull request
Jun 1, 2026
…onts (#3351) ## Summary Adds a new opt-in config flag `experimental.optimizedFonts` (default `false`). When enabled, FastStore self-hosts the Inter font via `@fontsource/inter`, eliminating the render-blocking request to `fonts.googleapis.com` and improving FCP by ~660ms on Lighthouse mobile. This is the fourth slice of #3345. > **Stacked PR.** Base branch is `fix/vte-18-partytown-migration` (#3344), so the `pnpm-lock.yaml` diff here only shows the `@fontsource/inter` entries instead of the full lockfile reformat that the partytown PR already includes. After #3344 lands, this PR will be rebased onto `dev` automatically and the diff against `dev` will remain minimal. ## How it works - `discovery.config.default.js` exposes `experimental.optimizedFonts: false`. - `packages/core/src/fonts/inter.ts` is an empty stub. When the flag is `true`, `next.config.js` uses webpack's `NormalModuleReplacementPlugin` to rewrite imports of `src/fonts/inter` to `packages/core/fonts/inter.ts` (located **outside** `src/`, so Babel never scans it). - `packages/core/fonts/inter.ts` side-effect imports the `@fontsource/inter` CSS files (400/500/600/700/900), which ship the self-hosted Inter `.woff2` assets. - `_app.tsx` imports `src/fonts/inter` because Next.js requires global CSS imports to live in `_app.tsx`. - When the flag is `false` (default), webpack never bundles the package — **zero cost when off**. ## Why plain CSS imports instead of `next/font/google` The first attempt used `next/font/google`, which fails at build time for stores shipping a custom `.babelrc.js` (Next.js disables SWC, and `next/font` requires SWC). `@fontsource/inter` is compiler-agnostic plain CSS + woff2, so it works with both Babel and SWC. ## Dependency justification (per [AGENTS.md > Dependency Discipline](https://github.com/vtex/faststore/blob/dev/AGENTS.md#dependency-discipline)) Recorded in detail in `packages/core/CHANGELOG.md`: - **Need:** powers the opt-in `experimental.optimizedFonts` path; required by the Babel-safe constraint above. - **Evaluation:** MIT-licensed, actively maintained (5.x, regular releases), no critical CVEs, ships its own types/exports. - **Bundle impact:** zero when the flag is off — `NormalModuleReplacementPlugin` keeps it out of the module graph. - **Bucket:** `dependencies` (must be resolvable when stores opt in). - **Pinning:** declared locally in `packages/core/package.json`, caret-pinned to `^5.2.6`. Requesting maintainer approval per the Agent Autonomy Boundaries (new runtime dependency in a published package). ## How to enable in a store 1. In `discovery.config.js`: ```js experimental: { optimizedFonts: true, } ``` 2. Remove any `<link rel="stylesheet" href="https://fonts.googleapis.com/...">` from `customizations/src/fonts/WebFonts.tsx` to avoid duplicate font loads. If the store uses a font other than Inter, **do not enable this flag** — keep the existing `WebFonts.tsx` configuration. ## Test plan - [ ] With `optimizedFonts: false` (default): build is byte-identical to before, no `.woff2` in the bundle. - [ ] With `optimizedFonts: true`: `.woff2` files ship, no request to `fonts.googleapis.com`, Inter renders correctly across weights 400/500/600/700/900. - [ ] Babel store (custom `.babelrc.js`) builds cleanly with the flag on. - [ ] SWC store builds cleanly with the flag on. - [ ] `pnpm test --filter=@faststore/core` passes (new tests under `test/fonts/`). - [ ] Lighthouse mobile FCP improves on a real store with the flag enabled. Made with [Cursor](https://cursor.com) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Added an opt-in experimental `optimizedFonts` flag to self-host the Inter font (defaults to false), enabling optional local font delivery to reduce external requests. * **Documentation** * Updated changelog with guidance for enabling the font optimization and related setup notes. * **Chores** * Added runtime dependency to support self-hosted Inter fonts. * **Tests** * Added unit tests to verify the default flag value and font-module behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Cursor <cursoragent@cursor.com>
lemagnetic
pushed a commit
that referenced
this pull request
Jun 16, 2026
](https://app.coderabbit.ai/change-stack/vtex/faststore/pull/3348?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack){kind=link}
](https://app.coderabbit.ai/change-stack/vtex/faststore/pull/3350?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack){kind=link}
](https://app.coderabbit.ai/change-stack/vtex/faststore/pull/3349?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack){kind=link}
…onts (#3351) Adds a new opt-in config flag `experimental.optimizedFonts` (default `false`). When enabled, FastStore self-hosts the Inter font via `@fontsource/inter`, eliminating the render-blocking request to `fonts.googleapis.com` and improving FCP by ~660ms on Lighthouse mobile. This is the fourth slice of #3345. > **Stacked PR.** Base branch is `fix/vte-18-partytown-migration` (#3344), so the `pnpm-lock.yaml` diff here only shows the `@fontsource/inter` entries instead of the full lockfile reformat that the partytown PR already includes. After #3344 lands, this PR will be rebased onto `dev` automatically and the diff against `dev` will remain minimal. - `discovery.config.default.js` exposes `experimental.optimizedFonts: false`. - `packages/core/src/fonts/inter.ts` is an empty stub. When the flag is `true`, `next.config.js` uses webpack's `NormalModuleReplacementPlugin` to rewrite imports of `src/fonts/inter` to `packages/core/fonts/inter.ts` (located **outside** `src/`, so Babel never scans it). - `packages/core/fonts/inter.ts` side-effect imports the `@fontsource/inter` CSS files (400/500/600/700/900), which ship the self-hosted Inter `.woff2` assets. - `_app.tsx` imports `src/fonts/inter` because Next.js requires global CSS imports to live in `_app.tsx`. - When the flag is `false` (default), webpack never bundles the package — **zero cost when off**. The first attempt used `next/font/google`, which fails at build time for stores shipping a custom `.babelrc.js` (Next.js disables SWC, and `next/font` requires SWC). `@fontsource/inter` is compiler-agnostic plain CSS + woff2, so it works with both Babel and SWC. Discipline](https://github.com/vtex/faststore/blob/dev/AGENTS.md#dependency-discipline)) Recorded in detail in `packages/core/CHANGELOG.md`: - **Need:** powers the opt-in `experimental.optimizedFonts` path; required by the Babel-safe constraint above. - **Evaluation:** MIT-licensed, actively maintained (5.x, regular releases), no critical CVEs, ships its own types/exports. - **Bundle impact:** zero when the flag is off — `NormalModuleReplacementPlugin` keeps it out of the module graph. - **Bucket:** `dependencies` (must be resolvable when stores opt in). - **Pinning:** declared locally in `packages/core/package.json`, caret-pinned to `^5.2.6`. Requesting maintainer approval per the Agent Autonomy Boundaries (new runtime dependency in a published package). 1. In `discovery.config.js`: ```js experimental: { optimizedFonts: true, } ``` 2. Remove any `<link rel="stylesheet" href="https://fonts.googleapis.com/...">` from `customizations/src/fonts/WebFonts.tsx` to avoid duplicate font loads. If the store uses a font other than Inter, **do not enable this flag** — keep the existing `WebFonts.tsx` configuration. - [ ] With `optimizedFonts: false` (default): build is byte-identical to before, no `.woff2` in the bundle. - [ ] With `optimizedFonts: true`: `.woff2` files ship, no request to `fonts.googleapis.com`, Inter renders correctly across weights 400/500/600/700/900. - [ ] Babel store (custom `.babelrc.js`) builds cleanly with the flag on. - [ ] SWC store builds cleanly with the flag on. - [ ] `pnpm test --filter=@faststore/core` passes (new tests under `test/fonts/`). - [ ] Lighthouse mobile FCP improves on a real store with the flag enabled. Made with [Cursor](https://cursor.com) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> * **New Features** * Added an opt-in experimental `optimizedFonts` flag to self-host the Inter font (defaults to false), enabling optional local font delivery to reduce external requests. * **Documentation** * Updated changelog with guidance for enabling the font optimization and related setup notes. * **Chores** * Added runtime dependency to support self-hosted Inter fonts. * **Tests** * Added unit tests to verify the default flag value and font-module behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Cursor <cursoragent@cursor.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Test-only branch that combines the two in-flight fixes so they can be validated together in a single deploy preview / starter store install:
fix/vte-14-plp-performance): PLP performance fixes (LCP image preload,fetchPriority=highon first product image, self-hosted Inter via@fontsource/inter, related SDK tweaks).fix/vte-18-partytown-migration): partytown dependency migrated from@builder.io/partytownto@qwik.dev/partytown@0.14.0.This PR is not intended to be merged — it exists only so we can spin up a CodeSandbox / starter store with both changes applied and validate they don't regress each other.
Conflicts resolved
pnpm-lock.yaml: regenerated against the mergedpackages/core/package.json(which now contains@fontsource/inter@^5.2.6from VTE-14 on top of the partytown migration from VTE-18). Workspaceoverrides:forgraphql/typescriptare preserved.Test plan
@faststore/core.Do not merge
After validation, this branch should be deleted. The actual fixes will land via PR #3344 (VTE-18) and the upcoming VTE-14 PR.
Made with Cursor
Summary by CodeRabbit
New Features
Performance
Documentation
Tests
Chores