-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Initial implementation of local search fix #1805
Conversation
This is a first shot at fixing #871. It works by adding a config option to the search plugin which generates an additional search_index.js file. This is simply search_index.json encapsulated in a global variable. This gets embedded into the site, along with a small JavaScript shim, by using the extra_javascript directive.
I was aware of how to work around this, but haven't done so because existing third party themes expect the |
Yeah, I agree, I don't really like the "hackiness" of it, but I did it that
way in order to minimize the amount of edits to existing code. I liked this
approach because it doesn't require any modification to the theme, which
for my use case would be a real pain to keep maintained. In the long run,
it might be better to standardize a global object that contained the search
index and any other useful JavaScript information and require the themes to
ditch the fetch API completely. This just seemed like a nice
low-maintenance fix.
A better way would be to write a custom version of just the search plugin,
that was my initial plan until I found out you can't have a "custom_dir"
for plugins like you can with themes. Once I realized I'd have to make
changes to my mkdocs install directory either way, I went with the easiest
option. I'm certainly open to suggestions though.
…On Tue, Jun 4, 2019, 7:37 PM Waylan Limberg ***@***.***> wrote:
I was aware of how to work around this, but haven't done so because
existing third party themes expect the json file. I hadn't considered
using a shim as a workaround. I'm not sure how I feel about that. Any input
@squidfunk <https://github.com/squidfunk>?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#1805?email_source=notifications&email_token=ACRUJBYMSWOYHSUKM64IXPTPY4DFVA5CNFSM4HTDHKU2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODW6IISI#issuecomment-498893897>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ACRUJB5FERYRYWV2S2AS6TTPY4DFVANCNFSM4HTDHKUQ>
.
|
@waylan @k4kfh In general, I would regard serving documentation locally as an edge case. As the OP suggested, there's no possibility to Regarding the creation of a echo "search_index_shimmed = $(cat site/search/search_index.json)" > site/search/search_index.js Valid JSON is also valid JavaScript. |
@squidfunk @waylan You're right that it could be handled with a batch script, and this would arguably be the easiest way, and wouldn't require any modifications to mkdocs. This was the direction I initially thought of going. However, I'm trying to make this work with existing automated building tools (and have the convenience of still being able to use Perhaps a better solution would be for me to PR a simple plugin for adding pre/post build shell scripts (or additional Python scripts) stored in your docs directory by adding them to the config. Something like: build_scripts:
- pre_build:
- "shellscripts/searchindexshim.sh"
- post_build:
- "shellscripts/otherscript.sh" This would let me (and anyone else) fix the local search problem by just dragging some script files into their docs directory and adding directives to the config file, no need to modify The potential problem with this would be cross-platform support, because if I wrote a bash script, that doesn't always translate 1:1 to a Windows |
As do I. In fact, this is why I haven't bothered to work out a solution. It is not something we aim to support.
Right. Looking at the patch as it stands now, it is incomplete. The extension currently adds a search script to the theme when My expectation has been that while we provide a basic search plugin, someone would come along and implement a better third-party search plugin. I'm inclined to take the position that supporting search over |
Well, having looked into the current design for plugins a little more, I think maybe a good solution would be to allow plugins to be stored within the documentation directory. Something like the Thoughts? |
I disagree. Plugins are Python packages. They belong where Python packages go. As the |
Okay, fair enough. What do you think of a plugin to add shell scripts to the build process? Better idea? Worse idea? |
That is certainly a use-case which many MkDocs users have for various reasons. And any existing plugin events could be used to make calls to shell scripts. There is no need to modify MkDocs to support that. Of course, I expect such a plugin to be implemented and maintained as a third-party plugin. |
I am closing this based on the above reasoning. |
Thanks for providing this. As @squidfunk pointed out, there's no need to change the search plugin. Adding Plus some one-line script to create the @("shim_localSearchIndex = ") + (Get-Content "search_index.json") | Set-Content "search_index.js" |
@k4kfh Why did you keep the JSON file around instead of using the JS file for both local and web? Performance reasons? |
@wilhelmer No particular reason. I guess I just didn't want to change something that didn't need changing. |
I would be interested in this "edge case" @k4kfh ! Have you maybe documented somehwere what you did to completly make mkdocs work with a server-less setup (file:///-url)? I also dont quite managed to get this local search to work. can you maybe share what you all did? a step-by-step-guide for noobies would be perfect :) |
Step-by-step guide:
|
Indeed! 😮 Simply works! Thanks! |
Currently, the mkdocs-localsearch plugin only works with the Material theme, and will stop working with Material when 5.x is released. I had a quick look at the 5.x search implementation, and it seems I'll have to rewrite the code entirely to make it work. I don't know RxJS, so maybe I won't be able to update the plugin at all. @waylan @squidfunk I'd love to reopen this discussion – why not provide the search index as a JS variable (in a seperate JS file) instead of a JSON? This way, no fetch (or While running a production documentation system locally may be an edge case, I think this is also a matter of UX. Right now, if you run Providing the search index via JS file would solve this, with zero downsides. Or are there any? Love to hear your thoughts on this. |
Because it would be backward incompatible. As a policy we always try to provide a graceful deprecation when introducing a backward incompatible change. In other words, for at least one release cycle we add the new way but continue to support the old way with a deprecation warning being issues when people use it. This gives theme devs the opportunity to update as their schedules allow during that release cycle. I looked at doing this when I refactored search into a plugin, but decided against it when I realized it would require a hard break of every third party theme out there. There's not really any way to issue a deprecation warning for this sort of thing. A second reason is that MkDocs does not "officially" support file based browsing (and never has). So it is not high on our priority list to find a workable solution which doesn't isolate all of the existing third party themes. |
For backward compatibility, you could provide something like plugins:
- search:
json_index: true # defaults to false ... and remove that option once most 3rd party themes have adopted to the JS index. |
@wilhelmer integrating this into Material v5 will probably be harder, yes. Search is actually carried out inside a web worker, but the search index is fetched in the main thread, as the main thread does some more work in trying to load pre-serialized indexes and merging them with the payload sent to the web worker. When releasing v5 I'll try to provide some architectural notes which should clear up how things are divided and why. Regardless of that I'll still consider this pretty much an edge case. However, one other option to make it work locally would be to inline the search index into the template as JSON. I'm doing the same for the localizations which are needed by the application logic, see this snippet. You could just shim |
@squidfunk As far as I can tell, v5 uses I'm afraid whichever approach I choose, I won't be able to pull this off on my own. I'm a tech writer, not a developer. Any help would be much appreciated 🙏 |
@wilhelmer I'm actually not sure if this inlining approach is feasible, as the I can look into this when I find some time, which unfortunately will definitely be after the release of v5. As v5 is a complete rewrite, I'll also suspect that there may be some problems/issues with which I/we might have to deal first. You can open an issue on the Material issue tracker for that, maybe somebody else can help out. It would be awesome if you could write up some learnings / directions / ideas, so we can start a discussion how we could maybe support local search. BTW, I'm in favor of |
@squidfunk For an inelegant and despicable, yet pragmatic solution, maybe you could change the if (g_localSearchIndex) {
const data$ = g_localSearchIndex;
} else {
const data$ = ajax({...})
} ... so my plugin can put the search index data in |
@wilhelmer yes, that would be possible, but there might be more things to consider. Let's discuss it in a new issue over at mkdocs-material. Is the README of your project still up to date? I'm asking because #1805 (comment) doesn't match the instructions. |
Yes, the README is up to date. The comment is outdated. |
As an off-topic discussion has continued despite multiple requests to take the discussion elsewhere, I am locking this issue. |
I use mkdocs at work, and we generally just view the site using file:/// urls, so we have been missing the search function since browser security tightened up. This is a simple fix for that (fixes #871). Here is the summary:
By slightly modifying the search plugin in
mkdocs
and adding a small JS file to yourdocs
directory, it's possible to get the search function working in all browsers, even if you open the HTML files directly (a situation which would previously cause cross-site-scripting errors).The Problem:
mkdocs
search feature relies on a client-side JavaScript search library calledlunr.js
. Out of the box, the JavaScript that powers the docs site tries to fetchsearch_index.json
using the HTML5fetch
API. When viewing the docs site locally (i.e. not via a web server), modern browsers block this fetch request because they think it's cross site scripting.The Workaround:
The browser still allows
.js
files to be embedded in HTML (via<script>
tags) when viewing the site from your filesystem. It just doesn't allow JavaScript to fetch files from your filesystem programmatically (for security reasons). However, if we change thesearch_index.json
file tosearch_index.js
, we can turn the JSON file into an executable JavaScript file which sets the search index JSON object as a global variable. See the pseudo-code below:Original
search_index.json
Modified
search_index.js
With these modifications,
search_index.js
can be embedded in mkdocs HTML pages just like any other JavaScript, making the search index accessible from the browser (via a global variable instead of via an HTML5fetch
call).These modifications can be automated by adjusting the
mkdocs
search plugin. The modifications I've made allow you to enable/disable this "shim" from withinmkdocs.yml
, and it still generates the regularsearch_index.json
file as well.The Last of the Fix
The modifications above solve the problem almost completely. However, the theme (in our case, mkdocs-material) is unaware of these changes, so it's still trying to grab the search index by calling the
fetch
function.The solution is to add another short JavaScript to the site which decorates/wraps the
fetch
function. It behaves just like the normal nativefetch
code, UNLESS it detects afile://
URL that ends insearch_index.json
, in which case it "fakes" the expected output of thefetch
command. So no modification to the theme is necessary, because the theme has no idea it's not dealing with the normalfetch
API.This file looks messy, particularly the return statement, but it only modifies
fetch
's behavior in one very specific situation (the only situation that fetch gets used, in our case).I didn't update the documentation yet because I have a feeling that using the
extra_javascript
directive to addsearch_index.js
andfetch_shim.js
is unnecessary, but I couldn't find a programmatic way to do that. Please advise. After that's addressed, I'll be happy to write proper documentation for this modification.