Skip to content

Conversation

@griest024
Copy link
Member

PR Checklist

  • Commit message follows our contributing guidelines
  • Tests added/updated (for bug fixes/features)
  • Documentation added/updated (for bug fixes/features)

PR Type

  • Bug fix
  • Feature
  • Style update
  • Refactor
  • Test
  • Build
  • CI
  • Docs
  • Performance
  • Other (please describe)

Current behavior

Fixes: #4094
Part of: #

New behavior

Breaking change?

  • Yes
  • No

Additional context

@griest024 griest024 requested review from a team as code owners October 30, 2025 19:37
@@ -0,0 +1,2 @@
Sitemap: https://next.daff.io/sitemapindex.xml
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extract the robots.txt out to another PR

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done, coming after this is merged

resolve,
join,
} from 'node:path';
import { create } from 'xmlbuilder2';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was looking at the depgraph of xmlbuilder2. It looks like it has a few more deps. It also appears slower than https://github.com/NaturalIntelligence/fast-xml-parser/ according to their benchmarks (I had to run these manually).

Do you think we should write to use fast-xml-parser? I don't know if you already evaluated this.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't. Given that this is a dev only dep, I didn't put that much thought into it.

const sitemap = options.sitemaps[name];
if ('prerenderedRoutes' in sitemap) {
try {
const routes = Object.keys(JSON.parse(await readFile(join(PROJECT_ROOT, <string>sitemap.prerenderedRoutes), 'utf-8')).routes);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Write a test for this builder.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@griest024
Copy link
Member Author

@damienwebdev this needs a re-review

@griest024
Copy link
Member Author

@damienwebdev bump

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

daffio sitemap

2 participants