Skip to content
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

@astrojs/sitemap 1.3.1 adds endpoints with trailing slash #7080

Closed
1 task
fflaten opened this issue May 12, 2023 · 4 comments · Fixed by #7656
Closed
1 task

@astrojs/sitemap 1.3.1 adds endpoints with trailing slash #7080

fflaten opened this issue May 12, 2023 · 4 comments · Fixed by #7656
Labels
- P4: important Violate documented behavior or significantly impacts performance (priority)

Comments

@fflaten
Copy link
Contributor

fflaten commented May 12, 2023

What version of astro are you using?

2.4.5

Are you using an SSR adapter? If so, which one?

None

What package manager are you using?

npm

What operating system are you using?

Linux

What browser are you using?

Edge

Describe the Bug

Starting with astrojs/sitemap 1.3.0, endpoints like rss.xml are included in sitemap-0.xml.
Should sitemap include endpoints (data, images, rss etc.) at all?

If so, the trailing slash needs to be trimmed as the URLs are invalid.
<url><loc>https://example.com/rss.xml/</loc></url>

Repro: Run build in stackblitz sample and inspect sitemap-0.xml

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-xfdxg4

Participation

  • I am willing to submit a pull request for this issue.
@fflaten
Copy link
Contributor Author

fflaten commented Jun 2, 2023

Bonus: it doesn't add dynamic endpoints, only static. Still in 1.3.3
Ex.

  • rss.xml.ts -> included as rss.xml. Ok, but should users really have to filter it?
  • social.png.ts -> included as social.png. Ok, but should it be there?
  • [...slug].png.ts -> missing. Good imo, but still inconsistent with above

@ematipico ematipico added the - P4: important Violate documented behavior or significantly impacts performance (priority) label Jun 12, 2023
@Oceaneyes123
Copy link

Hey, there's now a way to configure this not to include the trailing slash. You can configure your astro.config.mjs:

import { defineConfig } from 'astro/config'

export default defineConfig({
    trailingSlash: 'never'
})

For more information: https://docs.astro.build/en/reference/configuration-reference/#trailingslash

@fflaten
Copy link
Contributor Author

fflaten commented Jun 24, 2023

Unfortunately that's not the answer. trailingSlash is used to match dev server's behavior to your production host so you can catch errors in links etc. It does affect the URL style used in sitemap, but it's all-or-nothing option and trailing slash is always wrong for file-endpoints like rss.xml.

In my case I'd have to use trailingSlash: 'always' to match my host's behavior while using build.format = 'directory', but the sitemap will then also generate ../rss.xml/ (this issue). A request to that URL will look for .../rss.xml/index.html instead of the .../rss.xml file and return 404

@natemoo-re
Copy link
Member

Hey sorry it's taken us a while to get to this one! I'm working on this now.

natemoo-re added a commit that referenced this issue Jul 17, 2023
* fix(#7080): sitemap should only add trailing slash to pages

* fix(sitemap): only include pages in sitemap

* chore: add test

* chore: remove unused import

* docs: update readme
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P4: important Violate documented behavior or significantly impacts performance (priority)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants
@ematipico @fflaten @natemoo-re @Oceaneyes123 and others