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

Server Islands #963

Merged
merged 3 commits into from
Sep 6, 2024
Merged

Server Islands #963

merged 3 commits into from
Sep 6, 2024

Conversation

matthewp
Copy link
Contributor

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

@matthewp matthewp mentioned this pull request Jun 25, 2024
However this approach has some downsides:

- Eliminates the caching advantage gained by the script approach. Since the edge function injects personalized content the page cannot be cached globally.
- Only works will with Edge functions, so limited choice of hosts. Loss of portability.
Copy link

@kandros kandros Jun 25, 2024

Choose a reason for hiding this comment

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

Only works will with Edge functions,

typo

@doeixd
Copy link

doeixd commented Jun 26, 2024

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?


# Unresolved Questions

- During stage 2 there was some discussion about `props` which get serialized to the island. It could make sense to encrypt them to prevent mistakening leaking secrets. How/if this can be done hasn't been determined yet.
Copy link
Contributor

Choose a reason for hiding this comment

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

An alternative is signing the props, which would address the potential issue of untrusted props - though not the one of leaking secrets.

Copy link

Choose a reason for hiding this comment

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

Encrypting should address both, and has ~ the same implementation complexity

Copy link
Member

@Fryuni Fryuni Jul 7, 2024

Choose a reason for hiding this comment

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

I think it would be best to use a built-time-generated symmetric key with some algorithm available under the Web Crypto standard. That way it is available on basically every JS server runtime.

@sasoria
Copy link

sasoria commented Jun 26, 2024

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

jamesli2021 commented Jul 1, 2024

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

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
Copy link
Contributor Author

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

@thomasledoux1
Copy link

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

I’m using hybrid mode

@PeterDraex
Copy link

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

jamesli2021 commented Jul 17, 2024

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
Copy link
Contributor

@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
Copy link

Tc-001 commented Jul 17, 2024

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
Copy link
Member

Fryuni commented Jul 17, 2024

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

Tc-001 commented Jul 17, 2024

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
Copy link
Contributor Author

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

@jamesli2021
Copy link

Thanks for enlightening me!

@ematipico
Copy link
Member

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

@jamesli2021
Copy link

jamesli2021 commented Jul 18, 2024

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

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

@matthewp
Copy link
Contributor Author

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

@davidgauch
Copy link

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)

  1. GET. Triggers s SSG failure if request is too large, (fallback with error log in other envs).
  2. 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.)
  3. 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
Copy link

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

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

@davidgauch
Copy link

davidgauch commented Jul 20, 2024

@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
Copy link

jamesli2021 commented Jul 20, 2024

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
Copy link
Contributor Author

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

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

@matthewp
Copy link
Contributor Author

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
Copy link
Contributor Author

matthewp commented Jul 22, 2024

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

  1. Encrypting props.
  2. Use GET if possible with POST fallback, including adding preload links to speed up the island loading.
  3. 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
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Member

Fryuni commented Jul 22, 2024

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
Copy link
Contributor Author

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

@Fryuni
Copy link
Member

Fryuni commented Jul 23, 2024

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
Copy link
Contributor Author

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

@matthewp
Copy link
Contributor Author

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

@tedevra

This comment has been minimized.

@jamesli2021
Copy link

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
Copy link
Contributor

@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
Copy link

@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
Copy link
Contributor

@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
Copy link

ruohki commented Aug 1, 2024

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

danthegoodman1 commented Aug 8, 2024

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

  1. Server side render direct page loads with search (incl empty search) from a search param
  2. 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
Copy link
Contributor Author

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

Screen Shot 2024-08-15 at 2 49 38 PM

POST

Screen Shot 2024-08-15 at 2 49 45 PM

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
Copy link
Contributor Author

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

danfascia commented Aug 28, 2024

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.

@matthewp matthewp merged commit 6563e83 into main Sep 6, 2024
@matthewp matthewp deleted the server-islands branch September 6, 2024 20:19
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.