Allow passing user options to pagefind.createIndex

[controversial]

Description

This PR allows the user to specify options in pagefind.index that will be passed through to pagefind.createIndex, notably:

• excludeSelectors, which is useful for excluding content from pagefind where the user does not control the markup to add data-pagefind-ignore. Configuring this option is the main motivation for this PR.
• rootSelector, which could be useful if anyone wants to change the default placement of data-pagefind-body from its default placement on <main>.
• forceLanguage which seemed like it wouldn’t hurt to include but I don’t have a specific motivation for it, and can remove it if it seems it would work against Starlight’s locale handling.

I excluded the ability to configure logging options here, since they don’t affect the output index, and I excluded the config option for including /index.html paths since it doesn’t seem useful within Starlight.

These options weren’t practical to expose in Starlight before switching to the JS API in #3534 but now it seems reasonable to make them configurable.

This is a more generic solution to allow users to address problems like the one described in #3676

---

I’m eager for suggestions on how to include unit tests for this functionality, at a glance it seems like the existing unit tests mock most of the relevant functionality for pagefind, so I haven’t added tests here yet as it didn’t seem straightforward

[changeset-bot]

🦋 Changeset detected

Latest commit: 697254d

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	Patch

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

[astrobot-houston]

Hello! Thank you for opening your first PR to Starlight! ✨

Here’s what will happen next:

1. Our GitHub bots will run to check your changes.
   If they spot any issues you will see some error messages on this PR.
   Don’t hesitate to ask any questions if you’re not sure what these mean!

2. In a few minutes, you’ll be able to see a preview of your changes on Netlify 🤩

3. One or more of our maintainers will take a look and may ask you to make changes.
   We try to be responsive, but don’t worry if this takes a few days.

[astrobot-houston]

Lunaria Status Overview

🌕 This pull request will trigger status changes.

Learn more

By default, every PR changing files present in the Lunaria configuration's files property will be considered and trigger status changes accordingly.

You can change this by adding one of the keywords present in the ignoreKeywords property in your Lunaria configuration file in the PR's title (ignoring all files) or by including a tracker directive in the merged commit's description.

Tracked Files

Locale	File	Note
en	reference/configuration.mdx	Source changed, localizations will be marked as outdated.

Warnings reference

Icon	Description
🔄️	The source for this localization has been updated since the creation of this pull request, make sure all changes in the source have been applied.

[netlify]

❌ Deploy Preview for astro-starlight failed. Why did it fail? →

Name	Link
🔨 Latest commit	1220146
🔍 Latest deploy log	https://app.netlify.com/projects/astro-starlight/deploys/697a871c2d100300089db23d

[netlify]

✅ Deploy Preview for astro-starlight ready!

Name	Link
🔨 Latest commit	697254d
🔍 Latest deploy log	https://app.netlify.com/projects/astro-starlight/deploys/697b9d2d40ea7c0008d975fe
😎 Deploy Preview	https://deploy-preview-3674--astro-starlight.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
Lighthouse	1 paths audited Performance: 100 (no change from production) Accessibility: 100 (no change from production) Best Practices: 100 (no change from production) SEO: 100 (no change from production) PWA: - View the detailed breakdown and full score reports

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[delucis]

Thanks for the PR @controversial! We normally ask people first open a discussion for features like this to give us a bit of time to chat through the expectations, motivations, use cases etc. and solicit user feedback, which helps us prioritise and understand the value of things.

In this case, I do have some concerns around the usefulness of everything included here. I can see an argument for excludeSelectors (although we do already document how to “Exclude part of a page”, so I’d like to understand more about the scenarios where that approach is not sufficient). The rootSelector feels risky: I’m not quite clear what this would help with. And as you mentioned, the forceLanguage setting doesn’t have an obvious usage (I imagine in a basic Pagefind scenario you might have content that doesn’t include lang attributes for whatever reason and need to force it, but in Starlight’s case, we already make sure all content is tagged correctly).

We can chat about these here if you like, but given these doubts, it might make more sense to move to a discussion to outline all the use cases etc.

[controversial]

I’d be happy to open a discussion!

The primary motivation here was that I was trying to accomplish #3675 in user-land, and was surprised to find that i didn’t have the ability to configure excludeSelectors. Having excludeSelectors would allow users to fix this type of pagefind indexing issue without waiting for upstream changes, in cases where the user doesn’t control the markup that they’re trying to ignore.

I think that’s useful functionality even when paired with data-pagefind-ignore, because you can’t always set data-pagefind-ignore where you need it.

I’d be happy to drop the other two parts of this PR (rootSelector, forceLanguage) as I think the argument to be made for those is less clear

[delucis]

The primary motivation here was that I was trying to accomplish #3675 in user-land, and was surprised to find that i didn’t have the ability to configure excludeSelectors.

I guess to me the question is whether there are use cases that wouldn’t be “Fix a Starlight bug”, because if the API mainly allows user-land fixes of bugs on our end, it would be better to fix those bugs upstream rather than requiring individual users to patch things up if they notice. Have you hit other cases where control over excludeSelectors would have helped you?

[controversial]

I think the other cases would be anywhere that useres are rendering markup that they don’t directly control—this could be markup that Starlight generates, or it could be markup from 3rd-party remark plugins, or from 3rd-party components that are imported and rendered within mdx content. IMO it’s not uncommon to have markup in a page to which it would be difficult to directly attach data-pagefind-ignore

[delucis]

Thanks for chatting that through! Yeah, I agree there’s a reasonable argument for excludeSelectors here. Would you be able to clean up the PR to only include that? I’ve also left a couple of early review notes.

Re: testing — did you try excludeSelectors in a demo project? I think knowing it works correctly with a manual test (including e.g. for multiple different selectors) would be a good start. When switching to Pagefind’s Node API we decided for our CI testing, we should trust Pagefind behaves like it’s supposed to, so only test we pass the right values in from user config. That‘s probably sufficient here too.

[delucis]

I think this would still end up leaking the index configuration into the client bundle. It might be better to do this here when creating the virtual module so it isn’t included at all:

starlight/packages/starlight/integrations/virtual-user-config.ts

Line 139 in 49f153d

'virtual:starlight/pagefind-config': `export const pagefindUserConfig = ${JSON.stringify(opts.pagefind || {})}`,

And updating the virtual module type needs to be done manually too here:

starlight/packages/starlight/virtual-internal.d.ts

Lines 25 to 29 in 49f153d

	declare module 'virtual:starlight/pagefind-config' {
	export const pagefindUserConfig: Partial<
	Extract<import('./types').StarlightConfig['pagefind'], object>
	>;
	}

[delucis]

Given we’ll only support excludeSelectors, we can probably describe that here in a tiny bit of detail.

[delucis]

I’m not sure it’s useful to document this using a type from Pagefind which people can’t look up. But with the simplification, it’s probably not too bad to show the actual type? (I’m assuming it’s something like this.)

Suggested change

	index?: Pick<
	pagefind.PagefindServiceConfig,
	'rootSelector' | 'excludeSelectors' | 'forceLanguage'
	>;
	index?: {
	excludeSelectors?: string[];
	};

[delucis]

Hi @controversial are you still interested in landing these changes?

[controversial]

Yes, I am, sorry that I haven’t been able to work on this for a few weeks! Will try to make these changes today or tomorrow.

[delucis]

No worries! Just wanted to make sure if we should close or keep the PR. Take your time 🙌