Skip to content

feat: lcp-logo-eager-loading #3377

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

Open
renatomaurovtex wants to merge 3 commits into
base: dev
Choose a base branch
from
+104 −19
Open

feat: lcp-logo-eager-loading #3377

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e27391c
feat(core): prioritize critical images for LCP
renatomaurovtex Jun 2, 2026
ce877f6
fix(core): narrow LCP eager-loading scope and simplify head preload
renatomaurovtex Jun 2, 2026
de91b46
fix(core): reduce TTI timeout threshold
renatomaurovtex Jun 2, 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
2 changes: 1 addition & 1 deletion packages/core/src/components/navigation/Navbar/Navbar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ function Navbar({
title={logo.link ? logo.link.title : homeLabel}
prefetch={false}
>
<Logo src={logo.src} alt={logo.alt} />
<Logo src={logo.src} alt={logo.alt} loading="eager" />
</Link>
</>
)}
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/components/navigation/NavbarSlider/NavbarSlider.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ function NavbarSlider({
title={logo.link ? logo.link.title : homeLabel}
onClick={fadeOut}
>
<Logo alt={logo.alt} src={logo.src} />
<Logo alt={logo.alt} src={logo.src} loading="eager" />
</Link>
</NavbarSliderHeader.Component>
<NavbarSliderContent.Component {...NavbarSliderContent.props}>
Expand Down
21 changes: 5 additions & 16 deletions packages/core/src/components/templates/ProductListingPage/ProductListingPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,12 @@ import type {
ServerManyProductsQueryQueryVariables,
} from '@generated/graphql'
import { ITEMS_PER_PAGE } from 'src/constants'
import { getCriticalProductImagePreload } from 'src/components/ui/Image/getCriticalProductImagePreload'
import { useApplySearchState } from 'src/sdk/search/state'

import type { PLPContentType } from 'src/server/cms/plp'

import storeConfig from '../../../../discovery.config'
import { faststoreLoader } from 'src/components/ui/Image/loader'
import ProductListing from './ProductListing'
import { getStoreURL } from 'src/sdk/localization/useLocalizationConfig'

Expand Down Expand Up @@ -131,15 +131,9 @@ export default function ProductListingPage({
// 30vw × 412 × 2 = 247px → browser picks 320 (first step ≥ 247 in the srcset).
// Using 320 here makes the preload URL exactly match the <img> srcset selection,
// so the browser can reuse the preloaded response instead of fetching a second URL.
const rawLcpImageUrl: string | undefined =
const lcpImagePreload = getCriticalProductImagePreload(
server?.search?.products?.edges?.[0]?.node?.image?.[0]?.url
const lcpImageUrl = rawLcpImageUrl
? faststoreLoader({
src: rawLcpImageUrl,
width: 320,
quality: 75,
})
: undefined
)

return (
<SearchProvider
Expand All @@ -148,14 +142,9 @@ export default function ProductListingPage({
shouldResetInfiniteScroll={!storeConfig.experimental?.scrollRestoration}
{...searchParams}
>
{lcpImageUrl && (
{lcpImagePreload && (
<Head>
<link
rel="preload"
as="image"
href={lcpImageUrl}
fetchPriority="high"
/>
<link {...lcpImagePreload} />
</Head>
)}
{/* SEO */}
Expand Down
27 changes: 27 additions & 0 deletions packages/core/src/components/ui/Image/getCriticalProductImagePreload.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import { faststoreLoader } from './loader'

export interface CriticalProductImagePreload {
rel: 'preload'
as: 'image'
href: string
fetchPriority: 'high'
}

export function getCriticalProductImagePreload(
imageUrl?: string
): CriticalProductImagePreload | null {
if (!imageUrl) {
return null
}

return {
rel: 'preload',
as: 'image',
href: faststoreLoader({
src: imageUrl,
width: 320,
quality: 75,
}),
fetchPriority: 'high',
}
}
2 changes: 1 addition & 1 deletion packages/core/src/sdk/performance/useTTI.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { useEffect, useState } from 'react'

const TTI_TIMEOUT = 5000 // 5 seconds without long tasks as a criterion for Time To Interactive - https://web.dev/articles/tti
const TTI_TIMEOUT = 1000 // 5 seconds without long tasks as a criterion for Time To Interactive - https://web.dev/articles/tti

@coderabbitai coderabbitai Bot Jun 2, 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.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Update the stale comment.

The comment still says "5 seconds" but the constant is now 1000ms (1 second). This misleads readers about the actual TTI detection threshold.

📝 Proposed fix 
-const TTI_TIMEOUT = 1000 // 5 seconds without long tasks as a criterion for Time To Interactive - https://web.dev/articles/tti
+const TTI_TIMEOUT = 1000 // 1 second without long tasks as a criterion for Time To Interactive - https://web.dev/articles/tti
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const TTI_TIMEOUT = 1000 // 5 seconds without long tasks as a criterion for Time To Interactive - https://web.dev/articles/tti
const TTI_TIMEOUT = 1000 // 1 second without long tasks as a criterion for Time To Interactive - https://web.dev/articles/tti
🤖 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/performance/useTTI.ts` at line 3, The inline comment
for the TTI_TIMEOUT constant is stale (it says "5 seconds" while TTI_TIMEOUT is
1000 ms); update the comment in useTTI.ts next to the TTI_TIMEOUT constant to
reflect the actual threshold (e.g., "1 second without long tasks as a criterion
for Time To Interactive") so the text matches the numeric value 1000 and
includes the existing web.dev reference.

⚠️ Potential issue | 🟠 Major | ⚖️ Poor tradeoff

🧩 Analysis chain

🌐 Web query:

What is the recommended quiet window duration for Time to Interactive (TTI) measurement according to web.dev and web performance standards?

💡 Result:

According to web.dev and associated web performance documentation, the recommended quiet window duration for measuring Time to Interactive (TTI) is at least five seconds [1][2]. A quiet window is defined as a period of time during which there are no long tasks (tasks exceeding 50 milliseconds) and no more than two in-flight network GET requests [1][3]. It is important to note that TTI has been deprecated in modern performance auditing tools like Lighthouse (removed as of version 10) due to its high variability and sensitivity to outlier network requests and long tasks [1][4]. Official guidance now recommends using metrics such as Interaction to Next Paint (INP), Total Blocking Time (TBT), and Largest Contentful Paint (LCP) as more reliable indicators of page performance and responsiveness [1][4].

Citations:

🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="packages/core/src/sdk/performance/useTTI.ts"
if [ ! -f "$FILE" ]; then
  echo "Missing $FILE"
  exit 1
fi

echo "---- $FILE (first 120 lines) ----"
sed -n '1,120p' "$FILE" | cat -n
echo "---- $FILE (lines around TTI_TIMEOUT) ----"
rg -n "TTI_TIMEOUT|Time To Interactive|TTI|long task|quiet" "$FILE" || true

Repository: vtex/faststore

Length of output: 2387

Fix TTI_TIMEOUT in packages/core/src/sdk/performance/useTTI.ts to match TTI quiet-window guidance

  • useTTI sets TTI_TIMEOUT = 1000 (1s) as the quiet period since the last long task, but the inline comment says “5 seconds”, and web.dev’s TTI guidance uses an at-least-5s quiet window (plus constraints like limited in-flight GETs).
  • Update TTI_TIMEOUT (likely to 5000) and the comment, or clearly document why a 1s threshold is intentional and how the approximation (long tasks only) fits your use case.
  • If this measurement is for performance monitoring, consider whether modern proxies (INP/TBT/LCP) better match current standards since TTI is deprecated in tools like Lighthouse.
🤖 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/performance/useTTI.ts` at line 3, The TTI_TIMEOUT
constant used by useTTI is set to 1000 but the comment and web.dev guidance
indicate a 5s quiet window; update the TTI_TIMEOUT constant from 1000 to 5000
and revise the inline comment to reflect "5 seconds without long tasks" (or,
alternatively, if 1s is intentional, add a clear comment in useTTI explaining
why a 1000ms approximation is chosen and how long-task-only detection differs
from full TTI guidance); modify only the TTI_TIMEOUT definition and its comment
and ensure references to TTI_TIMEOUT in the useTTI logic remain unchanged.

@renatomaurovtex renatomaurovtex Jun 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Unrelated TTI_TIMEOUT change with a now-stale comment.

This isn't mentioned in the PR summary/deviations and doesn't relate to logo/LCP preload. Two issues:

  1. The comment still says "5 seconds" while the value is now 1000 (1s) — they contradict each other.
  2. Dropping the idle window 5s → 1s makes useTTI flip isInteractive to true much earlier, so anything gated behind it mounts sooner — potentially adding work inside the critical loading window and regressing LCP/CWV. Performance is NON-NEGOTIABLE here.

Was this committed by accident? If intentional, split it into its own PR with a rationale (and fix the comment); otherwise revert:

Suggested change
const TTI_TIMEOUT = 1000 // 5 seconds without long tasks as a criterion for Time To Interactive - https://web.dev/articles/tti
const TTI_TIMEOUT = 5000 // 5 seconds without long tasks as a criterion for Time To Interactive - https://web.dev/articles/tti

@eduardoformiga eduardoformiga Jun 9, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

pelo que lembro os 5 segundos sao intencionais. 1 segundo tem risco de long task ainda serem carregadas apos esse periodo.


/**
* Polyfill for requestIdleCallback, which is not available for every browser
Expand Down
40 changes: 40 additions & 0 deletions packages/core/test/components/ui/Logo.browser.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import { render } from '@testing-library/react'

import Logo from 'src/components/ui/Logo/Logo'

const imageSpy = vi.fn()

vi.mock('src/components/ui/Image', () => ({
Image: (props: Record<string, unknown>) => {
imageSpy(props)
return <div data-testid="logo-image" />
},
}))

describe('Logo', () => {
beforeEach(() => {
imageSpy.mockClear()
})

it('uses lazy loading by default', () => {
render(<Logo alt="FastStore" src="/logo.svg" />)

expect(imageSpy).toHaveBeenCalledTimes(1)
expect(imageSpy).toHaveBeenCalledWith(
expect.objectContaining({
loading: 'lazy',
})
)
})

it('keeps explicit loading overrides', () => {
render(<Logo alt="FastStore" src="/logo.svg" loading="eager" />)

expect(imageSpy).toHaveBeenCalledTimes(1)
expect(imageSpy).toHaveBeenCalledWith(
expect.objectContaining({
loading: 'eager',
})
)
})
})
29 changes: 29 additions & 0 deletions packages/core/test/sdk/head/getCriticalProductImagePreload.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import { getCriticalProductImagePreload } from 'src/components/ui/Image/getCriticalProductImagePreload'

describe('getCriticalProductImagePreload', () => {
it('returns null when no image URL is provided', () => {
expect(getCriticalProductImagePreload()).toBeNull()
})

it('builds preload metadata for a valid image URL', () => {
expect(getCriticalProductImagePreload('/product-image.jpg')).toEqual({
rel: 'preload',
as: 'image',
href: '/product-image.jpg',
fetchPriority: 'high',
})
})

it('transforms VTEX IDs image URLs using the expected preload sizing', () => {
expect(
getCriticalProductImagePreload(
'/ids/1557582/image-6.jpg?v=638808503053270000'
)
).toEqual({
rel: 'preload',
as: 'image',
href: '/ids/1557582-320-auto/image-6.webp?v=638808503053270000&quality=8',
fetchPriority: 'high',
})
})
})
Loading