getkirby / staticache

Static site performance on demand
MIT License
90 stars 9 forks source link

sitemap.xml throws error #20

Closed jaro-io closed 1 year ago

jaro-io commented 1 year ago

hey again 🐰 since i installed the plugin my sitemap is not available anymore. any ideas what’s happening here?

Screenshot 2023-01-26 at 23 31 47
lukasbestle commented 1 year ago

Do you use a plugin to generate the sitemap? If so, which one?

jaro-io commented 1 year ago

@lukasbestle ah, yes indeed.

fabianmichael/kirby-meta

lukasbestle commented 1 year ago

Thanks for the link. The plugin uses the Kirby page cache, but with a custom data format that is incompatible with page responses that are cached by Kirby:

https://github.com/fabianmichael/kirby-meta/blob/23f2f8fd80991b2ff132c4c54b3f138d5957d1a0/config/routes.php#L41-L50

Kirby stores an array with a html key (= body string) and a response key with response metadata.

The compatibility issue can be solved in either the kirby-meta plugin or in Staticache. However the most robust solution would be one in kirby-meta as it will also be compatible with the core page cache and other caching plugins.

Pinging @fabianmichael.

fabianmichael commented 1 year ago

@lukasbestle I’d be happy to update my plugin, but hit a roadblock. The StatiCache class assumes that the response to generate the file from is a regular page, which can be found by calling $kirby->page(), which is not true for the sitemap. So far it has always been a simple response, but I just tried to use a virtual page instead, to provide the plugin the regular page API. But unfortunately, that means that sitemap.xml is still not considered a child page of $site and thus cannot be found by the page() method. Any idea or does staticache needs to be updated as well?

https://github.com/getkirby/staticache/blob/main/src/StatiCache.php#L106-L107

lukasbestle commented 1 year ago

@fabianmichael You are right, this is something that can only be fixed in Staticache. What we could do is to automatically fall back to Url::to($key['id']) if the page was not found. Not a full fallback because of the language code that won't be respected in this case, but at least for the sitemap use case it should be enough.

fabianmichael commented 1 year ago

@lukasbestle Sounds like a start. I don’t understand the whole things that goes on internally when Kirby renders, caches and serves a page. But as far as I understand, this would do the trick. As soon as staticache is updated, I will update my plugin.

jonasfeige commented 1 year ago

Any ETA on this? Would be great if both plugins could be used together.

lukasbestle commented 1 year ago

@jonasfeige Sorry for the delay. I can't give you an ETA for an official fix, but you could temporarily replace this line with the following:

-       $url  = $page->url($key['language']);
+       $url  = $page?->url($key['language']) ?? Url::to($key['id']);
bnomei commented 1 year ago

@fabianmichael i just added a possible solution here: https://github.com/fabianmichael/kirby-meta/issues/39

fabianmichael commented 1 year ago

I’ve updated my plugin to always use its own cache. That way, the sitemap cannot be served static, but the CMS does not crash any longer (see dd6bdecc5c65237b11f505ddc37179099b0f4a15). As soon as someone confirms me that it works on their setup, I’ll create a new release of the meta plugin.