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

Make Starlight compatible with server output mode #1454

Merged
merged 8 commits into from
Feb 9, 2024

Conversation

Fryuni
Copy link
Member

@Fryuni Fryuni commented Feb 3, 2024

Description

Make projects using Starlight and output: 'server' build successfully.

All Starlight pages will still be pre-rendered as though they were emitted for a static build.
The built-in search, powered by Pagefind, will index any custom route that is also pre-rendered, SSR routes won't be indexed.

This PR is the simplified version of #1255, making Starlight compatible with SSR projects, but not allowing any SSR on Starlight pages themselves.

Copy link

changeset-bot bot commented Feb 3, 2024

🦋 Changeset detected

Latest commit: b37115f

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

Copy link

vercel bot commented Feb 3, 2024

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

Name Status Preview Updated (UTC)
starlight ✅ Ready (Inspect) Visit Preview Feb 9, 2024 8:19pm

@Fryuni
Copy link
Member Author

Fryuni commented Feb 3, 2024

It took me a while to open this because I was testing locally with Starlight docs itself. It doesn't work there but not because of this change, it is due to withastro/astro#9674, which breaks the page when using the Node adapter and trailingSlash: 'always'.

If anyone wants to try this locally, make sure to comment out the trailingSlash option until that bug is fixed :)

Copy link
Member

@delucis delucis left a comment

Choose a reason for hiding this comment

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

Looks great @Fryuni! Thanks also for the thorough testing, notes, and backlinks to related issues. I think that issue should obviously be fixed, but doesn’t have to block us in the meantime.

I’ve left a couple of queries — mostly requests for enlightenment — and a suggested a couple docs tweaks. Thanks again for tackling this!

@@ -43,11 +43,15 @@ export default function StarlightIntegration({
injectRoute({
pattern: '404',
entrypoint: '@astrojs/starlight/404.astro',
// Ensure page is pre-rendered even when project is on server output mode
prerender: true,
Copy link
Member

Choose a reason for hiding this comment

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

Mainly out of curiosity, why do we need both this AND the export const prerender? Seems like an API smell in Astro to me, so if these are both required I’d love to understand why.

Copy link
Member Author

Choose a reason for hiding this comment

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

It is something that I noticed while working on the routing logic. Different parts of Astro core have different behavior for prerendered and on-demand routes, but those different parts don't all have access to both those methods of defining whether something is prerendered.

For example, the conflict warning about on-demand routes with the same dynamic segments runs before the file is read, so it only knows about this prerender parameter here. Another part, that generates another warning about not matching a static path in the dev server, only knows about the prerender constant from the file, not the value passed here.

So to have a consistent behavior everywhere this needs to be passed in both places.

I haven't found anything but warning that check only one of those, but I haven't checked too deeply either.

Copy link
Member Author

Choose a reason for hiding this comment

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

This could also relate to endpoints not being prerendered if just this flag is passed in (which I mentioned here: withastro/astro#7721 (comment)). But that is mostly speculation on my part, I haven't actually tried to debug that at all.

packages/starlight/index.ts Outdated Show resolved Hide resolved
.changeset/cool-days-look.md Outdated Show resolved Hide resolved
.changeset/cool-days-look.md Outdated Show resolved Hide resolved
docs/src/content/docs/manual-setup.mdx Outdated Show resolved Hide resolved
Co-authored-by: Chris Swithinbank <swithinbank@gmail.com>
Copy link
Member

@delucis delucis left a comment

Choose a reason for hiding this comment

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

Thanks for taking care of all that @Fryuni! Should be ready to go. Suggesting one small tweak to the changeset.

Hoping to release this tomorrow — weekend SSR fans get ready 🙌

.changeset/spicy-moose-approve.md Outdated Show resolved Hide resolved
Co-authored-by: Chris Swithinbank <swithinbank@gmail.com>
@delucis delucis merged commit 1d9ef56 into withastro:main Feb 9, 2024
9 checks passed
@astrobot-houston astrobot-houston mentioned this pull request Feb 9, 2024
@Fryuni Fryuni deleted the prerender-on-server branch February 9, 2024 21:32
HiDeoo added a commit to HiDeoo/starlight that referenced this pull request Feb 10, 2024
* main: (96 commits)
  [ci] format
  i18n(ko-KR): update `manual-setup.mdx` (withastro#1482)
  i18n(ko-KR): update `community-content.mdx` (withastro#1483)
  [ci] format
  [ci] release (withastro#1481)
  [ci] format
  Make Starlight compatible with server output mode (withastro#1454)
  [i18nIgnore] community content: article description copy edit (withastro#1408)
  [ci] format
  i18n(it): Updated plugins.md and community-content.mdx (withastro#1480)
  i18n(fr): update `resources/community-content` (withastro#1479)
  [ci] format
  i18n(it): Modified everything in the /guides folder (withastro#1456)
  i18n(it): Modified frontmatter.md and overrides.md (withastro#1457)
  i18n(es): fix syntax highlighting with `diff`-like syntax example (withastro#1477)
  Add CodingCat.dev stream video to community content page (withastro#1475)
  i18n(fr): fix frontmatter `label` default value translation (withastro#1476)
  [ci] format
  [ci] release (withastro#1474)
  Set default color theme in template (withastro#1473)
  ...
HiDeoo added a commit to HiDeoo/starlight that referenced this pull request Feb 20, 2024
* main: (126 commits)
  [ci] format
  [ci] release (withastro#1488)
  Fix rendering issue for text with unhandled Markdown directives (withastro#1489)
  i18n(es): update community-content (withastro#1493)
  i18n(es): update manual-setup (withastro#1492)
  Updates Korean UI translations (withastro#1487)
  i18n(fr): update `manual-setup` (withastro#1484)
  [ci] format
  i18n(ko-KR): update `manual-setup.mdx` (withastro#1482)
  i18n(ko-KR): update `community-content.mdx` (withastro#1483)
  [ci] format
  [ci] release (withastro#1481)
  [ci] format
  Make Starlight compatible with server output mode (withastro#1454)
  [i18nIgnore] community content: article description copy edit (withastro#1408)
  [ci] format
  i18n(it): Updated plugins.md and community-content.mdx (withastro#1480)
  i18n(fr): update `resources/community-content` (withastro#1479)
  [ci] format
  i18n(it): Modified everything in the /guides folder (withastro#1456)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌟 core Changes to Starlight’s main package 📚 docs Documentation website changes 🌟 minor Change that triggers a minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants