search

lumeland / lume

🔥 Static site generator for Deno 🦕
https://lume.land
MIT License
1.88k stars 86 forks source link

[sitemap plugin] lastmod is ctime #643

Closed iacore closed 1 month ago

iacore commented 2 months ago

Version

No response

Platform

No response

What steps will reproduce the bug?

import sitemap from "lume/plugins/sitemap.ts";

site.use(sitemap());

How often does it reproduce? Is there a required condition?

100%

What is the expected behavior?

it should be mtime

What do you see instead?

inside sitemap.xml:

<lastmod>(ctime here)</lastmod>

Additional information

data.date is ctime i think

oscarotero commented 2 months ago

It makes sense. date value is used to assign a date to a page, but this date can mean anything. By default Lume uses the creation date, but technically it could be what do you want (you can assign the date you want in the frontmatter).

I this case, I agree that mtime makes more sense, but this value is not available automatically in the page date. Maybe Lume should have both dates, but I'm not completely sure about that.

As a workaround, you can create a preprocessor to obtain this data:

site.preprocess([".html"], (pages) => {
  for (const page of pages) {
    const info = page.src.entry?.getInfo();
    page.data.lastmod = info?.mtime;
  }
});
iacore commented 2 months ago

wouldn't what your proposed be the default for a sitemap? i assume only search engines use read a XML sitemap.

oscarotero commented 2 months ago

lastmod is not a reliable value. Sometimes in some CI environments it's the present time (the moment where the file is created after cloning the repo, instead of when this file content was modified for the last time). Another option is using Git Last Updated value, but this also doesn't work for CI like GitHub Actions, because the repository is cloned without the history (only the last commit).

iacore commented 2 months ago

well that is cursed. you should probably paste this paragraph to https://lume.land/plugins/sitemap/

oscarotero commented 1 month ago

Fair enough. I've updated the documentation including that.