[WIP] Move route data to Astro.locals

[delucis]

Description

• Early draft PR exploring moving Starlight’s route data object to Astro.locals — currently this is passed down via Astro.props to all our templating components.

• There are some tricky nuances here for sure.

• For example, middleware runs for all routes on a site, which can include non Starlight pages. For these we can’t generate route data. For now I’ve reflected this in the types for locals and asserted Astro.locals.routeData! in the components. There’s not really a sensible way to guard that without duplicating a null check in every component, but also it feels like route data should be defined in all cases where Starlight’s <Page> is rendered.

• The <StarlightPage> component poses some challenges. Right now you pass some props and it generates route data. I’ve hacked this in by assigning that generated data to locals inside the component for now, but this does mean you can’t transform data for pages created this way with additional middleware. Not sure there’s any way to solve this?

  • One way might be to have a dedicated system of “route data middleware” implemented at the Starlight level instead of using generic Astro middleware, e.g.

    starlight({
      routeMiddleware: './some-file.ts',
    })

    We could bundle those in a virtual module and have both Starlight’s locals.ts and <StarlightPage> use them or something:

    context.locals.routeData = await useRouteData(context);
    for (const fn of routeMiddleware) {
      context.locals.routeData = fn(context.locals.routeData);
    }

• The current branch rips out the prop drilling entirely. That means any overrides that rely on Astro.props will break. Could it be worth doing something to ease migration? Deprecate props but keep them around? Throw an error something like we did for labels? Something to think about.

• There’s probably a tidier way to do some of the code — just did the quick and easy thing for now. (For example, Content is currently typed as optional in the route data object to make it easy to throw it in where I needed it, without checking all the places that type is used. But could probably tidy that up to have a dedicated separate type.)

• Can also discuss naming here. So far it’s Astro.locals.routeData, but there’s probably an argument for something a bit more descriptive like Astro.locals.starlightRoute or even just Astro.locals.starlight potentially.

To-do

• Remove demo middleware from docs

[changeset-bot]

🦋 Changeset detected

Latest commit: 13d2038

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	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

[netlify]

✅ Deploy Preview for astro-starlight ready!

Name	Link
🔨 Latest commit	13d2038
🔍 Latest deploy log	https://app.netlify.com/sites/astro-starlight/deploys/67a3696f74ec5a00085d916a
😎 Deploy Preview	https://deploy-preview-2390--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 (🟢 up 1 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 site configuration.

[astrobot-houston]

size-limit report 📦

