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

Replace middleman-search with lunr with Typesense DocSearch #702

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

Conversation

tnir
Copy link
Collaborator

@tnir tnir commented Jul 18, 2022

What was the end-user problem that led to this PR?

As middleman-search (RubyGems, GitHub) has not been maintained for more than 5.5 years (for example manastech/middleman-search#38), bundler-site maintainers/contributors cannot upgrade lunr to the latest without their additional efforts (or other community's efforts than maintainers/contributors of this repo).

Closes #691

What was your diagnosis of the problem?

UI can be replaced by https://github.com/algolia/docsearch or its forks. Backend (current /search/lunr-index.json) can be replaced by Algolia DocSearch with automated crawling or Typesense cloud with manual crawling by us.

What is your fix for the problem, implemented in this PR?

  • Removes middleman-search gem and relevant gems (mini_racer)
  • Removes JS/CSS codes and Middleman configuration for middleman-search
  • Removes (the load of) Popover CSS from Bootstrap 5 along with the above JS code removal.
  • Also the load of /application.min.js is moved to outside of <head>.

A crawler (which is expected to run once a day in production) is currently run by my local.

A cloud project on Typesense cloud called k0cw8zgj4i592lsqp-1.a1.typesense.net is used.

Why did you choose this fix out of the possible options?

If Typesense offers us to some tiny instance at no cost and the permission to the UI, we might be able to use this change instead of Algolia's DocSearch.

Signed-off-by: Takuya Noguchi [email protected]

@deivid-rodriguez deivid-rodriguez temporarily deployed to bundler-site-tnir-types-qv0qbj July 18, 2022 07:27 Inactive
@tnir tnir force-pushed the tnir-typesense-docsearch-691 branch from 83a8174 to 57cbf49 Compare July 18, 2022 10:16
@deivid-rodriguez deivid-rodriguez temporarily deployed to bundler-site-tnir-types-qv0qbj July 18, 2022 10:16 Inactive
@olleolleolle
Copy link
Member

So nice to get rid of the v8 too!

@deivid-rodriguez
Copy link
Member

Algolia alternative seems easier for now but I have to say I find this UI very nice looking.

@tnir tnir mentioned this pull request Jul 22, 2022
6 tasks
@tnir tnir force-pushed the tnir-typesense-docsearch-691 branch 2 times, most recently from 0b2e6e1 to b4698e4 Compare July 23, 2022 03:51
@deivid-rodriguez deivid-rodriguez temporarily deployed to bundler-site-tnir-types-qv0qbj July 23, 2022 03:51 Inactive
@tnir tnir force-pushed the tnir-typesense-docsearch-691 branch from b4698e4 to adfecb6 Compare July 23, 2022 04:01
@deivid-rodriguez deivid-rodriguez temporarily deployed to bundler-site-tnir-types-qv0qbj July 23, 2022 04:02 Inactive
@tnir tnir force-pushed the tnir-typesense-docsearch-691 branch from adfecb6 to fcfe5a0 Compare July 23, 2022 05:01
@deivid-rodriguez deivid-rodriguez temporarily deployed to bundler-site-tnir-types-qv0qbj July 23, 2022 05:01 Inactive
@tnir tnir marked this pull request as ready for review July 23, 2022 05:03
@deivid-rodriguez
Copy link
Member

This looks fine performance wise, and the UI seems nice too. But the presence of a logo is a blocker and the majority of maintainers seem to prefer to take over middleman-search maintenance?

@simi
Copy link
Member

simi commented Jul 23, 2022

This looks fine performance wise, and the UI seems nice too. But the presence of a logo is a blocker and the majority of maintainers seem to prefer to take over middleman-search maintenance?

indeed, let's focus on that one

@tnir
Copy link
Collaborator Author

tnir commented Jul 23, 2022

Features, look and feel are now all set. A small tweak in UI was submitted as a PR to the upstream, which is however NOT blocked to get this merged.

Any feedback is welcome again.

@deivid-rodriguez
Copy link
Member

@tnir You plan is unclear to me, since we don't have permission to go ahead without a logo. Everybody agrees to keep middleman-search for now.

@tnir
Copy link
Collaborator Author

tnir commented Jul 23, 2022

Just my previous comment was delayed. Just send back to draft at the moment while not sure this can be usable in the future.

@tnir tnir marked this pull request as draft July 23, 2022 23:43
@tnir tnir removed the request for review from deivid-rodriguez July 23, 2022 23:44
@tnir tnir added this to the Architecture overhaul milestone Jul 23, 2022
@deivid-rodriguez
Copy link
Member

Ah, I see, makes sense now if comments are read in the right order :)

@tnir
Copy link
Collaborator Author

tnir commented Jul 24, 2022

By the way, another problem underlies in assets/javascripts/search.js. The hard way.

@deivid-rodriguez
Copy link
Member

I understand, but for now it seems our best option is to maintain these ~200 lines of JavaScript and this gem for searching...

@deivid-rodriguez deivid-rodriguez temporarily deployed to bundler-site-tnir-types-yfuaot July 29, 2022 06:36 Inactive
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.

meta: Replace lunr (middleman-search) with DocSearch
4 participants