starlight
starlight copied to clipboard
Add support for SSR
Summary
Add configuration for not prerendering Starlight pages.
This enables using components with SSR logic on Starlight pages, whether in the content or as component overrides.
Notes
Double index
To do this without triggering multiple warnings, there are now two .astro pages for the index entrypoint:
- The pre-existing
index.astrois always prerendered and usesgetStaticPathsandAstro.propsto receive information about the route. - A new
indexSSR.astrothat is never prerendered and retrieves the information about the route from the dynamic route parameter.
Route parameter indexing
To make the SSR rendering efficient, a lookup map for all the content was created using the parameter that will be received from the dynamic route.
Pagefind
Pagefind indexing requires access to the HTML file generated by the routes, but those are not available when building for SSR, so Pagefind is not supported when using SSR.
~~The solution that I implemented for this is that when both SSR and Pagefind are enabled for Starlight, the content of the docs is pre-rendered for indexing but not included in the final build. The final content served will be from SSR, but the index will be built from that static rendering.~~
Default behavior
~~Starlight will default to the same as any other Astro page, prerendering when the output mode is not defined, when it is static, and when it is hybrid. When the output mode is server, Starlight will default to SSR.~~
~~Starlight previously didn't build at all when the site had output mode set to server, so the different default is not, AFAICT, a breaking change.~~
Starlight will default to prerender pages, which differs from the default for Astro pages. This is to maintain compatibility and because docs pages are most commonly not dynamic. Users must set prerender: false in their Starlight configuration to enable on-demand docs page rendering. At the moment, this applies to all Starlight pages. Future work could be done to support SSR on only some of the pages.
TODO
- [x] Disable Pagefind by default and make it incompatible
- [x] Move all
child_processlogic to build plugins (git information) - [x] Add tests for SSR builds
The latest updates on your projects. Learn more about Vercel for Git βοΈ
| Name | Status | Preview | Updated (UTC) |
|---|---|---|---|
| starlight | β Ready (Inspect) | Visit Preview | Jul 15, 2024 0:45am |
π¦ Changeset detected
Latest commit: fac878204c0f5c0d7cefba69d34feac54592fcad
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
Hello! Thank you for opening your first PR to Starlight! β¨
Hereβs what will happen next:
-
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!
-
In a few minutes, youβll be able to see a preview of your changes on Vercel π€©
-
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.
For anyone that want to try this out on their website to check for problems you can install it with:
npm install https://gitpkg.now.sh/Fryuni/starlight/packages/starlight?ssr-support
yarn add https://gitpkg.now.sh/Fryuni/starlight/packages/starlight?ssr-support
pnpm add https://gitpkg.now.sh/Fryuni/starlight/packages/starlight?ssr-support
Probably blocked by https://github.com/withastro/astro/issues/9392, due to that bug any project using Starlight with SSR cannot add any other page to the project as everything will be overridden by Starlight's [...slug] route.
Any updates on that? I have to patch package to add export const prerender = true; to the index.astro to make it work with @astrojs/vercel/serverless
This depended on withastro/astro#9439, released on 4.2. It's on my pipe to come back to this and make any necessary changes. I'll probably ping people to review it by next Tuesday.
waiting for this to be released π
Looking good. Like the decision to exclude ssr pages from pagefind.
How the new errors look like:
Not a review per se as I did not jump into the code yet, but I started testing the PR with different scenarios and I noticed some weird behavior when running the tests in watch mode. The output seems a bit weird but more importantly, the tests are endlessly running in a loop and making my CPU go crazy ^^
https://github.com/withastro/starlight/assets/494699/940d9dc8-406a-4437-9126-4fd01878cd00
Ooh, right. I need to ignore the tmp folder so it doesn't trigger the test again. Good call!
Mirroring here some Discord messages here in case someone encounter this:
- @Fryuni did not manage to repro the issue on macOS and Linux.
- This is still happening for me on macOS, even tried on a fresh clone. I'll try to investigate a bit more when I get the time.
An additional question: one of your messages on Discord states that this PR should enable the use of the Cloudflare adapter to deploy a Starlight website to Cloudflare with SSR. I tried to set this up (not a huge Cloudflare user so maybe I did something wrong, altho I mostly followed the adapter guide) but this is breaking for me at build time due to unsupported Node dependencies (which makes sense to me with the current setup but considering your message, is this expected to work?).
It should work. I moved the Node APIs that were needed for git to be used only on build time.
I'll double check to see what else is missing, but the goal of this PR is for it to work on all SSR adapters and have only the same requirements as Astro itself
When can we use this feature? Very much needed for my latest project. Just need to be able to deploy the site on cloudflare. Please help. Any estimated time? Also any reason why still import { fileURLToPath } from 'node:url'; is used in packages\starlight packages\starlight\integrations packages\starlight\utils This is causing errors when trying to build for cloudflare, as these are node api, i assume. Please give us a example which we can deploy to cloudflare, if you need help in testing, i can do that.
@Fryuni. Thanks for your support. I have a tested this new changes and tried to deploy to Cloudflare. I was able to deploy it. But there is a catch, I have to get a Workers Paid subscription. Because in cloudflare there is limit for function file size (1 MiB), but the basic starlight function size is around 8-9 MiB (output = "server"). In Workers paid plan, the limit is 10 MiB. So i was able to deploy it. But for someone planning to implement this in a much bigger project which uses more server side capabilities, this might be something to look out for.
But once again, Thank you very much @Fryuni
but the basic starlight function size is around 8-9 MiB (output = "server"). In Workers paid plan, the limit is 10 MiB. So i was able to deploy it. But for someone planning to implement this in a much bigger project which uses more server side capabilities, this might be something to look out for.
Yeah, I don't think there is much for us to do on that front. In server output mode the entire content and all the tooling to render the content has to be shipped into a single function to be processed on each request, so that is a lot of data.
Hey folks! Thanks for this effort @Fryuni. I just wanted to let you know that I shipped a website with your patched version from https://gitpkg.now.sh/Fryuni/starlight/packages/starlight?ssr-support to make Starlight work with https://authjs.dev/ (which requires SSR) and it worked seamlessly! I'm using Vercel's adapter.
I might reserve some time to take a look at the ins and outs of this PR and provide some assistance in getting it merged into the main branch.
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 | manual-setup.mdx | Source changed, localizations will be marked as outdated. |
| 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. |
Hey @marcelovicentegc, thanks for the ping! There is not much blocking this PR; it is just a big change, and the team hasn't had the time to review it yet.
@delucis any chance this gets closer to the top of your to-do list? π₯Ί
To share a small update, I continued over the past few days the list of SSR scenarios I had, which was blocked at Cloudflare last time, and all of them worked without any problems with all the adapters I tried, no issue at all :tada: Amazing work :clap:
I'll continue with a more code-oriented review when I get a bit more free time, but so far, personally, this is looking super great!
First, I'd like to apologize for the delay in reviewing this PR. It's obviously a very large PR, we have other tasks on the side, we all have our own lives, but I still feel bad for not having enough time to review properly this PR earlier. Thanks a lot for your patience.
It's not a problem at all @HiDeoo, thanks for the review. It was a fun journey, especially teasing @delucis on Discord (sorry if I overdid it π ).
Now I'm closer to becoming a first-time contributor to Starlight π€£π€£π€£
Second, and this could have been my first point, I'd like to say "wow". I obviously already played a lot with the PR in the past, and being familiar with your work, I was already expecting a high quality PR, but I have to say that while I was reviewing the code, I was still amazed by the quality of the work you did. π
Oohh! Thank you π This was my first time using a lot of the tools involved on Starlight (like Vite and Vitest), so looking at it now there is a lot of things to improve. But I did the best I could at the time.
To give a global overview of my review, I think there is only 1 major issue that would need to be fixed, a few spots that could be refactored to avoid some code duplication, some nitpicks regarding the new example but overall, I think the PR is in a very good shape.
Oh god π The example was mostly for debugging as I developed, I haven't done anything to make it useful for others yet. I hope you haven't spent too much time on it.
There are many conflicts to resolve, so it's a great opportunity for refactors. I'll work on it this week(end).
To add a few extra details that I didn't mention in the review:
- I did not review yet the documentation changes (altho they should be pretty straightforward and relatively small)
- Nit: we have the
__tests__/folder for our unit tests,__e2e__/for our Playwright tests so far.__tests_functional__feels a bit weird, maybe we could just go with__functional__/or__integration__/or something like that which would not repeat thetestsword?- We should document the new type of tests in the "Testing" section of the
CONTRIBUTING.mdfile and just like we did with e2e tests, add some description to help users identify when they really need functional tests and always prefer unit tests when possible.
The use case for functional tests seems quite limited for Starlight. It is for when we want to test the final output of the whole build and serving process (so they can't be unit tests) but don't care about what is rendered in the browser (so it doesn't need to be e2e). Although not strictly necessary, it might be better to turn the functional tests into e2e tests so there is not yet another test bed in the repo. (there was no e2e tests back then)
The example was mostly for debugging as I developed
Oh yeah, and it helped me a lot too. I also think it'll bring a lot of value for people who want to get started with SSR or even just to debug/setup a repro quickly at a fairly low maintenance cost once it's done.
(there was no e2e tests back then)
I totally forgot about that and that's a very good point. I don't see either a huge need of "functional" test for Starlight on the long run so I really like your idea of refactoring the few you added to be e2e ones as I don't think they need anything Vitest specific. @playwright/test would do the job just fine for that very limited usage and nothing prevent us from not interacting with a browser in such tests, we could even tag these tests differently if we want.
FYI I've rebased this PR to a recent version of starlight (https://github.com/aaronmondal/starlight/pull/1) to test whether this works with Qwik. Works pretty much out-of-the-box :+1:
It might be worth noting that in the experiments I ran with @astrojs/cloudflare, the SSR variant with expressive code enabled caused a large server-side bundle size (I think it was ~8MB for shiki or something, and compression/treeshaking didn't seem to have an effect) which ultimately prevented deploying the SSR variant to Clouflare Pages/workers/functions (https://github.com/TraceMachina/nativelink/blob/f5e72760e722a34023e9196073d23fc38443e5ef/docs/astro.config.mjs#L28).
@HiDeoo, any idea how I could optimize the e2e git test?
It runs on 4s locally on a considerably old Intel Mac Air, but for some reason, it is reaching the 30s timeout on CI. I'm out of ideas π
Deploy Preview for astro-starlight ready!
| Name | Link |
|---|---|
| Latest commit | fac878204c0f5c0d7cefba69d34feac54592fcad |
| Latest deploy log | https://app.netlify.com/sites/astro-starlight/deploys/66db82540bb5aa00083aa912 |
| Deploy Preview | https://deploy-preview-1255--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: 92 (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.
Updated the end-to-end fixtures to the latest Astro release to match the updates in #2287 β tests still all seem good!
Just wanted to say I'm excited! :partying_face: Congrats, @Fryuni !
Hey, folks! I just wanted to let you know that I've been following this PR for the last couple of months and wanted to congrats everyone involved on this effort. You did an amazing job here @Fryuni @HiDeoo and @delucis! I'm saving this PR as an example of open source contribution.
Really cool to see these in-depth discussions happening async, and moving forward!
Again, congrats and thank you @Fryuni !
Trabalho incrΓvel, @Fryuni! Congrats team!
