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

fix(search): searchDataURL must not use relative URL #505

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rluders
Copy link

@rluders rluders commented Dec 1, 2022

This basically allows the flexsearch to work when you have a suffix at your Hugo hosting, like: https://<user>.pages.domain.com/<repository>

It always will load the data using the $searchData.Permalink instead using the $searchData.RelPermalink.

Fixes #363

unbing pushed a commit to unbing/hugo-book that referenced this pull request Feb 13, 2023
There's already pending PR (alex-shpak#505),
but this isn't approved somehow. Creating my own just for easier tracking.
@unbing
Copy link

unbing commented Feb 13, 2023

Looks like we need to update search-data.js as well. Otherwise, we cannot click the outcome of the search results (href is broken).

@vmercierfr
Copy link

We have a similar issue with publishing our documentation on GitHub pages.
As @unbing mentioned, line 11 in search-data.js must be updated as well to fix search results.

As a workaround, we are using the following patch:

--- themes/book/assets/search.js        2023-11-23 16:41:11
+++ themes/book/assets/search.js        2023-11-23 16:41:08
@@ -5,7 +5,7 @@
 {{ $searchConfig := i18n "bookSearchConfig" | default "{}" }}

 (function () {
-  const searchDataURL = '{{ $searchData.RelPermalink }}';
+  const searchDataURL = '{{ $searchData.Permalink }}';
   const indexConfig = Object.assign({{ $searchConfig }}, {
     doc: {
       id: 'id',
--- themes/book/assets/search-data.json 2023-11-23 17:13:03
+++ themes/book/assets/search-data.json 2023-11-23 17:13:08
@@ -8,7 +8,7 @@
 {{ range $index, $page := $pages }}
 {{ if gt $index 0}},{{end}} {
     "id": {{ $index }},
-    "href": "{{ $page.RelPermalink }}",
+    "href": "{{ $page.Permalink }}",
     "title": {{ (partial "docs/title" $page) | jsonify }},
     "section": {{ (partial "docs/title" $page.Parent) | jsonify }},
     "content": {{ $page.Plain | jsonify }}

And running

 patch -p0 < patch_book.txt

@rluders can you update the PR to apply changes on search-data.json?

@iyear
Copy link

iyear commented Dec 1, 2023

Any news about it?

@rluders
Copy link
Author

rluders commented Dec 1, 2023

Someone can take this PR, adjust it, and create a new PR with the update. I'm a little bit busy to get it rebased and adjusted now.

@alex-shpak
Copy link
Owner

alex-shpak commented Dec 4, 2023

Hello! Sorry for a long waiting.
This is hard choice, the Permalink vs RelPermalink discussions have sparkled before, also regarding URL for different languages as that could be on other subdomains.
I feel like hardcoding domain into theme output is not ideal, so could you show me some example of wringly generated URLs so I can take a look?

Changing this will likely cause someone else to come and ask to change it back :)

@millionhz
Copy link

@alex-shpak see #566 (comment)

@millionhz
Copy link

So after a bit of research, I have found that .RelPermalink won't work in javascript templating because:

  1. .RelPermalink required prefixes like src= and href= etc, as mentioned in relativeURLs setting does not work for every URL gohugoio/hugo#8734 (comment)
  2. .RelPermalink with the appropriate prefix fails to generate the proper relative link in the javascript template. I figured this out through trial and error. This mentions post-processing, which may be a relevant reason, too.
  3. the relurl function with the slashes trimmed does not work in the javascript template either. (it's like our javascript file does not know what the baseURL is)

Having that said:

  • .Permalink seems like the only viable option for now
  • we can wait for Hugo to introduce a dedicated URL post-processing functions for these kinds of use cases as suggested here
  • We can refactor the serach.js and search-data.json files so that there is no need to call .RelPermalink in them. It will cost readability and modularity, but it will fix the problem

@alex-shpak
Copy link
Owner

Thanks for spending time on the research, so far I always tried to use only .RelPermalink and similar logic everywhere, with goal in mind that docs site could be compiled and read from filesystem, but perhaps use-case for that is very low and I can reconsider that.

Option perhaps to have a parameter to switch RelPermalink and Permalink in critical places, like search or multi-lang links 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

flexsearch ignores baseURL path leading to 404s
6 participants