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

Add Algolia DocSearch plugin #1168

Merged
merged 42 commits into from
Nov 29, 2023
Merged

Add Algolia DocSearch plugin #1168

merged 42 commits into from
Nov 29, 2023

Conversation

delucis
Copy link
Member

@delucis delucis commented Nov 29, 2023

Description

  • Adds our first official plugin — for Algolia DocSearch.
  • Adds docs for site search in Starlight, including how to add this new plugin.
  • WIP because depends on several of our inflight PRs. Currently this is built on @HiDeoo’s Initial Starlight Plugins support #942 branch so will show many unrelated changes. I will rebase once that is merged.

HiDeoo and others added 25 commits October 20, 2023 12:12
* main: (103 commits)
  i18n(fr): update `guides/sidebar.mdx` (#1033)
  i18n(fr): update `reference/configuration.mdx` (#1034)
  i18n(fr): update `reference/frontmatter.md` (#1035)
  Fix docs component details (#1031)
  Overhaul getting started guide (#1026)
  i18n(zh-cn): Update sidebar.mdx & configuration.mdx (#1022)
  i18n(es): Update `configuration` & `sidebar` (#1029)
  [ci] format
  i18n(zh-cn): Update frontmatter.md (#1020)
  i18n(zh-cn): Update overrides.md (#1021)
  i18n(zh-cn): Update i18n.mdx (#1019)
  fix(docs-i18n-tracker): update `translations` import (#1025)
  [ci] format
  i18n(zh-cn): Update css-and-tailwind.mdx (#1018)
  [ci] format
  i18n(zh-cn): Update authoring-content.md (#1016)
  i18n(ko-KR): update `configuration.mdx` (#1015)
  i18n(ko-KR): update `sidebar.mdx` (#1014)
  i18n(ko-KR): update `i18n.mdx` (#1013)
  [ci] format
  ...
Copy link

changeset-bot bot commented Nov 29, 2023

🦋 Changeset detected

Latest commit: a266690

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@astrojs/starlight-docsearch Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link

vercel bot commented Nov 29, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
starlight ✅ Ready (Inspect) Visit Preview Nov 29, 2023 10:40pm

@github-actions github-actions bot added 📚 docs Documentation website changes 🌟 core Changes to Starlight’s main package labels Nov 29, 2023
Copy link
Member

@HiDeoo HiDeoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not have API keys to actually test this but the code looks fine to me so far and I think the comments you left will be a good template/starting point for plugin authors 👍

Left a few comments regarding the documentation.

docs/src/content/docs/guides/site-search.mdx Show resolved Hide resolved
docs/src/content/docs/guides/site-search.mdx Outdated Show resolved Hide resolved
docs/src/content/docs/guides/site-search.mdx Outdated Show resolved Hide resolved
Co-authored-by: HiDeoo <494699+HiDeoo@users.noreply.github.com>
@delucis
Copy link
Member Author

delucis commented Nov 29, 2023

I do not have API keys to actually test this

If you wanted to test, you can use the test credentials provided on the DocSearch site: https://docsearch.algolia.com/docs/DocSearch-v3#testing

About to do that myself now that this has all our recent changes merged in.

@HiDeoo
Copy link
Member

HiDeoo commented Nov 29, 2023

If you wanted to test, you can use the test credentials provided on the DocSearch site: docsearch.algolia.com/docs/DocSearch-v3#testing

Oh I did not know about that, thanks 👍

@HiDeoo
Copy link
Member

HiDeoo commented Nov 29, 2023

Well, after playing with for a while, I can say that it's working amazingly well, great job.

The main thought I had was not related to this PR but mostly about extend which I wonder if accepting an array would not have been easier for users with multiple plugins extending the same schema.

Back on topic, I am not a big user of DocSearch but do you see people asking for other options? Like getMissingResultsUrl, initialQuery, searchParameters and such? Is this common? I see the Astro docs are using them quite heavily?

@HiDeoo
Copy link
Member

HiDeoo commented Nov 29, 2023

Also a related-unrelated kinda question: do you want me to setup a PR as a follow-up to this one to add back the plugin showcase and its references I removed from various pages that was in my plugin PR? Or maybe you have other plans, want to wait for more plugins, etc. Just let me know 👍

@delucis
Copy link
Member Author

delucis commented Nov 29, 2023

The main thought I had was not related to this PR but mostly about extend which I wonder if accepting an array would not have been easier for users with multiple plugins extending the same schema.

I did consider this but typing it was a nightmare. Especially as we support things like unions and intersections in the docs schema, which are even harder to generically combine.

For the i18n case specifically, it’s actually pretty OK because we only support z.object():

i18nSchema({
  extend: customSchema()
  	.merge(otherSchema())
  	.merge(z.object({ 'one.more': z.string() }),
})

For the docsSchema it will depend how the custom schemas are written how easy it is. With objects the same kind of approach can apply. With unions etc. it may be more complex.

Back on topic, I am not a big user of DocSearch but do you see people asking for other options? Like getMissingResultsUrl, initialQuery, searchParameters and such? Is this common? I see the Astro docs are using them quite heavily?

VERY good question. I kind of skipped over these during implementation as not being able to pas across the config boundary (and some are not really designed well for the plain JavaScript version of the component we’re using). I should probably take a look tomorrow at what we could do though. One option would be to let users provide a DocSearch options module or something, where they would export a function like:

export function getDocSearchOptions(routeData) {
  return {
    transformItems(items) {
      return items.map((item) => ({
        ...item,
        content: item.content.toUpperCase(),
      }));
    }
  }
}

@delucis
Copy link
Member Author

delucis commented Nov 29, 2023

do you want me to setup a PR as a follow-up to this one to add back the plugin showcase and its references I removed from various pages that was in my plugin PR?

I think the references removed in 0682f56 definitely already make sense! Don’t know if we need a separate plugins page (like a “Community Plugins” page) yet or not, although the nice thing would be giving some more visibility to them as a concept, so I’m not against either if you want to do that.

@delucis
Copy link
Member Author

delucis commented Nov 29, 2023

@HiDeoo What to do you think of treating support for those DocSearch options as a feature request to be added after an initial release? I don’t think there’s another way to build this plugin that would make supporting them easier, so releasing early shouldn’t be too risky (and we’ll release as a 0.1, so can always break later if needed).

@HiDeoo
Copy link
Member

HiDeoo commented Nov 29, 2023

I did consider this but typing it was a nightmare.

I see, this makes sense (and I'm not gonna be the one contradicting you on this one after my lost battle with it in the plugins PR 🤣).

One option would be to let users provide a DocSearch options module or something

Yeah, this is one we talked about on Discord for something else a long time ago and that I have used a few times to cross over. Not the most elegant but in the current state of things, I personally do not have other "easy" solutions and I think this may be ok for more "advanced" configuration scenarios.

I think the references removed in 0682f56 definitely already make sense!

Perfect, I'll setup a PR for this as a follow-up to this one.

although the nice thing would be giving some more visibility to them as a concept

I agree, altho it may be worth waiting for at least 2 or 3 plugins to get a dedicated page I think but that's something we can easily tweak later on.

What to do you think of treating support for those DocSearch options as a feature request to be added after an initial release?

I would think it's fine but as I'm not a huge user, I may be totally wrong on this one. But like you said, I don't think what you built would need a total rewrite to support this, and even more for a v0.1 so I would go for it imo and built on top of it later on based on what is needed for Astro docs and user feedback.

Copy link
Member

@HiDeoo HiDeoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Plugin #1 is cleared for takeoff in my book 🚀

@delucis delucis merged commit 8155d1a into main Nov 29, 2023
8 checks passed
@delucis delucis deleted the dx-618/docsearch branch November 29, 2023 22:54
@astrobot-houston astrobot-houston mentioned this pull request Nov 29, 2023
vasfvitor added a commit to vasfvitor/astro-starlight that referenced this pull request Dec 25, 2023
delucis added a commit that referenced this pull request Jan 3, 2024
* `overriding-components.md` #1168

* `getting-started.mdx` (#1241)

* `components.mdx` (#1102)

* `css-and-tailwind.mdx` (#1102)

* `customization.mdx` (#1102)

* `manual-setup.mdx` (#1102)

* translate filename in `components.mdx`

* fix: indentation

Co-authored-by: Chris Swithinbank <swithinbank@gmail.com>

---------

Co-authored-by: Chris Swithinbank <swithinbank@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📚 docs Documentation website changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants