Reason for excluding popstate events with a null state?

[jbms]

I saw that instant navigation ignores popstate events where the state is null:

mkdocs-material/src/assets/javascripts/integrations/instant/index.ts

Line 202 in 5d0cae7

filter(ev => ev.state !== null),

I observed in cases where there is a problem with the sitemap such that instant navigation is skipped for some pages but not all, we can end up with popstate events due to back/forward actions where the state is null. As a result of the filter expression, those events are ignored and the content is not updated even though the URL is.

Removing the filter seemed to fix that issue and not cause any other issues. So far I was only able to reproduce the issue while making some other changes that led to the sitemap URL list being incorrect, so perhaps this issue won't occur in practice, but I was wondering what the reason for the filter is at all.

[squidfunk]

I‘m answering from my cell so pardon the briefness - thanks for all your investigations regarding instant loading! As you may have seen, it’s a very complex topic. To address the open PRs (and questions like in this issue) we‘ll need reproducible examples where instant loading breaks down for each one of those. Without those cases, I’m not able to reason about the proposed changes.

Converting this issue to a discussion, as it is a question.

[jbms]

To reproduce in this repository's own documentation:

• Enable navigation.instant
• mkdocs build
• Manually edit site/sitemap.xml: delete the entry for the getting-started page, leaving everything else alone.
• Serve the documentation from a local webserver
• Open the getting started page (the one removed from the sitemap)
• Click on a link to a different page such as Home that is still in the sitemap.xml. This navigation will happen using instant navigation.
• Press the back button (to return to the getting-started page). The url will change but the page content will remain the same.

[squidfunk]

Thanks for providing the reproduction!

Manually edit site/sitemap.xml: delete the entry for the getting-started page, leaving everything else alone.

This is the part I don't understand - why would you want to do that? As I wrote here, the preprocess function uses knowledge about the structure of the sitemap.xml, which guarantees that:

1. The 1st entry is always the base URL
2. The 2nd entry (if available) is the first page that is not the home page
3. The 3rd entry (if available) is the first anchor on the first page

For this reason, manually editing sitemap.xml is not a use case I find to be of interest for this project. In the end, manually editing artifacts like partials, assets etc. may always yield to cases where some of the business logic or styling breaks down.

[jbms]

That structure is not exactly what I observe (see note below), but in any case it doesn't really matter for the purpose of this question.

Yes, I agree that manually editing sitemap.xml to exclude a page isn't something that you need to support. But that was just the simplest reproduction of this issue I could find.

Given the other issues that may arise (e.g. with the preprocess function as mentioned in #2391) or with different web server configurations/URL rewrite rules/etc., it seems wise to make the instant navigation more robust, so that if for some reason it fails to detect a page as being a candidate for instant navigation, that just leads to normal navigation, rather than e.g. forward/back breaking.

With use_directory_urls = false, the site_url is indeed first but I don't see any anchor links:

<?xml version="1.0" encoding="UTF-8"?>
<urlset xmlns="http://www.sitemaps.org/schemas/sitemap/0.9"><url>
     <loc>https://squidfunk.github.io/mkdocs-material/</loc>
     <lastmod>2021-03-07</lastmod>
     <changefreq>daily</changefreq>
    </url><url>
     <loc>https://squidfunk.github.io/mkdocs-material/getting-started/</loc>
     <lastmod>2021-03-07</lastmod>
     <changefreq>daily</changefreq>
    </url><url>
     <loc>https://squidfunk.github.io/mkdocs-material/creating-your-site/</loc>
     <lastmod>2021-03-07</lastmod>
     <changefreq>daily</changefreq>
    </url><url>
     <loc>https://squidfunk.github.io/mkdocs-material/publishing-your-site/</loc>
     <lastmod>2021-03-07</lastmod>
     <changefreq>daily</changefreq>
    </url><url>
...

With use_directory_urls = true, I see:

<?xml version="1.0" encoding="UTF-8"?>
<urlset xmlns="http://www.sitemaps.org/schemas/sitemap/0.9"><url>
     <loc>https://squidfunk.github.io/mkdocs-material/index.html</loc>
     <lastmod>2021-03-07</lastmod>
     <changefreq>daily</changefreq>
    </url><url>
     <loc>https://squidfunk.github.io/mkdocs-material/getting-started.html</loc>
     <lastmod>2021-03-07</lastmod>
     <changefreq>daily</changefreq>
    </url><url>
     <loc>https://squidfunk.github.io/mkdocs-material/creating-your-site.html</loc>
     <lastmod>2021-03-07</lastmod>
     <changefreq>daily</changefreq>
    </url><url>
     <loc>https://squidfunk.github.io/mkdocs-material/publishing-your-site.html</loc>
     <lastmod>2021-03-07</lastmod>
     <changefreq>daily</changefreq>
    </url><url>
...

[squidfunk]

To answer the initial question: popstate events with null states happen, when pushing the back button after using the table of contents, i.e. anchor navigation. There's no need to trigger instant loading when going back to a previous anchor on the same page, which is why they are filtered and effectively ignored.

[jbms]

It appears that even with the ev.state !== null filter removed, the distinctUntilKeyChanges("pathname") filter already handles that case.

[squidfunk]

That's correct for the case of fetching the document, but the problem is that the pop$ observable will now trigger, which will break scroll restoration for anchor-only navigation. To reproduce, comment out the line which filters for ev.state !== null, open a page, click some anchors in the table of contents, and then use the back button. Scroll restoration is broken.

Besides fixing bugs on instant loading, I'm currently not considering refactoring it. It works very well. I will address the other issues you reported, but manually editing sitemap is definitely a non-issue.