fix(core): let sites override the collection sitemap route - #2385
fix(core): let sites override the collection sitemap route#2385danielmlr wants to merge 1 commit into
Conversation
37e848b guarded the injection of robots.txt and sitemap.xml with hasUserDefinedPublicRoute() so a site's own route file wins, but left /sitemap-[collection].xml unguarded — the one of the three SEO routes that serves content data (every published entry of every SEO-enabled collection, with a public cache header). A site that defines its own sitemap-[collection].xml.ts gets Astro's duplicate-route warning, and which handler answers depends on route-precedence luck; Astro has announced the collision will become a hard error. Apply the same guard to the third route. existsSync treats the square brackets as literal path characters, so the helper needs no change. The existing "skips root SEO routes" test keeps asserting that defining sitemap.xml.ts alone does NOT suppress the collection route; the new test covers the collection-specific override. Verified the new test fails on the unpatched source. Requested in emdash-cms#1089 (use case 3), which 37e848b covered for the other two routes only. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 144a76e The changes in this PR will be included in the next version bump. This PR includes changesets to release 17 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
@emdash-cms/admin
@emdash-cms/auth
@emdash-cms/auth-atproto
@emdash-cms/blocks
@emdash-cms/cloudflare
@emdash-cms/contentful-to-portable-text
emdash
create-emdash
@emdash-cms/gutenberg-to-portable-text
@emdash-cms/plugin-cli
@emdash-cms/plugin-types
@emdash-cms/registry-client
@emdash-cms/registry-lexicons
@emdash-cms/registry-verification
@emdash-cms/sandbox-workerd
@emdash-cms/x402
@emdash-cms/plugin-ai-moderation
@emdash-cms/plugin-atproto
@emdash-cms/plugin-audit-log
@emdash-cms/plugin-color
@emdash-cms/plugin-embeds
@emdash-cms/plugin-field-kit
@emdash-cms/plugin-forms
@emdash-cms/plugin-webhook-notifier
commit: |
There was a problem hiding this comment.
This is the right fix for the right problem, and it follows the established pattern exactly.
#1089 asked for a way to override or disable the injected per-collection sitemap route; #1386 implemented the override guard for robots.txt and sitemap.xml but missed /sitemap-[collection].xml. This PR simply applies the same guard to the remaining route so sites can ship their own sitemap-[collection].xml.ts without Astro's duplicate-route warning. That is consistent with EmDash's convention of backing off when the host site defines a public route file, and the change stays additive (sites without an override see no difference).
What I checked:
- The new condition in
injectCoreRoutesmirrors the two existing guards exactly:!options.srcDir || !hasUserDefinedPublicRoute(options.srcDir, "sitemap-[collection].xml"). hasUserDefinedPublicRoutechecks bothpages/<basename>.<ext>andpages/<basename>/index.<ext>against the literal file path, so square brackets are treated as literal filename characters, which is correct here.- The existing test "skips root SEO routes that are defined by the site" still asserts that overriding
sitemap.xmlalone does not suppress the collection sitemap; the new test covers the collection-specific override and confirms the unaffected routes remain. - Changeset is a proper user-facing patch note for
emdash. - The change touches only build-time route injection; no admin UI strings, no database queries, no localization, no authorization concerns, and no logged-out query-count impact.
I found no issues. The PR is clean and can be approved as-is.
What does this PR do?
#1386 added a
hasUserDefinedPublicRoute()guard so a site's ownrobots.txtandsitemap.xmlroute files suppress the injected defaults — but left/sitemap-[collection].xmlunguarded. It is injected unconditionally, so a site that ships its ownsitemap-[collection].xml.tsgets Astro's duplicate-route warning ("A dynamic SSR route cannot be defined more than once"), and which handler answers rests on route-precedence luck. Astro has announced the collision will become a hard error in a future version.Of the three SEO routes this is also the only one that serves content data (every published entry of every SEO-enabled collection, with a public cache header) — so it is the one a site is most likely to need to replace or close, e.g. a non-public deployment behind auth, or the per-collection customisations described in #1089.
This PR applies the same guard to the third route.
existsSynctreats the square brackets as literal path characters, sohasUserDefinedPublicRoute()needs no change. Sites without their own file see no difference; sites with one now get a clean override instead of a warning and undefined precedence.The existing "skips root SEO routes that are defined by the site" test keeps asserting that a site-defined
sitemap.xml.tsalone does not suppress the collection route; the new test covers the collection-specific override. The new test fails on the unpatched source.Closes #1089 — #1386 implemented its Option C for two of the three routes; this covers the remaining one, which the issue lists as use case 3. With it, all three injected SEO routes can be replaced by shipping a route file, with no
node_modulespatching. (An explicit config option, the issue's Option A, would still be a reasonable follow-up, but is a design decision beyond this fix.)Type of change
Checklist
pnpm typecheckpassespnpm lintpassespnpm testpasses (or targeted tests for my change) —tests/unit/astro/routes.test.ts, 9/9pnpm formathas been runAI-generated code disclosure
Screenshots / test output
New test against the unpatched source (proves it can fail):
With the fix: