Tavano/use platforms optimization #1500
Conversation
Tagging OptionsShould a new tag be published when this PR is merged?
|
WalkthroughRestructured image URL optimization logic in Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ 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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
website/components/Image.tsx(1 hunks)
🔇 Additional comments (3)
website/components/Image.tsx (3)
104-112: LGTM! Clean implementation of Wake optimization.The function correctly follows the same pattern as other platform optimizers, appending width and height query parameters to optimize Wake/VNDA platform images.
141-143: LGTM! Appropriate fallback for generic optimization.The global optimization check is now correctly positioned to only affect non-platform-specific URLs, allowing the generic Deco optimization service to be toggled independently of platform-native optimizations.
125-139: Verify that platform-specific optimizations intentionally bypass the global image optimization flag.The code confirms that VNDA, Shopify, VTEX, and Wake optimizations (lines 121-139) execute before the
isImageOptmizationEnabled()check (line 141), meaning they will always apply regardless of theenableImageOptimizationfeature flag. This is a significant behavior change.Please confirm this is intentional—that native platform optimizations should always be used independent of the global optimization flag. If confirmed, consider adding an explicit comment in the code to document why platform-specific optimizations bypass the feature flag, and update any documentation describing the optimization behavior.
| if (originalSrc.includes("fbitsstatic.net/img/")) { | ||
| return optimizeWake(opts); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n website/components/Image.tsx | head -150Repository: deco-cx/apps
Length of output: 5058
🏁 Script executed:
rg -i "fbitsstatic" --type ts --type tsx --type js -B 2 -A 2Repository: deco-cx/apps
Length of output: 83
🏁 Script executed:
rg -i "fbitsstatic" -A 2 -B 2Repository: deco-cx/apps
Length of output: 294
🏁 Script executed:
rg -i "wake|optimizeWake" -B 2 -A 2Repository: deco-cx/apps
Length of output: 23296
🏁 Script executed:
fd -e "test" -o -e "spec" | head -20Repository: deco-cx/apps
Length of output: 172
🏁 Script executed:
rg "getOptimizedMediaUrl|Image\.tsx" -l --type-listRepository: deco-cx/apps
Length of output: 445
🏁 Script executed:
rg "getOptimizedMediaUrl" -B 3 -A 3Repository: deco-cx/apps
Length of output: 2156
🏁 Script executed:
rg "test|spec" website/components/ -iRepository: deco-cx/apps
Length of output: 43173
Strengthen the Wake URL pattern check for consistency and correctness.
The check at line 121 uses .includes("fbitsstatic.net/img/"), which differs from the more precise pattern checks for other platforms (VNDA and Shopify use .startsWith() with full protocol; VTEX uses regex domain matching). The .includes() approach could match this string anywhere in the URL, including in query parameters or as part of a different domain (e.g., https://evil.com?redirect=fbitsstatic.net/img/x), potentially causing Wake optimization to be incorrectly applied to non-Wake images.
Align with other platform checks by using a precise pattern:
- if (originalSrc.includes("fbitsstatic.net/img/")) {
+ if (originalSrc.startsWith("https://fbitsstatic.net/img/") ||
+ originalSrc.startsWith("http://fbitsstatic.net/img/")) {
return optimizeWake(opts);
}Or with regex to support subdomains if needed:
- if (originalSrc.includes("fbitsstatic.net/img/")) {
+ if (/^https?:\/\/[^\/]*fbitsstatic\.net\/img\//.test(originalSrc)) {
return optimizeWake(opts);
}📝 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.
| if (originalSrc.includes("fbitsstatic.net/img/")) { | |
| return optimizeWake(opts); | |
| } | |
| if (originalSrc.startsWith("https://fbitsstatic.net/img/") || | |
| originalSrc.startsWith("http://fbitsstatic.net/img/")) { | |
| return optimizeWake(opts); | |
| } |
| if (originalSrc.includes("fbitsstatic.net/img/")) { | |
| return optimizeWake(opts); | |
| } | |
| if (/^https?:\/\/[^\/]*fbitsstatic\.net\/img\//.test(originalSrc)) { | |
| return optimizeWake(opts); | |
| } |
Summary by CodeRabbit
Refactor
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.