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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃悰 BUG: Sitemap generation does not work with SSR #3682

Closed
1 task
sebholstein opened this issue Jun 22, 2022 · 12 comments 路 Fixed by #3689
Closed
1 task

馃悰 BUG: Sitemap generation does not work with SSR #3682

sebholstein opened this issue Jun 22, 2022 · 12 comments 路 Fixed by #3689
Assignees
Labels
- P2: has workaround Bug, but has workaround (priority)

Comments

@sebholstein
Copy link

What version of astro are you using?

1.0.0-beta.51

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

nodejs

What package manager are you using?

pnpm

What operating system are you using?

Mac

Describe the Bug

The generated sitemap file looks like this (no entries basically):

<?xml version="1.0" encoding="UTF-8"?><urlset xmlns="http://www.sitemaps.org/schemas/sitemap/0.9"></urlset>

In non-SSR mode, it contains all the entries that I would expect.

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-2uh2aq?file=astro.config.mjs

Participation

  • I am willing to submit a pull request for this issue.
@github-actions github-actions bot added this to Needs Triage in 馃悰 Bug Tracker Jun 22, 2022
@bholmesdev
Copy link
Contributor

Hi @SebastianM! This is a known limitation of our sitemap integration sadly. Since we do not expose the final build output for SSR, we cannot generate a complete sitemap. This is partially by design, since dynamic endpoints like [slug].astro can't be predicted when generating a sitemap.

We should definitely document this on our sitemap integration README though, and/or offer a warning in the console during SSR. Curious what behavior you expected here!

@bholmesdev bholmesdev added - P1: chore Doesn't change code behavior (priority) s1-small labels Jun 22, 2022
@sebholstein
Copy link
Author

Hey @bholmesdev

I tried SSR yesterday for the first time and my first impression was: "why aren't there the purely static sites get stored/generated as files?".
I think it would have a lot of benefits, in particular for better TTFB times - but that's another topic.

What I also thought was, that the sitemap integration would still figure out that there are static pages and add at least these site. From a DX perspective, it seems that my configuration is broken because a sitemap xml gets generated but it's empty.

I think it would also be (theoretical) possible for dynamic pages in SSR runtime mode: if there would be an api to tell the sitemap integration that there's a new page to add to the list, the sitemap could get updated (and maybe also cached afterwards).

@bholmesdev
Copy link
Contributor

Summarizing ideas from thread:

  • log a warning when using sitemap with SSR
  • document the customPages integration option to manually list sitemap pages
  • [future] add static SSR routes to astro:build:done hook output

鈽濓笍 I'll close this issue after addressing the first two!

@bholmesdev bholmesdev self-assigned this Jun 23, 2022
@bholmesdev bholmesdev added - P2: has workaround Bug, but has workaround (priority) and removed - P1: chore Doesn't change code behavior (priority) labels Jun 23, 2022
馃悰 Bug Tracker automation moved this from Needs Triage to Done Jun 23, 2022
@maxcountryman
Copy link

@bholmesdev is there a concrete plan to address the third item or is that only for potential consideration?

My two cents: it's unfortunate to have to manually collate pages if you opt into SSR.

@modelorona
Copy link

If you've got a simple site that's a couple pages + blog posts, assuming you keep your md files in src/content/blog (or similar), you can use the code below:

const __filename = fileURLToPath(import.meta.url);
const __dirname = path.dirname(__filename);
const directoryPath = path.join(__dirname, 'src', 'content', 'blog');
const files = fs.readdirSync(directoryPath);
const siteUrl = 'https://...';
const blogUrls = files.map((file) => {
  const fileName = file.split('.')[0];
  return `${siteUrl}/${fileName}`;
});

which can be used as

sitemap({
  customPages: ['your non blog urls here'].concat(blogUrls),
  priority: 0.5,
  changefreq: 'monthly'
})

@jhedstrom
Copy link

It would be great if the above was added to the documentation! It seems to work for predictable dynamic routes.

@offwork
Copy link

offwork commented Feb 19, 2024

Since the xml nodes of pages created with SSR do not appear in the sitemap, how should we use the sitemap for dynamic pages created with SSR?

@IlirEdis
Copy link

Really?! Just moved to Astro and at the very end of the project i see this limitation. I'd never thought that this would be something which would block my project. Being a framework which focuses on websites and having this limitation is a pity!
Hope you find a solution asap.

@bholmesdev
Copy link
Contributor

Ah, sorry to hear that @IlirEdis! I agree it isn't ideal once you lean into SSR. But we are limited by a fundamental piece here: if a route is a dynamic SSR route, we cannot predict all routes it can render. It could be infinite! Hence the lack of a sitemap. Do you have an idea of the behavior you want to see? Want to hear from @offwork as well.

@IlirEdis
Copy link

Hi @bholmesdev

Since i switched from Nextjs to Astro, there we can create a sitemap.ts file inside the app folder then in this file we can fetch data (in my case) from cms and then map over them:

const Pages = pages?.map((page) => ({
    url: `${baseUrl}/${page.params.pageSlug}`,
  }));

I believe you are aware of this behaviour in Nextjs.
It seems we can't fetch data inside astro.config.mjs so probably a way to fetch the actual rendered routes and return them inside the sitemap() function in config.

@bholmesdev
Copy link
Contributor

@IlirEdis ah ha, I haven't worked with sitemaps in Next.js, so that is good to know. I moved these thoughts to a roadmap discussion started by a community member. I'll see what we can do! withastro/roadmap#906

@IlirEdis
Copy link

@bholmesdev thanks. I appreciate it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P2: has workaround Bug, but has workaround (priority)
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

7 participants