Skip to content

fix: Added fetchPriority and removed#1144

Merged
iago1501 merged 4 commits intovtex-apps:masterfrom
felipelourencoacct:fix/added-fetchpriority-productImage
Oct 13, 2025
Merged

fix: Added fetchPriority and removed#1144
iago1501 merged 4 commits intovtex-apps:masterfrom
felipelourencoacct:fix/added-fetchpriority-productImage

Conversation

@felipelourencoacct
Copy link
Copy Markdown
Contributor

What problem is this solving?

Adds fetchPriority to ProductImage to improve LCP, as recommended by PageSpeed Insights.
Removes data-vtex-preload to ensure fetchPriority takes effect.

How to test it?

Workspace

Screenshots or example usage:

Before

image image

After

image

Describe alternatives you've considered, if any.

Related to / Depends on

How does this PR make you feel? 🔗

![](put .gif link here - can be found under "advanced" on giphy)

@vtex-io-ci-cd
Copy link
Copy Markdown
Contributor

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

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
Copy link
Copy Markdown

vtex-io-docs-bot Bot commented Aug 11, 2025

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 🎉🎉

@vmourac-vtex
Copy link
Copy Markdown
Contributor

/review

@qodo-code-review
Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Browser Support

Verify that adding the fetchPriority attribute does not cause TypeScript or runtime issues in environments/browsers that do not support it, and that the new JSX typing change correctly propagates to all usages of .

ref={imageRef}
className={`${applyModifiers(handles.productImageTag, 'main')}`}
style={{
  width: '100%',
  height: '100%',
  maxHeight: maxHeight || 'unset',
  objectFit: 'contain',
}}
src={imageUrl(src, DEFAULT_SIZE, MAX_SIZE, aspectRatio)}
srcSet={srcSet}
alt={alt}
title={alt}
loading={index === 0 ? 'eager' : 'lazy'}
fetchPriority={index === 0 ? 'high' : 'low'}
// WIP
LCP Targeting

Confirm that index === 0 always corresponds to the LCP image in all product image layouts; otherwise, fetchPriority 'high' may be assigned to a non-LCP image, reducing effectiveness.

ref={imageRef}
className={`${applyModifiers(handles.productImageTag, 'main')}`}
style={{
  width: '100%',
  height: '100%',
  maxHeight: maxHeight || 'unset',
  objectFit: 'contain',
}}
src={imageUrl(src, DEFAULT_SIZE, MAX_SIZE, aspectRatio)}
srcSet={srcSet}
alt={alt}
title={alt}
loading={index === 0 ? 'eager' : 'lazy'}
fetchPriority={index === 0 ? 'high' : 'low'}
// WIP

@vmourac-vtex
Copy link
Copy Markdown
Contributor

/improve

@qodo-code-review
Copy link
Copy Markdown

PR Code Suggestions ✨

CategorySuggestion                                                                                                                                    Impact
General
Align fetch priority with loading

Ensure that fetchPriority aligns with the loading strategy to avoid
deprioritizing non-lazy images on slow networks. Use 'auto' for non-hero images
to let the browser decide and reserve 'high' only for the primary image. This
prevents unintended resource contention and improves overall page performance.

react/components/ProductImages/components/ProductImage.tsx [108-109]

 loading={index === 0 ? 'eager' : 'lazy'}
-fetchPriority={index === 0 ? 'high' : 'low'}
+fetchPriority={index === 0 ? 'high' : 'auto'}
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly points out that using fetchPriority='auto' for non-hero images is a better practice, as it allows the browser to intelligently prioritize resources, potentially improving performance over a fixed 'low' priority.

Low
Remove redundant title attribute

Avoid duplicating alt text in the title attribute, which can create redundant
announcements for assistive technologies. Remove title or set it only when it
conveys additional, non-duplicative information.

react/components/ProductImages/components/ProductImage.tsx [104-107]

 src={imageUrl(src, DEFAULT_SIZE, MAX_SIZE, aspectRatio)}
 srcSet={srcSet}
 alt={alt}
-title={alt}

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 5

__

Why: This is a valid accessibility improvement, as duplicating the alt text in the title attribute can be redundant for screen reader users; however, this code was not modified in the PR.

Low
Ensure centered image positioning

Explicitly set objectPosition to ensure predictable centering when using
objectFit: 'contain', preventing unexpected cropping or alignment issues across
browsers. This reduces layout shifts in edge cases.

react/components/ProductImages/components/ProductImage.tsx [98-103]

 style={{
   width: '100%',
   height: '100%',
   maxHeight: maxHeight || 'unset',
   objectFit: 'contain',
+  objectPosition: 'center center',
 }}

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 4

__

Why: The suggestion to add objectPosition is a good practice for ensuring consistent image centering with objectFit: 'contain', but it addresses an issue not directly related to the PR's changes.

Low
  • More

@vmourac-vtex
Copy link
Copy Markdown
Contributor

/improve --pr_code_suggestions.commitable_code_suggestions=true

@@ -94,7 +94,6 @@ const ProductImage: FC<Props> = ({
>
<img
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Suggestion: Avoid setting a low fetch priority when loading="lazy" is used; browsers treat lazy images as low priority automatically, and explicitly forcing low can harm above-the-fold cases when index logic changes. Only set fetchPriority="high" for the first image and omit it otherwise to prevent unintended network prioritization issues. [general, importance: 6]

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR improves LCP performance by adding fetchPriority to the ProductImage component and removing the data-vtex-preload attribute, following PageSpeed Insights recommendations.

  • Added fetchPriority attribute with 'high' priority for first image and 'low' for subsequent images
  • Removed data-vtex-preload attribute to ensure fetchPriority takes effect
  • Updated changelog to document the changes

Reviewed Changes

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

File Description
react/components/ProductImages/components/ProductImage.tsx Added fetchPriority attribute and removed data-vtex-preload to optimize LCP
CHANGELOG.md Documented the ProductImage component changes

Comment thread react/components/ProductImages/components/ProductImage.tsx Outdated
Comment thread react/components/ProductImages/components/ProductImage.tsx
Comment thread react/components/ProductImages/components/ProductImage.tsx Outdated
Comment thread react/components/ProductImages/components/ProductImage.tsx
@iago1501 iago1501 merged commit 7582f15 into vtex-apps:master Oct 13, 2025
3 of 5 checks passed
@vtex-io-ci-cd
Copy link
Copy Markdown
Contributor

vtex-io-ci-cd Bot commented Oct 13, 2025

Your PR has been merged! App is being published. 🚀
Version 3.178.4 → 3.178.5

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.store-components@3.178.5

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants