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 not cacheable #12975

Open
1 task done
JannikWempe opened this issue Jan 13, 2025 · 12 comments
Open
1 task done

Server Islands not cacheable #12975

JannikWempe opened this issue Jan 13, 2025 · 12 comments
Labels
- P2: nice to have Not breaking anything but nice to have (priority) feat: server islands Related to Server Islands (scope)

Comments

@JannikWempe
Copy link

JannikWempe commented Jan 13, 2025

Astro Info

Astro                    v5.1.3
Node                     v18.20.3
System                   Linux (x64)
Package Manager          npm
Output                   server
Adapter                  @astrojs/netlify
Integrations             none

If this issue only occurs in one browser, which browser is a problem?

No response

Describe the Bug

Server islands can't be cached effectively. Yes, you can use cache headers and your CDN will cache it.

BUT on server mode. The p query param of the auto-generated server island URL will change with every request. (The p param should be stable using static output but I think there are use cases for server islands in server mode (e.g. different cache durations)).

Here is a screenshot from my personal page showing two requests in the network tab:
image

Note that the p param changes between requests. You can check it out here if you want (note that I configured CloudFront to ignore the p param for caching).

So the CDN would have to ignore the p param but that would result in another issue. The p param contains the encrypted props that are passed to the server island component. Ignoring the p param for caching would result in server islands with different props sharing the same cache. That would not work...

So currently you can only choose between:

  • not caching (because of changing p)
  • caching, but potentially sharing cache between components that have different props passed to them

Why does that happen?

p changes between requests because the iv that is used for encryption is random (which it probably should be for strong encryption). See here.

What's the expected result?

One idea to prevent the issue:
Add another query param that is a hash of the passed props. The same props would result in the same generated hash.

With that, the CDN could ignore p and the caching would work as expected because the new query param would be different for different props and thus not sharing the same cache for them anymore.

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-s2hf8p8k-4ntncmcw

Participation

  • I am willing to submit a pull request for this issue.
@github-actions github-actions bot added the needs triage Issue needs to be triaged label Jan 13, 2025
@ematipico
Copy link
Member

Please update your reproduction, there's no p prop provided to the island

@JannikWempe
Copy link
Author

Please update your reproduction, there's no p prop provided to the island

I am not talking about a p prop, but the generated p query param that is used in the auto-generated _server-islands/ prefixed path (see my screenshot).

p is changing on every request, even if you don't pass any props. That is why it can't be cached.

@ematipico
Copy link
Member

it seems this is being addressed here #12956

can you confirm?

@ematipico
Copy link
Member

Closing as duplicate of #12949

@ematipico ematipico closed this as not planned Won't fix, can't repro, duplicate, stale Jan 13, 2025
@JannikWempe
Copy link
Author

That is related, for sure. But it looks like it is just about server islands without any props being passed to it.

What about a server island that I pass props to? How should I cache those?

For that reason, I don't think this is a duplicate...

@ematipico
Copy link
Member

The reproduction and issue talk about "no props" - as I was corrected, but now you bring up server islands with props.

1 similar comment
@ematipico
Copy link
Member

The reproduction and issue talk about "no props" - as I was corrected, but now you bring up server islands with props.

@ematipico ematipico reopened this Jan 13, 2025
@ematipico ematipico added the needs discussion Issue needs to be discussed label Jan 13, 2025
@JannikWempe
Copy link
Author

I am talking about props in general from the beginning 🤔

image

The point is: the same server island should be able to use the same props.

<!-- these should share a cache -->
<ServerIsland server:defer /> <!-- same props as next line (no props at all) -->
<ServerIsland server:defer /> <!-- same props as previous line (no props at all) -->

<!-- these should share another cache -->
<ServerIsland server:defer something="test" /> <!-- same props as next line -->
<ServerIsland server:defer something="test" /> <!-- same props as previous line -->

I am sorry if that wasn't clear enough, I'll update the reproduction.

