-
Notifications
You must be signed in to change notification settings - Fork 51
Add trailing slash to site_url #251
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
base: master
Are you sure you want to change the base?
Add trailing slash to site_url #251
Conversation
|
Thanks for reporting this. As far as I can tell, both MkDocs and the built-in theme support in mike handle this properly. What theme are you using? From this bit: it looks like you might be using the Material theme, so in that case, I think this might be a bug over there. |
|
It's a chain of issues. It starts with this PR. Then, the config that mike generates is passed to mkdocs: The base_url is set directly to site_url for error pages, so this issues only happens with those. Then, in material for mkdocs, the base_url is embedded as is into a script config inside the base template (it has the id and a version controlled js-bundle is also included in that base template: That bundle contains the following snippets of javascript, executed chronologically: var Ca=R("#__config")
St=JSON.parse(Ca.textContent)
function xe(){return St}
t=xe()
r=je(new URL("../versions.json",t.base))As you can see, it calls I'm not sure where I should go next. I could go to mkdocs, and file a PR that adds a trailing slash there (this line), or I could go to material for mkdocs and file a PR there, that adds a trailing slash to What do you recommend? I reckoned that this is the appropriate place to file the PR |
|
@cubernetes This issue could come up without mike being involved (e.g. if an MkDocs user manually set |
|
Hm, I'm not entirely sure, because if I have a simple mkdocs.yaml: ...
site_url: http://localhost:8080/some/path
theme:
name: material
...Then the 404 page always has However, as soon as I use |
|
Oh, I see. The MkDocs validation runs earlier than I thought, and the I'd still lean towards this being a bug in MkDocs, since it would probably be more robust to append the trailing slash to the |
|
Awesome, thanks a lot! |
`site_url` has to end with a trailing slash, because when MkDocs processes the Jinja2 templates, it will embed a script tag into the final html:
```js
<script id="__config" type="application/json">{"base": "/some/path/4.1.1", ...}</script>
```
The JS-relevant part of mike then handles this as follows
`fetch(ABS_BASE_URL + "../versions.json")`
or, if you look at the actual js-bundle:
`new URL("../versions.json", t.base)`
And if `t.base` doesn't end with a trailing slash, the last component is completely ignored, which is obviously not what we want.
896f62a to
ff13fbe
Compare
|
FYI: I added slashes to the unit tests, I think you have to approve the workflow once more if you're interested in the test outputs now, unless you're waiting for answers in the mkdocs issue |
site_urlhas to end with a trailing slash, because when MkDocs processes the Jinja2 templates, it will embed a script tag into the final html:The JS-relevant part of mike then handles this as follows
fetch(ABS_BASE_URL + "../versions.json")or, if you look at the actual js-bundle:
new URL("../versions.json", t.base)And if
t.basedoesn't end with a trailing slash, the last component is completely ignored, which is obviously not what we want:Note that I'm not well versed in the MkDocs/mike codebases, I might've overlooked something, but my gut feeling says this might be correct