fix(web-vitals): correct next-gen image format detection logic#273
Conversation
The condition used || instead of &&, which made it always true (a URL can never contain both 'webp' and 'avif'). This caused false positive warnings for images already using webp or avif.
📝 WalkthroughWalkthroughThe change modifies the image-format validation logic in Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/runtime/web-vitals/plugin.client.ts (1)
83-84: Optional: Consider more precise format detection.The current
includes()check could theoretically match false positives in path segments (e.g.,https://example.com/webpages/photo.pngwould match 'webp'). If you want stricter matching, you could check file extensions or use a regex pattern like/\.(webp|avif)(\?|$)/i.That said, this is pre-existing behavior and many CDNs use format in query params (e.g.,
?format=webp), so the current approach may be intentionally permissive.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/runtime/web-vitals/plugin.client.ts` around lines 83 - 84, The string-includes checks against performanceEntry.element.src can yield false positives (e.g., matching "webp" in a path segment); update the detection in plugin.client.ts to use a stricter test: parse or test the URL with a regex that matches file extensions or format query params (e.g., test /\. (webp|avif)(\?|$)/i or check URLSearchParams for a format param) and replace the two includes(...) checks with that regex/URL-based check on performanceEntry.element.src to ensure only true webp/avif images are excluded.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/runtime/web-vitals/plugin.client.ts`:
- Around line 83-84: The string-includes checks against
performanceEntry.element.src can yield false positives (e.g., matching "webp" in
a path segment); update the detection in plugin.client.ts to use a stricter
test: parse or test the URL with a regex that matches file extensions or format
query params (e.g., test /\. (webp|avif)(\?|$)/i or check URLSearchParams for a
format param) and replace the two includes(...) checks with that regex/URL-based
check on performanceEntry.element.src to ensure only true webp/avif images are
excluded.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0d1b4969-0970-46fc-9ddb-22516e72d59c
📒 Files selected for processing (1)
src/runtime/web-vitals/plugin.client.ts
commit: |
|
Nice catch ! Thanks! |
2 participants
The LCP image format check warns users to serve images in next-gen formats (webp/avif), but the condition
!src.includes('webp') || !src.includes('avif')is always true because no URL contains both format strings simultaneously. By De Morgan's law,!A || !B=!(A && B), so even a.webpimage triggers the warning.Changing to
&&makes the check work as intended: only warn when the image is neither webp nor avif.