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

Version switcher lately never succeeds at staying on the same page #7160

Closed
2 of 4 tasks
ilyagr opened this issue May 7, 2024 · 15 comments
Closed
2 of 4 tasks

Version switcher lately never succeeds at staying on the same page #7160

ilyagr opened this issue May 7, 2024 · 15 comments
Labels
bug Issue reports a bug resolved Issue is resolved, yet unreleased if open

Comments

@ilyagr
Copy link
Contributor

ilyagr commented May 7, 2024

Bug description

(Update: This reproduction no longer works -- or rather, there is no bug, everything works as intended -- after I downgraded that website to be compiled with the old 9.5.4 version of mkdocs-material. It should be possible to reproduce this with https://github.com/squidfunk/mkdocs-material-example-versioning, but it requires recompiling that website with a recent version of mkdocs-material)

  1. Go to https://martinvonz.github.io/jj/prerelease/config/
  2. Use the version switcher to switch to the "v0.17.0" version

Observed: you end up at https://martinvonz.github.io/jj/v0.17.0/

Desired behavior: you should end up at https://martinvonz.github.io/jj/v0.17.0/config/

Context

I can flesh out this bug report if desired, but I believe I found the likely culprit to be a628293, specifically the changes to the fetchSitemap and extract functions.

After that commit, something clearly wrong is happening in:

const location = getLocation()
const path = location.href.replace(config.base, "")
return sitemap.has(path.split("#")[0])
? new URL(`../${version}/${path}`, config.base)
: new URL(url)

As I debug, with the reproduction I describe below, I find that sitemap is a Map object with value (as abbreviated by the Firefox debugger):


Map(31) {
"https://martinvonz.github.io/jj/v0.17.0/" → (1) [...],
"https://martinvonz.github.io/jj/v0.17.0/FAQ/" → (1) [...],
"https://martinvonz.github.io/jj/v0.17.0/branches/" → (1) [...],
... }

​```

Meanwhile, path.split("#")[0] has the value "config/". So, the operation sitemap.has(path.split("#")[0]) cannot evaluate to true.

Related links

https://squidfunk.github.io/mkdocs-material/setup/setting-up-versioning/?h=version#stay-on-the-same-page-when-switching-versions

Reproduction

See description

Steps to reproduce

See description

Browser

Firefox 125.0.3 on Mac OS Sonoma

Before submitting

ilyagr added a commit to ilyagr/jj that referenced this issue May 7, 2024
I'm hoping this will fix version switching.

squidfunk/mkdocs-material#7160
@ilyagr ilyagr changed the title The functionality that tries to stay on the same page when using the version switcher is broken Staying on the same page when using the version switcher is broken May 7, 2024
@ilyagr ilyagr changed the title Staying on the same page when using the version switcher is broken Staying on the same page when using the version switcher is broken lately May 7, 2024
@ilyagr ilyagr changed the title Staying on the same page when using the version switcher is broken lately Version switcher lately never succeeds at staying on the same page May 7, 2024
@squidfunk
Copy link
Owner

Thanks for reporting. Since you already digged into possible causes, would you like to create a PR? Happy to collaborate in gravitating towards a solution, and I agree that it's likely related to the latest refactoring.

@squidfunk squidfunk added the bug Issue reports a bug label May 7, 2024
@ilyagr
Copy link
Contributor Author

ilyagr commented May 7, 2024

What's a good way to set up a dev environment for the theme, especially one where I can use the version switcher?

@ilyagr
Copy link
Contributor Author

ilyagr commented May 7, 2024

I was hoping https://github.com/squidfunk/mkdocs-material/blob/master/CONTRIBUTING.md would link to an explanation, but it seems to link to nonexistent pages.

@ilyagr
Copy link
Contributor Author

ilyagr commented May 7, 2024

Ah, I found https://squidfunk.github.io/mkdocs-material/customization/#environment-setup. I'll let you know if I have questions after reading that.

@ilyagr
Copy link
Contributor Author

ilyagr commented May 7, 2024

Setting up something with a version switcher seems tricky. I guess I can follow the instructions and then try to make my own project (with my documentation, managed with Poetry) depend on mkdocs-material from the git cloned directory. Then I can try to run mike and see whether things work.

@squidfunk
Copy link
Owner

Jup, correct. Let me know if you have troubles. I think you can use mike serve, or deploy for testing on a new repo. I recommend to just use pip and install the bare minimum. You could also work off our versioning example (that was built with a prior version), and work with that.

@ilyagr
Copy link
Contributor Author

ilyagr commented May 7, 2024

It does seem that downgrading mkdocs-material to 9.5.4 helps. Unfortunately, that version claims to be incompatible with the new MkDocs 1.6.

I will hopefully get to working on fixing this... at some point.

@kamilkrzyskow
Copy link
Collaborator

Perhaps you could try checking out my hunch about the URLs that are stored in the sitemap:

However, I see that your site has different URLs in the sitemap, so the hunch is likely incorrect 🤔
It properly stores the current /version/ in the URLs, instead of reusing a static one.

This "shadow" issue has been here for quite some time, good that someone is willing to properly debug it 💪

@squidfunk
Copy link
Owner

Thanks @ilyagr and @kamilkrzyskow! I'll tag this issue with "needs reproduction", because none was provided. To go forward, we definitely need a minimal reproduction, because we need something to test a potential fix against ☺️ Also, as said, if somebody makes progress, please open a PR. I'm happy to assist. If nobody is up to it, I'll try to find some time asap.

@squidfunk squidfunk added the needs reproduction Issue lacks a minimal reproduction .zip file label May 8, 2024
@ilyagr
Copy link
Contributor Author

ilyagr commented May 8, 2024

One way to create a reproduction is to add a GitHub action to https://github.com/squidfunk/mkdocs-material-example-versioning to compile a "nightly" version of its docs nightly. That would show the problem, among with any potential future problems. This is on the list of my "might get to at some point" things to do.

@ilyagr
Copy link
Contributor Author

ilyagr commented May 8, 2024

The procedure in https://squidfunk.github.io/mkdocs-material/guides/creating-a-reproduction/ is not very good for this case, since it doesn't involve mike. Without mike, compiled docs don't even include a visible version switcher.

@squidfunk
Copy link
Owner

The reproduction still allows us to run mike with some instructions to observe the issue.

@squidfunk
Copy link
Owner

Fixed in ceacfe5. This turned out to be a regression in 9.5.5 where we refactored the sitemap loading logic, which was necessary to get instant navigation right. My testing shows that the issue is fixed now, and we redeployed our versioning example which shows that it works.

@squidfunk squidfunk added resolved Issue is resolved, yet unreleased if open and removed needs reproduction Issue lacks a minimal reproduction .zip file labels May 12, 2024
@squidfunk
Copy link
Owner

Released as part of 9.5.22.

@ilyagr
Copy link
Contributor Author

ilyagr commented May 12, 2024

Thank you very much, I appreciate it! I'll try it out in a bit, will let you know if it fails for me, but it looks like it should be fine.

I was thinking of making the functionality a bit more robust by parsing the UR in the sitemap, ignoring the domain name, and only looking at the directory path. Then, for example, the version switcher from https://squidfunk.github.io/mkdocs-material-example-versioning/latest/ would work if I deployed it at https://ilyagr.github.io/mkdocs-material-example-versioning/latest/ without recompiling. I was then hoping for maybe eventually adjusting the behavior of mike serve so that the version switcher worked there too.

If that sounds at all reasonable, you might see a PR from me yet, or feel free to borrow the idea and run with it.

Thank you again!

ilyagr added a commit to ilyagr/jj that referenced this issue May 12, 2024
squidfunk/mkdocs-material#7160 is supposed to
be fixed now. I will double-check that the version switcher still
works after this commit. The functionality is a bit brittle, so it's
hard to test locally before merging this.

We do want to upgrade, since this will allow us to upgrade to MkDocs
1.6, which has a nice feature of disallowing links to broken document
section (beyond just checking for links to non-existent files).

I carefully avoided unnecessary upgrades in this commit, but we
should also run `poetry update` shortly, once we check this
works properly.
ilyagr added a commit to ilyagr/jj that referenced this issue May 12, 2024
squidfunk/mkdocs-material#7160 is supposed to
be fixed now. I will double-check that the version switcher still
works after this commit. The functionality is a bit brittle, so it's
hard to test locally before merging this.

We do want to upgrade, since this allows us (and seems to also require
us) to upgrade to MkDocs 1.6, which has a nice feature of disallowing
links to broken document section (beyond just checking for links to
non-existent files).

I carefully avoided unnecessary upgrades in this commit, but we
should also run `poetry update` shortly, once we check this
works properly. Also, the feature of MkDocs 1.6 I mentioned needs
enabling.
ilyagr added a commit to ilyagr/jj that referenced this issue May 12, 2024
squidfunk/mkdocs-material#7160 is supposed to
be fixed now. I will double-check that the version switcher still
works after this commit. The functionality is a bit brittle, so it's
hard to test locally before merging this.

We do want to upgrade, since this allows us (and seems to also require
us) to upgrade to MkDocs 1.6, which has a nice feature of disallowing
links to broken document section (beyond just checking for links to
non-existent files).

I carefully avoided unnecessary upgrades in this commit, but we
should also run `poetry update` shortly, once we check this
works properly. Also, the feature of MkDocs 1.6 I mentioned needs
enabling.

Finally, I plan to change version specifiers from Poetry's `^5.6.22`
syntax to more plain inequalities, since I keep forgetting what the
former syntax means. I started with this commit.
ilyagr added a commit to ilyagr/jj that referenced this issue May 12, 2024
squidfunk/mkdocs-material#7160 is supposed to
be fixed now. I will double-check that the version switcher still works
after this commit. This only affects the version switcher from
"prerelease" to other versions.. The functionality is a bit brittle, so
it's hard to test locally before merging this.

We do want to upgrade, since this allows us (and seems to also require
us) to upgrade to MkDocs 1.6, which has a nice feature of disallowing
links to broken document section (beyond just checking for links to
non-existent files).

I carefully avoided unnecessary upgrades in this commit, but we
should also run `poetry update` shortly, once we check this
works properly. Also, the feature of MkDocs 1.6 I mentioned needs
enabling.

Finally, I plan to change version specifiers from Poetry's `^5.6.22`
syntax to more plain inequalities, since I keep forgetting what the
former syntax means. I started with this commit.
ilyagr added a commit to ilyagr/jj that referenced this issue May 12, 2024
squidfunk/mkdocs-material#7160 is supposed to
be fixed now. I will double-check that the version switcher still works
after this commit. This only affects the version switcher from
"prerelease" to other versions.. The functionality is a bit brittle, so
it's hard to test locally before merging this.

We do want to upgrade, since this allows us (and seems to also require
us) to upgrade to MkDocs 1.6, which has a nice feature of disallowing
links to broken document section (beyond just checking for links to
non-existent files).

I carefully avoided unnecessary upgrades in this commit, but we
should also run `poetry update` shortly, once we check this
works properly. Also, the feature of MkDocs 1.6 I mentioned needs
enabling.

Finally, I plan to change version specifiers from Poetry's `^5.6.22`
syntax to more plain inequalities, since I keep forgetting what the
former syntax means. I started with this commit.

(I was surprised MkDocs was allowed to be upgraded with the ^1.5.2 spec.
I now suspect that what I previously thought it should mean is expressed
as ~1.5.2, but inequalities are clearer).
ilyagr added a commit to ilyagr/jj that referenced this issue May 12, 2024
squidfunk/mkdocs-material#7160 is supposed to
be fixed now. I will double-check that the version switcher still works
after this commit. This only affects the version switcher from
"prerelease" to other versions.. The functionality is a bit brittle, so
it's hard to test locally before merging this.

We do want to upgrade, since this allows us (and seems to also require
us) to upgrade to MkDocs 1.6, which has a nice feature of disallowing
links to broken document section (beyond just checking for links to
non-existent files).

I carefully avoided unnecessary upgrades in this commit, but we
should also run `poetry update` shortly, once we check this
works properly. Also, the feature of MkDocs 1.6 I mentioned needs
enabling.

Finally, I plan to change version specifiers from Poetry's `^5.6.22`
syntax to more plain inequalities, since I keep forgetting what the
former syntax means. I started with this commit.

(I was surprised MkDocs was allowed to be upgraded to `1.6` with the
`^1.5.2` spec.  I now suspect that what I previously thought it should
mean is expressed as `~1.5.2`, but inequalities are clearer).
ilyagr added a commit to martinvonz/jj that referenced this issue May 13, 2024
squidfunk/mkdocs-material#7160 is supposed to
be fixed now. I will double-check that the version switcher still works
after this commit. This only affects the version switcher from
"prerelease" to other versions.. The functionality is a bit brittle, so
it's hard to test locally before merging this.

We do want to upgrade, since this allows us (and seems to also require
us) to upgrade to MkDocs 1.6, which has a nice feature of disallowing
links to broken document section (beyond just checking for links to
non-existent files).

I carefully avoided unnecessary upgrades in this commit, but we
should also run `poetry update` shortly, once we check this
works properly. Also, the feature of MkDocs 1.6 I mentioned needs
enabling.

Finally, I plan to change version specifiers from Poetry's `^5.6.22`
syntax to more plain inequalities, since I keep forgetting what the
former syntax means. I started with this commit.

(I was surprised MkDocs was allowed to be upgraded to `1.6` with the
`^1.5.2` spec.  I now suspect that what I previously thought it should
mean is expressed as `~1.5.2`, but inequalities are clearer).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue reports a bug resolved Issue is resolved, yet unreleased if open
Projects
None yet
Development

No branches or pull requests

3 participants