Convert to a js library/extension

[ericholscher]

We should probably remove the sphinx from the name of this extension, because I believe it should work fine with mkdocs. We might just want to name it readthedocs-search or readthedocs-livesearch or something.

[ericholscher]

I guess the integration with the JS is sphinx-specific, but doesn't seem like it needs to be?

[stsewd]

so, mkdocs already does search as you type by default, and we already override that https://github.com/readthedocs/readthedocs.org/blob/master/readthedocs/core/static-src/core/js/doc-embed/search.js#L257-L257

And I guess by renaming you mean repurposing the extension to just be a js library?

[ericholscher]

Is the mkdocs implementation worth stealing, instead of our own code?

I'm not 100% sure what makes sense, it would depend on what we think is best.

[stsewd]

Is the mkdocs implementation worth stealing, instead of our own code?

Like, you mean for sphinx too? (bc it already works for mkdocs p:) The implementation is really simple, last time I checked it does a request per char typed.

If you mean something like "improving" the one from mkdocs, then we will be changing the theme (or we can just port the "debounce" functionality).

This is another reason I'd vote for integrating this into our theme or the sphinx base theme.

[ericholscher]

It seems like the issue is not being able to provide the same UX everywhere, so I feel like we want to have a JS library that provides the same interface regardless of site theme. Integrating it into the theme makes it totally unusable for any non-standard theme, instead of making it really easy for any site to use.

[stsewd]

That's a good point, making it a library makes sense them. I'll take a look how this looks with mkdocs, but first I want to reduce the scope of the functions we have so is easy to move.

[ericholscher]

@stsewd not a huge priority, just something to do when we have time.

[humitos]

I opened https://github.com/readthedocs/meta/discussions/39#discussioncomment-3300752 to discuss something similar to what you've talked here. I'm linking it here so we can find it easily.

It would be good to find some time to prioritize this work now that we are talking about supporting other doctools. By using the SSS API and this Javascript extension we will be able to use it as-is on those other doctools without requiring an integration per doctool 💯

[ericholscher]

@stsewd any estimate on how much work this would be? My initial understanding is not a ton, but it would be useful to have that confirmed by someone who knows the code a bit better.

[stsewd]

@ericholscher I have removed the dependency of underscore from the extension already :D, but it still depends on jquery for the animations, we could bundle the extension with jquery if we want or see if it's easy to replace with vanilla js.

But anyway, I think we could ship something in 2 or 3 days (3 beause I'd love to do #118 before making it as library, and also my limited CSS skills p:). 3 days I'd say in the worst case, I think 2 days should be enough.

[agjohnson]

We should reuse our standard front end scaffolding -- webpack, lessc, and es6 with babel -- to make this a standalone library. Anything that you don't have experience with would be good to hand off, I'm happy to pair on any of this.

[humitos]

I just wanted lo link this tweet here because it's related with this extension somehow and I liked the search modal. Note that search results are shown on a different page, which I think is not the goal of this issue, but it seems other themes are already something similar to what this extension does: https://twitter.com/choldgraf/status/1564530512533807104

[stsewd]

Some ideas for the library, I'm thinking we can put all the logic inside an object that users can use/attach to their projects as they wish. This sphinx extension will still be useful in order to do all that for the user.

const RTDSearch = {
   project: null,
   version: null,
   config: {/* defautls */},

   init: function(project, version, config) {
   },
   show(default_query) {
   },
   close() {
   }
}

The code would still live in this repo, but we can move it any time :)

[agjohnson]

We're a ways out from starting this conversion, but I'd have a lot of thoughts here later.

I'd rather we rather we stick to the patterns already established in a few other repositories -- ext-theme, site-community, doc-diff, and the sphinx theme to some degree. This is the build system/etc, but also an ES6 class structure instead of an object.

I'd also go further and use Webpack's import system for default CSS styling for this, the flyout, and doc-diff is already using this pattern.

But this will take a bit more thought upfront as to the integration points that we'll need to provide, and how we will be surfacing this library/module to documentation builds. We need to start this conversation before I'd take on rebuilding this or hoverxref as a standalone library. These all need to work together eventually, and I'll want a singular structure and workflow for them all.

[stsewd]

I'd rather we rather we stick to the patterns already established in a few other repositories -- ext-theme, site-community, doc-diff, and the sphinx theme to some degree. This is the build system/etc, but also an ES6 class structure instead of an object.

I can take a look at that.

I was proposing doing that since we will be releasing a major version of the extension because of #131, and was thinking we could take that as an opportunity to change other things.

I can write a design doc about how this will look like if you all are good with moving in that direction, otherwise, can also just build on top of what we have and leave the library conversion for later.

[agjohnson]

Makes sense. I'd vote let's punt on the library conversion for now, and we can do that separately. When we have a bit more direction, I can help guide you on a big upgrade here, so that everything is similar to our existing patterns. That way, we aren't making big JS changes twice.

[ericholscher]

That's useful context. I'm fine shipping multiple major versions here, since users should be able to pin them if they want to keep the old version. So we don't need to try and do both changes at once here. 👍

[humitos]

Lot of things have changed in 10 days 😉

I think this work as a Sphinx extension doesn't make sense anymore now. This is going to be useful for any doctool now 💯

I'd propose to archive this repository and continue the work of the search as you type inside the new RTD client. It doesn't make sense to have a different repository for something is going to be used only by the new js client.

The code is not a lot and can be easily migrated to the client. I've started that work already by reusing this exact code.

I'm happy to continue this work from here if you agree with this proposal.

[agjohnson]

I'd agree that this code doesn't need to exist in a separate repository, but would also say that if the current structure of this code isn't yet causing problems with your exploration, we can come back to this in the next iteration of the front end JS work.

@stsewd is actively working on CSS and JS here right now, and upgrading to use APIv3 for search. I'd like to see that work wrapped up first, and would say we're not yet at a place where we can deprecate this repository yet. Time frames seem pretty aligned here though.

[humitos]

Yes. I want that work to be done here before moving forward with my plan.

I'm not saying we have to do what I'm proposing now. I'm just expressing that "making this repository a js library" and/or "keep maintaining a Sphinx extension" don't make sense anymore and we should not keep doing work in those directions.

Once the work Santos started here is done, we should stop there.

[humitos]

but would also say that if the current structure of this code isn't yet causing problems with your exploration, we can come back to this in the next iteration of the front end JS work.

Well, I needed some modifications to this repository to make it work as I needed: https://github.com/readthedocs/readthedocs-sphinx-search/pull/133/files

[humitos]

I just closed #133 because it doesn't make sense anymore. We are going to keep the JavaScript code for this addon in the readthedocs-client repository itself. IMO, eventually, we should archive and stop maintaining this Sphinx extension and move all our effort to the "Search Addon" from the new client. It won't make sense to keep it as a "Sphinx extension" anymore since it will be 100% integrated on Read the Docs without requiring using Sphinx.

In case there are still value that the Sphinx extension offers over the "Search Addon", we should implement those features in the new addon instead of keep maintaining them here.