@ematipico ematipico removed the needs discussion Issue needs to be discussed label Jan 13, 2025
@ematipico
Copy link
Member

I believe Astro doesn't check if props are static or not, or even if their values are shared across islands. It's possible we could actually improve that

@ematipico ematipico added - P2: nice to have Not breaking anything but nice to have (priority) feat: server islands Related to Server Islands (scope) labels Jan 13, 2025
@github-actions github-actions bot removed the needs triage Issue needs to be triaged label Jan 13, 2025
@kaytwo
Copy link
Contributor

kaytwo commented Jan 13, 2025

My workaround for this issue is to use this pattern: https://docs.astro.build/en/guides/client-side-scripts/#pass-frontmatter-variables-to-scripts

along with Astro sessions that hold any sensitive user data server side so it doesn’t have to get passed directly back to the server as a prop.

Can you say a bit more about your use case? Are the props passed into the server rendered component dynamic for some reason? Where does that dynamism come from? If it comes from the server, you should be able to pass the data in the body instead of the props like above (much easier to just interpolate into the component if it doesn’t need to be accessible from the client side js). If it’s coming from the client side (like a “who is logged in” indicator), the server should also know that, or if you’re eg using a react component you can update it on client side render with a useEffect.

@kaytwo
Copy link
Contributor

kaytwo commented Jan 13, 2025

The use case that I designed the “no props = no encryption” for was browser side caching, which can’t be done on the basis of specific parameters afaik. This is part of why I would like any solution to this to not be cdn specific.

spitballing here - would it be possible to send the encrypted props as an X-Component-Props header, then only put a hash of the props in the query string? I only use the node and cloudflare adapters but I believe you’d be able to read the custom headers out on the server side in at least those two if not all adapters.

My one concern with this approach is that in public caches it enables chosen plaintext “existence” attacks, where I can check the cache response (explicit hit header or timing) to determine whether someone else was looking at a component with a given prop. For instance a stock ticker that takes the stock symbol as a prop, I could check occasionally to see what stocks were being queried.

I think caching is great but would want to be careful with what app innards get exposed to the outside world, and if sensitive data is being passed back and forth at all would probably prefer to use an alternate method to accomplish it.

@datner
Copy link

datner commented Jan 13, 2025

The case is mostly for caching server islands in something like a CDN in an e-commerce scenario.
Say you have a list

declare const items: string[] // <- dynamic, not static
{items.map(it => <Component server:defer item={it} />)}

This would render a dynamic list of items. As people have said, you can denote cache requests by specific query parameters in your CDN provider, and it's clear that currently, caching by p is moot.
However, in e-commerce applications, this caching can be of vital importance. Invalidating the entire storefront to change the price of a single item is nonsensical. (Dumping your entire database into memory and rendering every single storefront and every page you host from scratch on every typo someone makes it even more absurd, so SSG is irrelevant)
The concerns raised by @kaytwo are valid, but so are the concerns of the users and their choices rather than the framework. That is to say, keeping the p query param impenetrable is fine, but maybe adding some way to attach a stable query param defined by the user explicitly would go a long way.

{items.map(it => <Component server:defer server:query={it} item={it} />)}

would create a ?p=XXXXXXX&ck=abc123 (ck as in cache-key or whatever)

Yes, this takes a bite out of the 2048-byte limit, but considering that the point is caching through a CDN, where even responses to POST requests can be cached, this is basically a non-issue.

Basically, the mental model shift is from a server island holding dynamic information unique to the specific user viewing a page to a server island holding dynamic information unique to itself regardless of user or page.
The general use case is not to be fully SSG, so you don't need to deploy the app multiple times per second and not be fully SSR, so you don't just stress-test your backend on how good it is with massive concurrency, pseudo-ISR if you will.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P2: nice to have Not breaking anything but nice to have (priority) feat: server islands Related to Server Islands (scope)
Projects
None yet
Development

No branches or pull requests

4 participants