feat: added support for additional image formats in build-rss.js #3396
Conversation
SahilDahekar
requested review from
derberg,
akshatnema,
magicmatatjahu,
anshgoyalevil,
Mayaleeeee and
asyncapi-bot-eve
as code owners
✅ Deploy Preview for asyncapi-website ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
WalkthroughThe changes in this pull request focus on improving the MIME type detection logic within the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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
Documentation and Community
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3396 +/- ##
==========================================
- Coverage 86.59% 86.47% -0.13%
==========================================
Files 21 21
Lines 664 658 -6
==========================================
- Hits 575 569 -6
Misses 89 89 ☔ View full report in Codecov by Sentry. |
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-3396--asyncapi-website.netlify.app/ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
scripts/build-rss.js (2)
66-76: LGTM! Well-structured MIME type mapping.The implementation of
mimeTypesconstant improves code maintainability by centralizing the MIME type definitions. The coverage of image formats is comprehensive.Consider adding a helper function to normalize file extensions before lookup:
+ function normalizeExtension(filename) { + return filename.substring(filename.lastIndexOf('.')).toLowerCase(); + } const mimeTypes = { '.jpeg': 'image/jpeg', // ... other mappings };
Line range hint
66-99: Consider caching MIME type results.For better performance in high-traffic scenarios, consider implementing a caching mechanism for MIME type lookups, especially if the RSS feed is generated frequently.
You could use:
- In-memory caching for MIME types during runtime
- File-based caching for feed generation results
- HTTP caching headers in the RSS feed response
This would reduce computational overhead and improve response times for RSS feed consumers.
🧰 Tools
🪛 eslint
[error] 63-63: Replace
pwith(p)(prettier/prettier)
[error] 78-78: 'post' is never reassigned. Use 'const' instead.
(prefer-const)
[error] 80-80: 'title' is already declared in the upper scope on line 18 column 47.
(no-shadow)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
scripts/build-rss.js(2 hunks)
🧰 Additional context used
🪛 eslint
scripts/build-rss.js
[error] 96-96: Replace "@url" with '@url'
(prettier/prettier)
[error] 97-97: Replace "@length" with '@length'
(prettier/prettier)
[error] 98-98: Replace "@type" with '@type'
(prettier/prettier)
🔇 Additional comments (1)
scripts/build-rss.js (1)
93-99: 🛠️ Refactor suggestion
Address string quote consistency and improve enclosure properties.
The enclosure handling logic looks good, but there are a few improvements to consider:
- Fix quote consistency as per static analysis:
item.enclosure = {
- "@url": base + post.cover,
- "@length": 15026, // dummy value, anything works
- "@type": mimeType
+ '@url': base + post.cover,
+ '@length': 15026, // dummy value, anything works
+ '@type': mimeType
};- Consider replacing the hardcoded length with actual file size:
const https = require('https');
function getFileSize(url) {
return new Promise((resolve, reject) => {
https.get(url, (response) => {
resolve(response.headers['content-length'] || 15026);
}).on('error', reject);
});
}Let's verify the cover image URLs in the posts:
🧰 Tools
🪛 eslint
[error] 96-96: Replace "@url" with '@url'
(prettier/prettier)
[error] 97-97: Replace "@length" with '@length'
(prettier/prettier)
[error] 98-98: Replace "@type" with '@type'
(prettier/prettier)
|
/rtm |
Description
build-rss.jsRelated issue(s)
Resolves #3357
Summary by CodeRabbit
Bug Fixes
Refactor