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

Prerender API #362

Merged
merged 7 commits into from
Jan 12, 2023
Merged

Prerender API #362

merged 7 commits into from
Jan 12, 2023

Conversation

natemoo-re
Copy link
Member

@natemoo-re natemoo-re commented Oct 25, 2022

Summary

  • Users should be able to deploy an Astro server without sacrificing the speed or cacheability of static HTML.
  • The Prerender API (previously "Hybrid Output") allows users to statically prerender specific pages/ at build time.
  • The API surface is relatively small: include export const prerender = true in any file in the pages/ directory that should be prerendered

Links

@tony-sull
Copy link

I'm wondering how this will work for dynamic routes like [slug].astro or [...path].astro, would getStaticPaths() work to build static pages for a subset of matching routes?


```astro
---
export const output = 'server'

Choose a reason for hiding this comment

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

will it be possible for our language tools to give type checking and auto-complete as an export const?

Copy link
Member Author

Choose a reason for hiding this comment

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

Probably possible! Can add a note about this.

Copy link
Member

Choose a reason for hiding this comment

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

Confirming that it's possible! We'll have to either provide a type for it, or fake completions / diagnostics (like NextJS does)

- Having a separate mode for `hybrid` increases cognitive overhead and the learning curve of Astro.
- `hybrid` output should arguably be the default `output` mode (similar to Next.js), but to avoid a breaking change we're going with an opt-in adoption approach

# Alternatives

Choose a reason for hiding this comment

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

Might be nice to add cache headers as an alternative here, even if just to show how many hoops have to be jumped through to properly handle caching in a CDN via headers, it's not the easiest balancing act 😅

Copy link

Choose a reason for hiding this comment

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

I completely agree that Astro should just always be hybrid!
In astro.config the output option could be renamed to defaultOutput to set the default for the hybrid mode. The default mode can be overridden by exporting the output from the frontmatter. 🤔

This would be the most flexible approach imho and also would be super logical and not change the learning curve.

  • I am totally unclear how the output would look like and how we would handle adapters as we always need an ssr-adapter to do anything on the server. E.g. we would always export a directory with client and server folders, which is weird if there is no ssr happening.
  • I understand that this would mean a breaking change, but still worth a though.

@panwauu
Copy link

panwauu commented Oct 26, 2022

Maybe we should keep in mind to allow the syntax to support other future modes (other than static or server).
These are just some thoughts. Of course, we should go for the easiest solution that provides the needed flexibility.

I have seen some interest (myself included) in ISR. This is currently mostly solved with cache headers, which can be painful IMHO. ISR would require an option to set the rebuild cycle. This would not really be compatible with the current proposal. One way with the current proposal would be to encode the cycle in the string. This would require parsing and make the typing more complex. It does not seem clean at all.

---
export const output = 'isr-1000' // `isr-{cycleInMs}`
---

Or we could add the cycle as an option to add per file. Which would add another option only for ISR.

---
export const output = 'isr'
Astro.response.isrCycle = 1000
---

@matthewp
Copy link
Contributor

As from the call, I think we have multiple concepts we are grouping under output but are actually 2 or 3 different things.

  • output is how Astro builds the site.
    • static build to HTML
    • server build to JS that can be run on a server.

Then there's this other thing called edge which supposedly builds for an edge function. But an edge function is a server too. The difference between the two is more cultural than real.

imo, when you specify edge vs. server that's really a signal to the adapter (or to multiple adapters).

So in my mind we have two concepts, one is a signal to Astro and one is a signal to adapters. We could call this other thing mode or target or runtime or layer or something else. But I think this other thing is configuration that needs to be set in each route.

@panwauu
Copy link

panwauu commented Oct 26, 2022

output

  • output is how Astro builds the site.

    • static build to HTML
    • server build to JS that can be run on a server.

This would mean, that there is no such thing as a hybrid output, right? We can set the output to static to produce a static HTML site and to server to produce something to be run on a server which might include some static files.

output is how Astro builds the site.

  • static: HTML to be served directly
  • server: output includes js to be run on server with possible additional static files

Not really sure here. Would we serve the static content ourselves, or would the adapter take care of serving the static files?
For example, the vercel-adapter builds to two folders, a static folder (with css, client-side js, ...) and a functions folder containing the server functions.

adapter configuration

So in my mind we have two concepts, one is a signal to Astro and one is a signal to adapters. We could call this other thing mode or target or runtime or layer or something else. But I think this other thing is configuration that needs to be set in each route.

This is currently handled by passing arguments to the adapter ASAIK. With the vercel adapter by importing the correct one:

import vercel from '@astrojs/vercel/edge';
import vercel from '@astrojs/vercel/serverless';

It could also be handled by passing arguments to the adapter when registering it.

@kyr0
Copy link

kyr0 commented Nov 1, 2022

Hi all,

I just wanted to share with you, that my recent learning was that having SSR and SSG pages split by folders might also be an idea worth thinking about (instead of having them split per route with a new mode). To me, it feels quite nice from the DX point of view.

I spiked this in a different way tho... with the current version 1.6.2, having different projects combined in one deployment; of course: Different projects aren't optimal and I had to come up with some solutions behind the scenes to get it working smoothly, but it does...;

My learning was that it feels quite clear, intuitive and nice to have SSR and SSG clearly split "at first sight". It also helps to understand separating the concerns and could lead to less boilerplate when the developer doesn't need to specify the type of execution per route:

Bildschirmfoto 2022-11-01 um 22 03 47

(this screenshot is showing a split at project level which isn't optimal; but maybe a folder naming convention under "pages", like "ssr" subfolder or "ssg"/"static" subfolder could be?)

If you wanna play with this:

Here is the impl: https://github.com/kyr0/turbo-hybrid-astro-on-vercel#welcome-to-turbo-hybrid-astro-on-vercel
Tech demo / live: https://turbo-hybrid-astro-on-vercel.vercel.app/ (pls scroll down and click on the cards, they lead you to the different pages. It is all deployed on the same Vercel deployment and works seamlessly in dev and build)

I also explained my spike results/learnings here:
https://github.com/kyr0/turbo-hybrid-astro-on-vercel#spike-result

@natemoo-re
Copy link
Member Author

Thanks for all the feedback, everyone! I am going to update this RFC to take all of it into account and we'll be chatting about this again in a more focused RFC-specific call this week.

For a quick summary of the expected changes:

  • Adding an additional hybrid output is unnecessary complexity for many reasons (mentioned above)
  • This RFC will shift focus to enabling static pre-rendering when using output: 'server'.
    • The default will be 'server' unless otherwise specified.
    • Dependencies for static routes will not be bundled in with the final server.
  • There are still some open questions about how adapters should handle statically generated pages
  • Per @kyr0's explorations, there's clearly some interest in configuring entire directories at once. I don't want to increase the scope of this RFC any more, but I think we could address this with a follow-up RFC.

Changes from `output: hybrid` to `output: server` + `export const output = "static"`
@tony-sull
Copy link

@natemoo-re one question I hadn't thought about until this morning, how will this work for integrations or components that either work differently in static vs server mode, or only support one build mode? Will an integration/component be able to tell what context it for a specific page being rendered?

Two examples that came to mind for me:

  • @astrojs/image sets up the build a bit differently for static vs server and renders src differently depending on the output target
  • not sure if this is still the case, but at one point astro-icon didn't support SSR and was best used for SSG sites

@natemoo-re
Copy link
Member Author

@tony-sull Hopefully this is outlined better in the updated RFC, but since this mode is now being treated the same as server, adapters will likely just work the same as before. If there is some post-processing that needs to happen on the .html files, those will be present in the manifest.

@FredKSchott
Copy link
Member

@tony-sull image will be an interesting one to think through, but I think server mode would still work as-is even for prerendered pages (worth confirming though).

@smithbm2316
Copy link

I really am a huge fan of this proposal and think that either Alternative option F or @kyr0's proposal would be the options that I favor the most for this feature.

I have no idea how complicated the implementation internally would be for either of these, but I particularly like the idea of alternative F that allows you to name a file with a .server.astro or .static.astro, similar to how Remix does that. I think it makes it immediately clear what the rendering strategy is for that file and makes it hard to be confused about it, unlike situations that I could see arising with export const output = 'static' where you no longer are able to use that variable name in your JS/TS for the file. It might not be immediately obvious to beginners that the variable there does anything special and could be a tripping up point.

Alternative C I think is also a decent option because then you're only ever using 1 specific named value in the Astro template, and you can extend that object with as many per-page options down the line with no breaking changes required. Feels like a better version of the current proposal with none of the future potential downsides.

Next.js 13 beta recently added quite a few new named variable exports in their files for configuring caching that I just am not a fan of, as I think that for beginners it's gonna be really hard to discover that those values mean something specific. It's nice if you know how it works because it's terse and quick to write, but I think it just is really hard to google/search for answers to if you're a beginner. If there's gonna be a named export that means something, I usually think using an object or function (like loader/action functions or meta/links objects in Remix) for it makes it more clear that those might be specific meta-framework features and not just something that a developer has created themselves (hence why i'd prefer alternative C if using a named export variable).

Looking forward to see how this RFC develops!

@natemoo-re natemoo-re marked this pull request as ready for review November 15, 2022 18:40
@natemoo-re natemoo-re changed the title Hybrid output mode Prerender API Dec 15, 2022
@natemoo-re natemoo-re merged commit d96635b into main Jan 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.