matthewp commented 2 months ago

Summary

Allow islands of server-rendered content that renders after the page load, allowing more cacheable pages.

<Avatar server:defer>
  <div slot="fallback">Guest</div>
</Avatar>

Links

doeixd commented 2 months ago

Maybe a dumb question, but. Can server islands be nested in other server islands?

Would it be beneficial for that to raise a warning somehow to raise awareness of waterfalls?

sasoria commented 2 months ago

This works great for Server-side rendered microfrontends! From what I remember IKEA was pretty interested in something like that.

I've managed to set up a very small example that shows how easy it is to do it. Essentially, this is how I've done the composition:

<Fragment set:html={fetch("http://www.my-app.com")}/>

The microfrontend itself is just an ordinary Astro app.

The only issue I'm encountering is that <!doctype html> is added to each microfrontend, but I think there is was a way to remove it with a custom plugin.

jamesli2021 commented 1 month ago

What will happen if we decide to include localization? We would need to change the term 'Guest,' and my team might have to make multiple changes over time, as it has happened countless times before.

I think the fallback could be made easier by residing in the components-level with:

...
<Fragment slot="fallback">
<div>Guest</div>
<Fragment>

Or it is going to result in more breaking changes across projects in the future especially for theme developers.

thomasledoux1 commented 1 month ago

Not sure if this is the place to comment on this, but I tried this out and it works great in dev mode, but once deployed to Vercel, it does not work (yet). I have the following dependency installed for the adaptor "@astrojs/vercel": "^0.0.0-server-islands-20240709194928" On the deployed version I see an API call going out for the server island, but that API calls gets back a 404. Example can be found here: https://website-thomas-astr-git-8c2233-thomas-ledouxs-projects-74351fc8.vercel.app/blog/search-static-astro-website

Branch: https://github.com/thomasledoux1/website-thomas-astro/tree/feature/server-islands Relevant file: https://github.com/thomasledoux1/website-thomas-astro/blob/feature/server-islands/src/layouts/BlogLayout.astro

matthewp commented 1 month ago

@thomasledoux1 thank you, that is definitely odd. Are you using 'hybrid' or 'server' output?

thomasledoux1 commented 1 month ago

@thomasledoux1 thank you, that is definitely odd. Are you using 'hybrid' or 'server' output?

I’m using hybrid mode

PeterDraex commented 1 month ago

Is it possible that the code running in the browser will be outdated - that the server will be running a newer version than the browser? Maybe this could happen due to browser cache? Or CDN cache?

In such cases, the request to server islands could fail - for example, when the server island has been renamed in the new version. Should this be addressed?

jamesli2021 commented 1 month ago

In one scenario, if the page is served from a CDN is an outdated content, what would happen if the script on client-side attempted to render components that are no longer valid or at the same time you have deploy updated Astro? Would the problem be similar to that of a Single-Page Application (SPA)?

I believe that if we can invalidate the cached page, it might resolve the problem or check if it's existed e.g. /v1/... and try /v2/...

ascorbic commented 1 month ago

@jamesli2021 yes, it would be similar to an SPA, but without the problems that SPAs have of staying open in a browser. As you identify, it's only an issue for the CDN. The solution there is to invalidate pages when you do a new build. Lots of hosts will do that automatically (Netlify, Vercel, Cloudflare Pages for example all do it by default), but if you're running your own server with a separate CDN then you'd need to set it up to do the invalidation.

Tc-001 commented 1 month ago

Maybe an integration could define a custom fetcher so platforms like Vercel could take advantage of skew protection?

For something like cloudflare the fallback could instead be the deploy-specific URL. Not ideal as cookies would break, but the island will be at least rendered.

Fryuni commented 1 month ago

This is something that can already be done on the adapters. It only needs to modify fetch requests to indicate the build version the frontend comes from and direct to the appropriate version on the backend, even though it is the URL for the "latest" release.

Vercel, for example, has this as "coming soon" for Astro. It could be added relatively quickly to the adapter.

I don't know about an equivalent feature on other platforms tho

Tc-001 commented 1 month ago

I don't know about an equivalent feature on other platforms tho

I don't know if there are any other ones, although maybe an adapter could add a middleware with the same function, this would also keep the cookies.

Something like the below could be automatically added (maybe behind a toggle)

app.get("/_astro/skew/:version/*", () => {
 return fetch(`https://${version}.${name}.pages.dev/${req.path}`)  
})

But that also needs safeguards for SSRF stuff

Edit: maybe the middleware can just detect the headers, I assumed that a fallback URL is needed, but that isn't the case because island paths are always dynamic

matthewp commented 1 month ago

@Fryuni I'm pretty sure we already implemented this for Vercel, cc @ematipico

jamesli2021 commented 1 month ago

Thanks for enlightening me!

ematipico commented 1 month ago

Yeah we already did :) not sure why they didn't update it

jamesli2021 commented 1 month ago

There is one scenario on our site which has <Nav/> on the layouts, if we have some case we can't easily modify the components, we can defer using <Fragment>? I have tested out experimental Server Islands and it does not work for my use case.

