-
Notifications
You must be signed in to change notification settings - Fork 23
feat: create sitemap generator #520
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
base: main
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #520 +/- ##
==========================================
+ Coverage 79.58% 79.97% +0.38%
==========================================
Files 120 127 +7
Lines 12056 12283 +227
Branches 841 866 +25
==========================================
+ Hits 9595 9823 +228
+ Misses 2458 2457 -1
Partials 3 3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Thanks! I know this is a draft, but a few notes that I thought I might share.
These aren't blockers or concerns, just little notes.
|
@araujogui is this ready for review? |
Not yet, I still need to implement: #520 (comment) |
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.
Pull request overview
This PR implements a sitemap.xml generator to improve SEO for Node.js API documentation by ensuring search engines can properly index the latest API pages. This addresses issue #255, which identified that missing sitemaps could cause search engines to show outdated information.
Key changes:
- Created a new sitemap generator that depends on the metadata generator
- Added sitemap template with standard XML structure
- Registered the generator in the public generators list
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/generators/sitemap/index.mjs | Main generator implementation that creates sitemap.xml with URL entries for all API pages and a main API index page |
| src/generators/sitemap/template.xml | XML template for the sitemap structure following the sitemap.org protocol |
| src/generators/index.mjs | Registered the sitemap generator in the publicGenerators export |
Comments suppressed due to low confidence (1)
src/generators/sitemap/index.mjs:85
- The sitemap generator lacks test coverage. Consider adding tests to verify the correct generation of sitemap.xml content, including URL construction, filtering of entries by depth, date formatting, and proper XML structure. Tests should also cover edge cases such as empty entry lists and entries with special characters in URLs.
async generate(entries, { output }) {
const lastmod = new Date().toISOString().split('T')[0];
const apiPages = entries
.filter(entry => entry.heading.depth === 1)
.map(entry => {
const path = entry.api_doc_source.replace(/^doc\//, '/docs/latest/');
const url = new URL(path, BASE_URL).href;
return {
loc: url,
lastmod,
changefreq: 'weekly',
priority: '0.8',
};
});
apiPages.push({
loc: new URL('/docs/latest/api/', BASE_URL).href,
lastmod,
changefreq: 'daily',
priority: '1.0',
});
const template = await readFile(
join(import.meta.dirname, 'template.xml'),
'utf-8'
);
const urlset = apiPages
.map(
page => dedent`
<url>
<loc>${page.loc}</loc>
<lastmod>${page.lastmod}</lastmod>
<changefreq>${page.changefreq}</changefreq>
<priority>${page.priority}</priority>
</url>
`
)
.join('\n');
const sitemap = template.replace('__URLSET__', urlset);
if (output) {
await writeFile(join(output, 'sitemap.xml'), sitemap, 'utf-8');
}
return sitemap;
},
};
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/generators/sitemap/index.mjs
Outdated
| const urlset = apiPages | ||
| .map( | ||
| page => dedent` | ||
| <url> |
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.
nit: use template files for this and then do simple key->value substitution. Or use proper rss/feed libraries OR xml libraries.
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.
Or use proper rss/feed libraries OR xml libraries.
You can probably use hast (but I'm also fine with it this way), seeing as the majority of it is a template
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.
If we use hast, we are probably going to need https://github.com/syntax-tree/hast-util-to-xast too
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.
I'm fine with whatever that uses the least amount of dependencies. We can also just use yet another template file, we can also simply use another dependency, like the xast one, or rss/feeds or whatever.
Description
Create sitemap.xml generator
Validation
Related Issues
Fixes #255
Check List
node --run testand all tests passed.node --run format&node --run lint.