Path	Size
/index.html	6.93 KB (0%)
/_astro/*.js	25.76 KB (0%)
/_astro/*.css	13.75 KB (0%)

[lorenzolewis]

Really exciting to see this!

The current branch rips out the prop drilling entirely. That means any overrides that rely on Astro.props will break. Could it be worth doing something to ease migration? Deprecate props but keep them around? Throw an error something like we did for labels? Something to think about.

With the i18n update didn't we have a somewhat breaking change (possibly that's the labels update you're talking about)? I feel like if it's at all possible to keep current behavior around and mark it as depreciated then it would be much more helpful. This change will probably impact A LOT of users who have spent time crafting component overrides.

Can also discuss naming here. So far it’s Astro.locals.routeData, but there’s probably an argument for something a bit more descriptive like Astro.locals.starlightRoute or even just Astro.locals.starlight potentially.

Is there any norm here in Astro-land for 3rd party integrations? This has a potential for name collisions if a user has implemented middleware, correct? If that's the case then I think it would be good to at the very least prefix any locals with starlight.

[HiDeoo]

Sharing a few quick thoughts before I forget them as I've been playing with the idea locally too and we can discuss more in depth later.

For now I’ve reflected this in the types for locals and asserted Astro.locals.routeData! in the components.

As this impacts components and not user components, I've been experimenting with a getter function instead that throws if the route data is not available, which would mean it was called in a non-Starlight route.

The <StarlightPage> component poses some challenges.

Definitely a tricky one indeed, been trying a few things but haven't found something that I'm happy with yet too.

Deprecate props but keep them around? Throw an error something like we did for labels?

As we have seen from the 0.28.0 reports, it's relatively easy to craft explicit errors for Astro.props.* usage in user code altho it gets confusing for users when it's coming from a plugin.

If we compare to labels, in a panel of 15 plugins, only 3 where using Astro.props.labels so the impact was pretty low. This would definitely not be the case for all other props. Still need to think about this one but if we go with the same approach as labels, we should make sure to mention it in the changeset/breaking change that users should make sure their plugins are up to date (which is something I didn't think about for labels).

[delucis]

Spent a bit of time revisiting this PR and feeling quite positive about the idea. Made the following adjustments, which helps it feel better:

1. I pulled 404 route data up into our route data process as well. This means that we can guarantee the locals will always contain Starlight data, even if it is only the 404 route for pages Starlight doesn’t know about. Avoids the need for a getter that throws an error (which I also played with for a bit) and means we really centralise all the data operations in the route data layer.

2. I renamed locals.routeData to locals.starlightRoute. Feels better to namespace it a bit and also feels better when using things like entry.data where the repeat “data” in the path — locals.routeData.entry.data — felt kind of repetitive and vague. starlightRoute expresses things nicely I think.

There’s a type error I’m not 100% sure about — I guess we’re pulling some of our virtual modules into view for type checking that we weren’t previously? Not sure, but haven’t investigate too much. Next up I’d like to tackle the idea of a route data “middleware”/“pipeline” so that users and plugins have a place to plug in and modify stuff.

[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	guides/overriding-components.mdx	Source changed, localizations will be marked as outdated.
en	guides/route-data.mdx	Source added, will be tracked.
en	reference/overrides.md	Source changed, localizations will be marked as outdated.
en	reference/plugins.md	Source changed, localizations will be marked as outdated.
en	reference/route-data.mdx	Source added, will be tracked.

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.

[martrapp]

Thanks, everyone, for the refreshers! I can explain it! ;-)

Most of the time I’ve worked with software, versioning usually started at 1.0. I'm still too used to thinking of 0.x versions as immature precursors that might never fully materialize. But that definitely doesn't apply to Starlight ;-) That's probably where my unease comes from. Apparently, I’m not the only one who finds years of 0.x versions odd. See Anthony Fu’s article on this for more: https://antfu.me/posts/epoch-semver.
Cheers!

[trueberryless]

Quick comment: I love the documentation addition from a7eebad

Really nice example 👍

[martrapp]

Really nice example 👍

First thing I'll do when it comes out, maybe a bit truncated ;-)

[trueberryless]

@delucis Can we please ask Sarah if we can leave the middleware? 🥺

It looks awesome!!!

Or do we not even have to ask Sarah in the Starlight Docs, only in Astro i guess...

CHRIS CAN WE LEAVE THE MIDDLEWARE IN PLS??? 🥺 🥺 🥺 🥺 🤣

[delucis]

Just updated the PR with proper APIs for plugins to use and a new middleware runner that adds support for next() callbacks to allow middleware to delay code execution until after other middleware runs.

Summary:

• Plugins can no longer directly update routeMiddleware in the Starlight config.
• Instead, they use a new addRouteMiddleware() callback. This has the same signature as the Astro addMiddleware() requiring an entrypoint and an optional order property.
• When we run plugins, we collect all middleware and then apply them based on the order values.
• Middleware handlers can await next() to delay code execution until later middleware has completed. Calling next() is optional.
• Docs have been updated to reflect these changes.

Marked this as ready for review as I think now we have all the pieces in place?

I’m still a bit nervous about just how breaking these changes are. I imagine a lot of plugins and projects will be impacted.

One possibility:

We could walk back the changes to drop Astro.props and do that later potentially.

In this scenario:

1. We still generate things on locals and run middleware
2. We then pass the route data as Astro.props down through components as currently

That would mean existing overrides would continue to work while people migrate. However, it would not be possible to avoid a future breaking change because at some point we’d still remove the prop drilling. So it may make sense to do it now and make sure we release this when we have time to help with updating as many plugins as possible.

---

Things still on my radar:

• There are some broken links that need fixing
• I still haven’t been able to revisit some feedback on the “Route Data” docs page @sarah11918 shared, but that felt like it could follow later if necessary.

[HiDeoo]

Played quite a bit this week-end with the new changes we discussed. This is working pretty well. Left a few questions and high-level comments/concerns but overall, no real issues with the new approach.

Amazing work 👏

[HiDeoo]

This part is a bit tricky:

• Currently, it's called for everything, e.g. even images
• It's called even for pages not rendered by Starlight even tho the wording make it looks like it.
• It can be called multiple time for Starlight custom pages (404 → custom page)

[delucis]

Thinking about this a bit more. I really don’t like how this leaks everywhere.

I moved stuff to an Astro middleware, which is clean and allowed me to type Astro.locals.starlightRoute as always being defined, which is nice for component code because you don’t need guards everywhere to check if stuff is available. But the fact it means we may generate lots of route data unnecessarily (e.g. on non-Starlight pages), or run twice (e.g. on custom pages) feels BAD 😓 The image endpoint could be worked around, but it still feels like a code smell.

Given that, we probably need to handle it a bit differently and align it more with these docs which say “every time Starlight renders a page”. That is when this data is actually needed (unlike the Astro.locals.t() which makes sense to support in other pages).

To do that, I think we probably need to do something like this:

• Move the route data process back to our Page component (similar to how the StarlightPage component does it currently).
• Type Astro.locals.starlightRoute as potentially undefined.
• Try to improve DX for components that depend on the route data. Couple of ideas:
  • Provide a utility like assertRouteData() that throws if the data isn’t available (i.e. component used outside of a route), e.g.

    ---
    import { assertRouteData } from '@astrojs/starlight/route-data';
    
    assertRouteData();
    const {} = Astro.locals.starlightRoute;
                         // ^ guaranteed to be defined
    ---

    (not sure if type guards like that work for nested object properties?)
  • Use a getter API instead that throws on non Starlight routes? That might feel more natural to people and would avoid extra imports:

    ---
    const {} = Astro.locals.starlightRoute;
                         // ^ under the hood, this is a getter with an
                         // initial implementation that throws, then we replace
                         // the implementation when generating route data

    (not sure we can add a getter at the top level like that or if it would need nesting, I can check)

What do you think of that @HiDeoo?

[HiDeoo]

I think this is closer to the initial approach I described in my first comment to the PR when I was playing with the idea. I don't think this would change any of the assumptions I made so far in my experiments but I would definitely have to confirm that once doable.

Regarding assertRouteData():

• If used internally (our own components), that would mean someone would not be able to use such component outside of Starlight. This was not something officially supported but it was technically possible by passing the expected data as a prop, it would have been possible I guess with the previous approach too but would no longer be possible with this one (depending on what the check actually is).
• I think we would need a version that does not throw, e.g. isRouteData() or something too?
• May need to double check on that but I don't think we can have such utility asserting the type of something without passing such thing, so at least assertRouteData(Astro).

Regarding a getter approach, this is exactly what I used in some early tests, seemed to work well altho if it throws by default, it prevents the easy creation of let's say a component doing something slightly different if used in a Starlight page vs in a non-Starlight page. A try/catch would be needed in that case. Not a big deal but just a thought I wanted to mention.

I think aligning on the approach described in the documentation (and probably the one people would expect?) makes more sense and was probably my biggest concern during the last review.

[trueberryless]

I think aligning on the approach described in the documentation (and probably the one people would expect?) makes more sense and was probably my biggest concern during the last review.

Just to give a quick feedback of a guy who doesn't understand 80% of the technical stuff that's being discussed here: Yes, I can confirm that the approach currently* described in the docs logically makes a lot of sense (although maybe hard to achieve behind the scenes) and is what I would expect! As I said, just my thoughts and honest feedback...

*Current docs ref:

You can customize route data using a special form of “middleware”.
This is a function that is called every time Starlight renders a page and can modify values in the route data object.

[delucis]

someone would not be able to use such component outside of Starlight

I think this is a reasonable position for us to take? Our components are designed to be used by us, not just anywhere, so I’m not sure it’s a goal to support that. If it IS, then we have to drop the locals idea entirely and stick to props. That’s also an option if we decide it’s important.

In a way there are two separate concerns:

1. Have a way to process data for each route (route middleware)
2. Simplify components and make the current route more widely available by moving route data to a global context (locals)

We don’t have to do both 1 and 2 necessarily. We could have a process that runs the route middleware, but then still maintain the prop drilling approach we use currently. That would make this feature non-breaking.

[HiDeoo]

someone would not be able to use such component outside of Starlight

I think this is a reasonable position for us to take?

Definitely, maybe I wasn't clear enough but I'm all for it, I was just mentioning it while thinking about everything.

It's already impossible to properly support that CSS-wise and most of the time doesn't make sense to use it outside of Starlight. The maintenance cost of supporting it would be too high too to justify it. The stance of saying to people "copy the code and cherry-pick what you need" like we're currently doing is the best way to go imo.

[delucis]

OK, gotcha! I personally do think that there are arguments that global context is not ideal if it can be avoided as a general rule, but I guess we’re deciding that the advantage of having an easily accessible data object for all components is powerful enough to justify this.

In that case, I’ll play with the getter option and we can call out explicitly in the changelog that you can’t use Starlight components elsewhere. (Well, you still could technically I guess. You would just need to set Astro.locals.starlightRoute = myPropsObject instead of passing myPropsObject into a component.)

For the future it would be good to understand the use cases people have for reusing things and see if we can address those with a proper approach. For example, if it is “I need the nav bar but nothing else” or “I just want search”, maybe there are targeted ways to support just those use cases. (For example with a dedicated simpler template some people have asked for. Or turning something like <Search> into a user component.)

[delucis]

Made the changes to switch to a getter and generate route data inside the routes themselves rather than in the middleware — feels good I think. LMK if it seems like a reasonable approach.

[HiDeoo]

Considering middlewares may run for non-Starlight pages, and in this case starlightRoute will contain the data of a 404, I think we may need to document a little bit more this behavior and add documentation regarding 404s.

Some questions coming to mind are:

• Should we document that ids for 404 route will always be 404?
• Would a user need to discriminate a real 404 vs a 404 entry data for a custom page?
• Is having starlightRoute always defined to avoid some nullish coalescing operator or a type-guard the best approach?
• Would a discriminated union be helpful in this case, e.g. { isStarlightRoute: false, starlightRoute: undefined } and { isStarlightRoute: true, starlightRoute: StarlightRouteData }?

[delucis]

I think this comment is resolved by moving to a getter that throws an error and deferring the middleware to only run in actual routes?