chore(fix): optimise images #36
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis pull request introduces a comprehensive image optimization utility for Cloudinary URLs across multiple Svelte components. The changes focus on creating a centralized Changes
Sequence DiagramsequenceDiagram
participant Component as Svelte Component
participant Utility as optimise-cloudinary.ts
participant Cloudinary as Cloudinary URL
Component->>Utility: Call optimizeCloudinaryUrl()
Utility-->>Component: Return optimized URL
Component->>Cloudinary: Request optimized image
Cloudinary-->>Component: Serve transformed image
Possibly related PRs
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
static/hero.pngis excluded by!**/*.png
📒 Files selected for processing (5)
src/lib/components/ui/ArticleCard.svelte(2 hunks)src/lib/components/ui/ArticleSpotlight.svelte(1 hunks)src/lib/services/article.service.ts(1 hunks)src/lib/utils/optimise-cloudinary.ts(1 hunks)src/routes/+page.svelte(2 hunks)
🧰 Additional context used
🪛 eslint
src/routes/+page.svelte
[error] 11-11: 'optimizeCloudinaryUrl' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 13-13: 'heroImage' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
🔇 Additional comments (4)
src/lib/components/ui/ArticleSpotlight.svelte (1)
4-4: LGTM! Good use of image optimization utility
The implementation correctly uses the optimizeCloudinaryUrl utility with the 'spotlight' type parameter.
Also applies to: 12-12
src/lib/utils/optimise-cloudinary.ts (2)
1-7: LGTM! Well-structured type definitions.
The types are well-defined and properly constrained for the image optimization use case.
39-46: LGTM! Clean URL construction with backward compatibility.
The URL construction logic is correct, and the legacy function is well-documented.
src/routes/+page.svelte (1)
11-14:
Remove unused imports and constants.
The following items are imported/defined but never used:
optimizeCloudinaryUrlimportheroImageconstant
This suggests the image optimization implementation might be incomplete.
✅ Verification successful
Remove unused imports and constants
The following items are defined but never used in the code:
optimizeCloudinaryUrlimportheroImageconstant
The heroImage constant is only defined but never referenced in the code, and the hero image is instead loaded from static files (/hero-720w.webp). The optimizeCloudinaryUrl import is also unused in this file, though it is actively used in other components like ArticleSpotlight and ArticleCard.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if heroImage is used elsewhere in the codebase
rg "heroImage" --type svelte
Length of output: 59
Script:
#!/bin/bash
# Check if heroImage is used in the file content
rg "heroImage"
# Check if optimizeCloudinaryUrl is used in the file content
rg "optimizeCloudinaryUrl"
# Show the content of the file to verify usage
cat "src/routes/+page.svelte"
Length of output: 3297
🧰 Tools
🪛 eslint
[error] 11-11: 'optimizeCloudinaryUrl' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 13-13: 'heroImage' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
src/lib/components/ui/ArticleCard.svelte(2 hunks)src/lib/components/ui/ArticleSpotlight.svelte(1 hunks)src/lib/utils/optimise-cloudinary.ts(1 hunks)src/routes/+page.svelte(2 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
src/lib/utils/optimise-cloudinary.ts
[error] 38-38: This let declares a variable that is only assigned once.
'transformParams' is never reassigned.
Safe fix: Use const instead.
(lint/style/useConst)
🪛 eslint
src/lib/utils/optimise-cloudinary.ts
[error] 38-38: 'transformParams' is never reassigned. Use 'const' instead.
(prefer-const)
🔇 Additional comments (8)
src/lib/utils/optimise-cloudinary.ts (2)
1-1: Good addition of 'hero' to the ImageType union.
This enhances flexibility by allowing hero images to be separately categorized.
7-7: DPR parameter inclusion looks good.
Allowing an optional device pixel ratio helps in delivering sharper images on high-resolution displays. Ensure to default it to 1 in the calling code if no dpr value is provided.
src/lib/components/ui/ArticleSpotlight.svelte (1)
4-4: Import statement is correct.
Bringing in the optimizeCloudinaryUrl function centrally unifies the image transformation logic across components.
src/lib/components/ui/ArticleCard.svelte (2)
5-5: Consistent import usage.
Importing from '$lib/utils/optimise-cloudinary' stresses a single source of truth for image handling.
17-23: Properly specifying the card preset.
Passing { type: 'card' } ensures consistent transformation across the codebase. Including explicit width/height to prevent CLS is also good practice. If the component design evolves, consider a responsive approach similar to ArticleSpotlight.
src/routes/+page.svelte (3)
11-12: Centralizing image utilities is good design.
Importing optimizeCloudinaryUrl and optimizeHeroImage ensures the same transformations are applied consistently across the app.
13-14: Hero image reference identified.
Defining heroImage in one place makes it easier to change or update the hero image across the site.
39-69: Responsive
Specifying multiple
There was a problem hiding this comment.
Actionable comments posted: 5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src/lib/components/ui/ArticleCard.svelte(2 hunks)src/lib/utils/optimise-cloudinary.ts(1 hunks)
🔇 Additional comments (5)
src/lib/utils/optimise-cloudinary.ts (4)
1-1: Great use of a discriminated union type for ImageType.
Defining a strictly limited set of valid image types helps you catch errors at compile-time. This is a straightforward and robust solution.
16-18: Simple and effective guard for invalid or non-Cloudinary URLs.
The early return for unsupported URLs ensures stable behavior across a broader range of content.
37-45: Flexible preset application.
The code caters to preset usage or manually specified dimensions while falling back to the 'card' preset. This covers typical usage scenarios elegantly.
52-54: Intuitive final URL construction.
Appending the transformation segment and optionally adding the 'coverImage/' prefix is clear. If you add other transformations or fluid breakpoints in the future, consider a well-structured approach (e.g., a partial path builder function) to keep complexity low.
src/lib/components/ui/ArticleCard.svelte (1)
5-5: Consistent usage of optimizeCloudinaryUrl.
Importing and reusing the existing utility function ensures a single source of truth for image optimizations, improving maintainability.
| const TRANSFORM_PRESETS: Record<ImageType, string> = { | ||
| card: 'w_464,h_464,c_fill,ar_1:1,g_center,q_auto,f_auto', | ||
| spotlight: 'w_640,h_475,c_fill,f_auto,q_auto', | ||
| hero: 'w_720,h_542,c_fill,g_auto,fl_progressive,q_auto:best,f_auto' | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider clarifying the hero preset.
The hero preset uses relatively large dimensions and a combination of progressive loading plus q_auto:best. This is ideal for high-resolution hero images. However, be cautious with large file sizes for slower connections. Consider offering multiple sizes via responsive techniques or limiting q_auto:best for data-constrained users.
| let baseUrl: string; | ||
| let path: string; | ||
|
|
||
| const uploadIndex = url.lastIndexOf('/upload/'); | ||
| const coverImageIndex = url.lastIndexOf('/coverImage/'); | ||
|
|
||
| if (uploadIndex !== -1) { | ||
| baseUrl = url.substring(0, uploadIndex + '/upload'.length); | ||
| path = url.substring(uploadIndex + '/upload/'.length); | ||
| } else if (coverImageIndex !== -1) { | ||
| baseUrl = url.substring(0, coverImageIndex); | ||
| path = url.substring(coverImageIndex + '/coverImage/'.length); | ||
| } else { | ||
| return url; | ||
| } | ||
|
|
||
| if (!path) return url; | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Path extraction logic is intuitive.
Your fallback to directly return the original URL if neither '/upload/' nor '/coverImage/' is found helps avoid unintended breakage. Just watch out for future expansions if other paths (e.g., '/someOtherPath/') become relevant.
| if (options.dpr) { | ||
| const sanitizedDpr = Math.max(1, Math.min(Math.abs(options.dpr), 3)); | ||
| transformParams.push(`dpr_${sanitizedDpr.toFixed(1)}`); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Smart DPR sanitization.
Clamping the DPR between 1 and 3 is a nice safeguard. However, in some edge cases (e.g., “super retina” devices), a higher DPR might be desired. Keep an eye on future device evolutions and be prepared to bump the upper limit if needed.
| <img | ||
| src={optimizeCloudinaryUrl(article.thumb, { type: 'card' })} | ||
| loading="lazy" | ||
| alt={article.title} | ||
| class="aspect-square w-full object-cover" | ||
| width={464} | ||
| height={464} | ||
| /> |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Good use of lazy loading and explicit dimensions.
Specifying width/height prevents layout shifts, and “lazy” loading optimizes performance. For an even more responsive approach, consider adding a srcset to leverage varying screen resolutions and reduce bandwidth usage for smaller devices.
Summary by CodeRabbit
New Features
<picture>element for hero images.Improvements
dprparameter.Bug Fixes