Layouts.astro

<Fragment server:defer>
    {Astro.url.pathname !== '/' && <Nav image={'myAvatar'} />}
</Fragment>

When the page is pre-render, this mean I would need to move all the logic into my components.

jamesli2021 commented 1 month ago

If the client-side JavaScript is included in the components, it will execute before the components are loaded.

matthewp commented 1 month ago

@jamesli2021 you can't server:defer a fragment. It needs to be a component you import.

davidgauch commented 1 month ago

I would like to throw in my 2 cents about GET vs POST discussion. My understanding is that server-islands works best with static-site generation. Because of that we have the opportunity to raise an error at SSG time. That would mean that we could potentially give the user 3 options (and possibly the ability to set it globally)

GET. Triggers s SSG failure if request is too large, (fallback with error log in other envs).
GET with Fallback. Try to use GET, silently fallback to POST when too large. (I think this should be mentioned prominently in the build message.)
POST. I foresee some people wanting this, potentially as a quick and dirty no-cache option.

I know that for my projects I would use Option 1 as the default behavior in my apps to help me ensure I'm maximally leverage CDN caching.

PeterDraex commented 1 month ago

I know that for my projects I would use Option 1 as the default behavior in my apps to help me ensure I'm maximally leverage CDN caching.

What kind of CDN caching are you referring to? @davidgauch

I think typically, server:defer would be only used for resources, which cannot be part of the static build, because they are user-specific or change frequently. To cache such resources, you would need to invalidate the cache for a specific URL when data change. If you can do that, you are probably not using SSG in the first place, but rather run astro in SSR mode and cache the output (incremental static regeneration).

I think caching server:defer requests makes sense only in niche cases - when you have an Astro component, which is big (many kB of HTML) and it's not critical for First Contentful Paint. Deferring it's loading until later would give you a better FCP time. Is that your case?

PeterDraex commented 1 month ago

I'm in favor of using GET requests by default, together with HTTP header preloading. It will make the pages noticeably faster.

davidgauch commented 1 month ago

@PeterDraex

What kind of CDN caching are you referring to? @davidgauch

I was re-iterating the caching mentioned in the 4th bullet in the "GET vs POST" section of this (now closed) ticket. No CDN I have used has supporting caching based on request bodies.

you are probably not using SSG in the first place, but rather run astro in SSR mode and cache the output

Sorry about being unclear. I should have said 'hybrid' page rendering which, when I use it, statically generates most of my performance-sensitive pages.

Is that your case?

Close enough. The thing I most want it for is not too large, it is just somewhat slow to generate and can be stale. Ideally I would slap on stale-while-revalidate in a header and reduce the number of requests that make their way to my backend.

jamesli2021 commented 1 month ago

Once the cookie is set in the middleware, it doesn't persist the cookies in server mode with pre-rendered pages or hybrid mode. It seems to be designed primarily for static pages.

This is important for CSRF token (double submit) when you will definitely need a form e.g. contact form on a static page.

matthewp commented 1 month ago

Server Islands are not just for SSG. The demo app shows an e-commerce site, which would most likely be SSR in order to add new products without redeploying.

As far as the GET vs. POST, I very much favor being able to do GET as well. That's why I want to avoid adding extra features some have asked for like providing the origin URL, as those things would bloat the URL.

I do think in most cases the props will be small enough that GET is possible. I would probably favor approach (2) from @davidgauch's proposal here as it's the most straightfoward to implement.

Also before rushing into config we need to confirm that GET is practical, so we need some data on that. We can implement that as part of the experimental flag and see how well it works.

Will probably do prop encryption first since that will affect the size, then follow up with this idea.

jamesli2021 commented 1 month ago

Just to get my understanding if e-commerce demo in SSR with no pre-render can still leverage on Server Islands?

matthewp commented 1 month ago

Right, prerendered not required. Can use regular SSR. You want to set cache headers in that case though, as having cached main page content is the primary point.

matthewp commented 1 month ago

Here's what I'm tracking needs to be added to the RFC before it's complete:

Encrypting props.
Use GET if possible with POST fallback, including adding preload links to speed up the island loading.
Handle non-200 responses from islands.

There's a lot of discussion already about (1) and (2) in this thread, I'm curious to hear what people think we should do about (3)?

My initial instinct is that if the response is 4xx or 5xx we should emit an event and not render the response to the DOM. I do need to check what libraries like htmx do here for reference.

ascorbic commented 1 month ago

I'm curious to hear what people think we should do about (3)

I wonder if there's value in an error slot, so there can be a separate fallback state that's not "loading"

matthewp commented 1 month ago

That's a pretty good idea. The one downside to a slot is that you need to define it on every island you use. I imagine a lot of people won't do so, so we'd still probably want to have an event. Maybe this makes sense then:

slot="error" this will render statically, inside of a <template>.
- If there's a 200 it gets wiped out along with the fallback content.
- If there's a non-200 it's used as the replacement for the fallback.
If not error slot is provided an astro:server-island-error event is fired.

Fryuni commented 1 month ago

