-
-
Notifications
You must be signed in to change notification settings - Fork 36
feat: add sitemap.xml generation script #1181
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add sitemap.xml generation script #1181
Conversation
📝 WalkthroughWalkthroughA new script for generating a sitemap XML file was introduced, along with a corresponding npm script and the addition of the sitemap package as a development dependency. The generated sitemap XML file is now included in the public directory. Changes
Sequence Diagram(s)sequenceDiagram
participant Script as generate-sitemap.js
participant FS as File System
participant Sitemap as sitemap package
Script->>FS: Ensure public directory exists
Script->>Sitemap: Create SitemapStream with hostname
Script->>Sitemap: Pipe links into SitemapStream
Sitemap-->>Script: Return sitemap XML buffer
Script->>FS: Write sitemap.xml to public directory
Script->>Script: Log success or error
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (3)
generate-sitemap.js
(1 hunks)package.json
(2 hunks)sitemap.xml
(1 hunks)
🔇 Additional comments (2)
package.json (2)
16-16
: LGTM!Script addition is correct and follows existing naming conventions.
68-68
: sitemap dependency is up-to-date and secureVerified that
^8.0.0
is the latest release and no security advisories exist for thesitemap
package.• package.json: line 68
sitemap.xml
Outdated
@@ -0,0 +1 @@ | |||
<?xml version="1.0" encoding="UTF-8"?><urlset xmlns="http://www.sitemaps.org/schemas/sitemap/0.9" xmlns:news="http://www.google.com/schemas/sitemap-news/0.9" xmlns:xhtml="http://www.w3.org/1999/xhtml" xmlns:image="http://www.google.com/schemas/sitemap-image/1.1" xmlns:video="http://www.google.com/schemas/sitemap-video/1.1"><url><loc>https://si-admin-production.web.app/</loc><changefreq>daily</changefreq><priority>1.0</priority></url><url><loc>https://si-admin-production.web.app/about</loc><changefreq>monthly</changefreq><priority>0.7</priority></url></urlset> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Improve XML formatting and verify file location.
The XML is unformatted (single line) making it difficult to read. Consider adding proper indentation and line breaks. Also, the PR description mentions the sitemap should be in the public/
directory, but this file is in the root.
🏁 Script executed:
#!/bin/bash
# Check if public directory exists and verify intended location
fd -t d "public" --max-depth 2
ls -la sitemap.xml 2>/dev/null || echo "sitemap.xml not found in root"
Length of output: 141
Format and relocate sitemap.xml
- Add proper XML formatting: line breaks, indentation for each
<url>
entry, and ensure a trailing newline. - Move
sitemap.xml
from the repo root into the intendedpublic/
directory (e.g.website/public/
oradmin/public/
) to match the PR description.
🤖 Prompt for AI Agents
In sitemap.xml at line 1, the XML content lacks proper formatting such as line
breaks and indentation for each <url> entry, and it is missing a trailing
newline. Reformat the XML to include line breaks and indent each <url> element
for better readability, and add a newline at the end of the file. Additionally,
move the sitemap.xml file from the repository root into the appropriate public/
directory (e.g., website/public/ or admin/public/) as specified in the PR
description to ensure correct placement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
generate-sitemap.js (1)
15-33
: Improve consistency by using async file operations.The code still mixes async/sync patterns. While the try-catch and directory creation improvements are excellent, using
writeFileSync
afterawait
creates inconsistency.Apply this diff to use consistent async operations:
- fs.writeFileSync(outputPath, formattedXml); + await fs.promises.writeFile(outputPath, formattedXml);Additionally, the XML formatting approach on line 25 is quite basic and could potentially break valid XML structure. Consider using a proper XML formatter or removing it if readability isn't critical for automated consumption.
🧹 Nitpick comments (2)
generate-sitemap.js (2)
6-10
: Consider expanding the URL configuration.The current links array only includes two pages. For a production application, consider:
- Adding more relevant pages
- Externalizing this configuration to a separate file
- Adding validation for required properties
25-25
: Simplify or improve XML formatting.The current regex-based formatting
replace(/></g, '>\n<')
is fragile and could break valid XML. Consider either removing it entirely (search engines don't need pretty-printed XML) or using a proper XML formatter.For removal:
- const formattedXml = xml.toString().replace(/></g, '>\n<'); // Add line breaks for readability + const xmlString = xml.toString();Then update the writeFile call accordingly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
generate-sitemap.js
(1 hunks)public/sitemap.xml
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- public/sitemap.xml
🔇 Additional comments (2)
generate-sitemap.js (2)
1-4
: LGTM!Imports are appropriate and complete for the sitemap generation functionality.
12-13
: LGTM!The sitemap stream configuration and output path setup correctly address the previous review feedback about writing to the public/ directory.
Added an automated sitemap generator script using the sitemap library. This script creates a sitemap.xml file in the public/ directory to improve SEO and help search engines crawl the website more effectively.
Changes Made:
Created "generate-sitemap.js" with a list of important URLs.
Updated "package.json" with a new script: "generate-sitemap".
Generated "sitemap.xml" using the correct Firebase hosting URL.
How to Test:
Run "npm run generate-sitemap" and check if "sitemap.xml" is created successfully in the public/ folder.
Summary by CodeRabbit