"Virtual" pages prototype

[HiDeoo]

What kind of changes does this PR include?

• Changes to Starlight code

Description

This pull request is an attempt to:

• Closes Improve custom page injection #982
• Fix integrations and plugins injecting routes using injectRoute() being broken starting with Starlight v0.14.0

Note: This PR is a draft as it is not ready to be merged and is still a work in progress regarding most aspects from code, to tests, to documentation which have been mostly put in place to get feedback on the approach which is what I'm mostly interested in before spending more time on this.

This PR introduces a new component <VirtualPage /> (name up for discussion if the approach is accepted, I didn't find anything better yet) that can be used by integrations and plugins injecting routes to render dynamically generated content using the Starlight layout.

The preview includes a set of pages generated by a plugin using this approach:

• basic page with default sidebar
• page with no sidebar
• page with custom sidebar and a toc

Technical details

A lot of details are already included in the feature request but I'll provide some additional details here.

Before this PR, an integration or plugin injecting a new route would need to render the @astrojs/starlight/components/Page.astro and provide all the expected route data to it. This was mostly unsafe as there was no way for a plugin to provide all expected data and was a bit cumbersome, e.g. this example from my blog integration:

export function getPageProps(title: string, slug: string): Props {
  const entryMeta = getEntryMeta();

  return {
    ...entryMeta,
    editUrl: undefined,
    entry: {
      data: {
        head: [],
        pagefind: false,
        title,
      },
      slug,
    },
    entryMeta,
    hasSidebar: true,
    headings: [],
    id: slug,
    lastUpdated: undefined,
    pagination: {
      next: undefined,
      prev: undefined,
    },
    sidebar: [],
    slug,
    toc: {
      items: [],
      maxHeadingLevel: 0,
      minHeadingLevel: 0,
    },
  };
}

Another (and maybe more) important point is that this was impossible for a plugin to use the generated sidebar based on the Starlight configuration meaning all pages generated by a plugin would not have a sidebar, e.g. this breaking change in my OpenAPI integration:

Due to some API changes in Starlight, all OpenAPI documentation pages will no longer display a sidebar. If you wish to preserve a sidebar on these specific pages until this is fixed, please stick to the 0.2.2 release and Starlight <0.11.0 for the time being.

Starting with v0.14.0, the change that exposes localized UI strings in route data and having all internal components using the route data to access localized UI strings makes is now impossible for a plugin to use such an approach as it would require the plugin to provide localized UI strings which is not possible.

To address this, the proposed approach is to introduce a new component <VirtualPage /> that can (and should) be used by integrations and plugins to render dynamically generated content:

• This component is a wrapper around the <Page/> component
• It accepts a different (but slightly similar) set of props
• Some optional props will be automatically provided by the component, e.g. if a page does not provide a custom sidebar and does not explictly hide it, the default sidebar will be used
• It makes generating props a bit easier by removing the complexity of entry as they technically do not exist for virtual pages

Rendering a virtual page would look like this:

---
// plugin/src/Example.astro
import VirtualPage, {
  type VirtualPageProps,
} from '@astrojs/starlight/components/VirtualPage.astro';
import CustomComponent from './CustomComponent.astro';

export function getStaticPaths() {
	// Omitted for brevity...
}

const props = {
  title: 'My custom page',
  slug: 'custom-page/example',
  template: 'doc',
  hasSidebar: true,
  headings: [{ depth: 2, slug: 'description', text: 'Description' }],
  dir: 'ltr',
  lang: 'en',
  pagefind: true,
  head: [],
} satisfies VirtualPageProps;
---

<VirtualPage {...props}>
  <h2 id="description">Description</h2>
  <CustomComponent />
</VirtualPage>

If this an approach that is found acceptable, there is still a lot of work to be done as I mentioned earlier but so far I have not been able to find a better approach to address these issues.

[vercel]

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

Name	Status	Preview	Updated (UTC)
starlight	✅ Ready (Inspect)	Visit Preview	Feb 16, 2024 10:34pm

1 Ignored Deployment

Name	Status	Preview	Updated (UTC)
starlight-i18n	⬜️ Ignored (Inspect)	Visit Preview	Feb 16, 2024 10:34pm

[changeset-bot]

⚠️ No Changeset found

Latest commit: 8871b4d

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

[Genteure]

Thank you for working on this feature!

I just made another prototype with this prototype: withastro/docs#5551
I think the approach works pretty well and injected pages are easy to write and add.

From DX point of view, making all props but title optional or providing a helper that returns a set of defaults would be nice, since only title is required in page frontmatter and it's a little confusing where should I get those values.

A lot of things are not working correctly in withastro/docs#5551 right now, most of I assume are just because this PR is WIP and not implemented yet so I'm not too worried.
One that might need extra attention is, <Content/> from content collection entries are rendered as <p set:html="text here"></p>.
Preview link: https://docs-n9dghxdqp-astrodotbuild.vercel.app/fr/reference/glossary/

[HiDeoo]

Thanks for taking the time to try it out and the super valuable feedback, greatly appreciated! 🙌

After seeing your PR, I understand what you meant the other day on Discord and I'm not sure what I read, but I totally did not imagine that ^^ This makes way more sense to me now 👍

From DX point of view, making all props but title optional or providing a helper that returns a set of defaults would be nice, since only title is required in page frontmatter and it's a little confusing where should I get those values.

My original thinking was that if a plugin emits a page with some content, the plugin must knows the content so it also knows the language, the text direction, if it should be searchable or not, etc. Another point is that I tried to minimize the changes for the initial draft (we basically get what is the minimal expected props to get it working).

That's definitely something to think about more just like the <Content/> component you mentioned, this is not something I even tried personally yet.

[ematipico]

My original thinking was that if a plugin emits a page with some content, the plugin must knows the content so it also knows the language, the text direction, if it should be searchable or not, etc.

Although some websites don't have that information, or don't need to provide all of them because they are just adding a subset of the props.

The proposal looks sound to me and it's highly needed because v0.14.0 breaks the current usage of Page.

[delucis]

Thanks for the amazing work on this @HiDeoo! And apologies for breaking all your stuff with the i18n labels change — I didn’t think through how that could impact upstream stuff using <Page> like this.

On the whole I think this looks like a good direction. I think I tend to agree with people arguing for more default values so a minimal set of props are required — maybe even just title and slug? I think we have the power to choose sensible defaults for everything else. (For frontmatter, we could even parse it using Starlight’s frontmatter schema to ensure consistency and equivalent defaults.)

One thing I’d push for us to think about is how this fits into our i18n system. Are we able to have consistent fallback behaviour for virtual roots? (Right now other languages 404.) And will virtual routes be fully compatible with the language switcher? Admittedly this could be quite tricky because we’re not actually in charge of the routing — like withastro/roadmap#688 would allow us to be — so we don’t actually have access to the content. I wonder if we could provide helpers for getStaticPaths() that would handle that somehow?

But overall — big thumbs up! If I’m able to champion my roadmap proposal for programmatic content collection entries, we might want to eventually revisit this, but this looks ideal for unblocking this sooner rather than later!

[HiDeoo]

Thanks for the review 🙌 (and no worries, I think none of us thought about that potential change but imo, this was due to happen at some point anyway, as we have no dedicated story for the <Page/> component yet).

Considering the approach seems reasonable, I'll start thinking more about some of the raised points and will try to come up with an update to this PR. Regarding defaults, I'll review all the props and see what we can do, I would guess indeed title and slug could be the only required ones but I'll double check.

Regarding i18n, this is something I left as an open question for this first draft but indeed, definitely something to consider, although I'm not sure yet what the best approach would be.

[HiDeoo]

Unfortunately, yes, this is correct. (Or at least was when I last tested.)

Thanks for the confirmation, this is pretty much what I assumed altho turns out this may have changed and my understanding was based on an pretty weird behavior:

• To test my vi.stubEnv('BASE_URL', '/base/');, I used a VSC snippet to insert the following code in urlToSlug()

  console.log('🚨 [slugs.ts:108] import.meta.env.BASE_URL:', import.meta.env.BASE_URL);

• The log while running this specific test was showing me that the base was / so I assumed that the vi.stubEnv was not working.
• Turns out that if you replace the log with the following code:

  console.log('🚨 [slugs.ts:108] SOMETHING ELSE:', import.meta.env.BASE_URL);

  The log will show that import.meta.env.BASE_URL is /base/.

Super sketchy behavior so I think it's safer right now to keep this test as a test.todo and introduce base tests at a later time? Altho, the good news is that the test.todo test for the base seems to be working as expected.

[delucis]

Wow, now that is odd behaviour 😅 Thanks for testing though — I’ll mention this to the team as well in case there’s a simple way to unblock stubEnv support. Blu might know.

[sarah11918]

Gotta board a plane, so here are thoughts! Love this new page!

[sarah11918]

Just wondering aloud whether another term like "Docs pages" or Documentation pages conveys more info here. Or even, "Autogenerated Pages" ... something that might stand a bit more in contrast to "Custom" pages.

[delucis]

Yeah the naming was hard here. My thinking was that these are in src/content/, but yeah it’s not perfect. They’d probably both be “docs pages”, but maybe something like “autogenerated” could work… 🤔

[delucis]

OK! Let’s go! Can’t believe we’re finally here!

@HiDeoo, you have poured your heart and soul into this one and I want to express my gratitude for your patience, thoughtfulness, and care as we inched this to this point. Amazing dedication 🌟