If we are going to add events it would be nice to have an astro:server-island-content or similar so client code could react to a server island receiving the updated content.

matthewp commented 1 month ago

@Fryuni what sort of thing would you do in that event?

Fryuni commented 1 month ago

Tracking and performance monitoring from the user's perspective. Server might be replying fast but we could measure how long is it taking to actually get the information on the page. That way we can account for network, queueing and processing time to see what is the real experience.

matthewp commented 1 month ago

Have a draft up for encrypted props: https://github.com/withastro/astro/pull/11535 Still a few things to do. Would appreciate anyone who can review this to make sure I did things generally right.

matthewp commented 1 month ago

Added a section to the RFC to outline the intent of prop encryption.

tedevra commented 1 month ago

Heyho,

i have an issue trying to make the server-islands demo work with my catchall page route and cloudflare adapter, maybe you guys know whats going on.

as soon as i switch /src/pages/index.astro to /src/pages/[...index].astro

running "npm run dev" (astro dev) => everything still works....

running "npm run preview" (astro build && wrangler pages dev ./dist) => does not work anymore and ends up in a loop (rerendering another page instance again and again, breaking the server-island -> default fallback)

same result when deploying the ./dist to cf: https://06739354.vh-tobi-test.pages.dev/

code as zip: astro-server-islands-poc.zip

working as expected or is there a misinvocation of the catchall page route during the /_server-islands/* request?

any help is highly appreciated, thanks a lot and have a nice day Tobi

screenshot: left side: run dev right side: run preview

jamesli2021 commented 1 month ago

How can we cache individual components if we have content that will likely remain unchanged over time, or revalidate as set in the header's cache control, such as the navigation menu and footer?

ascorbic commented 1 month ago

@jamesli2021 I think it will depend on implementing GET request support, but you should be able to just set response headers as usual inside the island, using Astro.response

jamesli2021 commented 1 month ago

@ascorbic I think the naming convention for server islands could pose problems when we deploy changes, as previous components may still be cached (e.g., Element1, Element2, etc.).

My idea is to use Unix time as part of the naming /server-islands/element2?1818181818 or /server-islands/element1818181818 to properly invalidate the old cache, which could solve these issues.

ascorbic commented 1 month ago

@jamesli2021 that's the same with any page though. As with any other page it's your responsibility to handle the cache and purge it if needed. Hosts like Netlify and Vercel will handle it automatically, or you do it yourself.

ruohki commented 4 weeks ago

I have been using this approach to error handling in SSR components https://tobyrushton.com/blog/how-to-catch-astro-ssr-errors sadly , this doesn't work with server islands, anyone got a better solution? (Without embedding the same error handling logic in all astro files)

danthegoodman1 commented 2 weeks ago

Is it possible to handle the case of rendering a page where you are dynamically listing something, and for example you want to:

Server side render direct page loads with search (incl empty search) from a search param
Switch to server islands when doing subsequent searches on the page

In Remix I'd do this with a useFetcher and just recall the loader, but I'm not sure if in Astro I can dynamically switch between a list that was SSR, then re-render that with a server:defer component (basically dynamically adding that).

That would still allow caching for the no search result and no search pagination, but if someone did text search then we'd hit the server always and modify the response to not cache, if that makes sense.

Edit: to be clear this would be search-replacement of content listed on the page (https://hn.algolia.com style), rather than say a dropdown that shows under the search bar overlaid on existing content (google style)

matthewp commented 1 week ago

I've implemented the GET behavior as discussed in this thread, including the preload link. It's available in a preview release: astro@experimental--si-get

I deployed it to server-islands.com and ran the tests a few times. Here are the relevant waterfalls:

GET

POST

Results

Using GET with the preload is causing these requests to start between 180 - 400ms faster. Looking at the waterfalls I think the biggest factor here is that the preloads are being kicked off before images. Whereas with POST the requests are competing with the images. This might be a case where this particular app has a larger effect than others will.

matthewp commented 19 hours ago

We're hoping to ship this in 5.0, so I am doing a call for consensus on this RFC. This allows us to take the feature out of experimental and mark it as stable. The CFC period lasts a minimum of 3 days, if there are concerns about the API please raise them now. Bugs should be filed in the core repo, thanks!

danfascia commented 18 hours ago

That's a pretty good idea. The one downside to a slot is that you need to define it on every island you use. I imagine a lot of people won't do so, so we'd still probably want to have an event. Maybe this makes sense then:

slot="error" this will render statically, inside of a <template>.

If there's a 200 it gets wiped out along with the fallback content.

If there's a non-200 it's used as the replacement for the fallback.

If not error slot is provided an astro:server-island-error event is fired.

An error slot would be an amazing idea...

If you could have slots for error and loading state I think so much work of the typical suspense process that you repeat over and over would be dealt with. It would very much emulate the behaviour of a try... catch... finally... sequence

It would also be amazing if one day we could bring websockets to Astro. I mean I know we can, but I meant as part of the framework so that for example islands can remain bound to a socket for future updates.

withastro / roadmap

Server Islands #963

Summary

Links

GET

POST

Results