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

Edge runtime is not compatible with ISR #102

Closed
nitedani opened this issue Aug 13, 2024 · 5 comments
Closed

Edge runtime is not compatible with ISR #102

nitedani opened this issue Aug 13, 2024 · 5 comments

Comments

@nitedani
Copy link
Contributor

nitedani commented Aug 13, 2024

https://nextjs.org/docs/pages/building-your-application/data-fetching/incremental-static-regeneration

Good to know: The [edge runtime](https://nextjs.org/docs/pages/api-reference/edge) is currently not compatible with ISR, although you can leverage stale-while-revalidate by setting the cache-control header manually.

I have a few different ideas to solve this:

  1. When runtime is edge, and isr is enabled for a specific page, set the pages runtime to node. (could be breaking user code)
  2. When runtime is edge, and isr is enabled for a specific page, set the header 'Cache-Control', 's-maxage=15' in vercel config for the route. (maybe the the most simple for now)
  3. Allow setting the runtime in +config.js for a specific page, and document isr only works with the node runtime. Also allow setting cache-control headers in +config.js New setting headersResponse vikejs/vike#1803
@nitedani
Copy link
Contributor Author

I see there is a TODO already: 😄

// TODO
// edge: {
// env: { server: true },
// },
// headers: {
// env: { server: true },
// },

Maybe headers could be implemented in vike code.

@magne4000
Copy link
Owner

magne4000 commented Aug 13, 2024

My opinion is that if one has edge + ISR activated for an endpoint, it should be considered a configuration issue (and users should be warned accordingly). I just wonder if we should just warn the user and default to edge only, or just throw an error.

@nitedani
Copy link
Contributor Author

nitedani commented Aug 14, 2024

I agree, it's a configuration issue and we should throw an error. But I think we should also:

  • support caching on the edge(and node) runtime(with cache-control headers set through pages/somepage/+config.js)
  • let the user choose the runtime through pages/somepage/+config.js

For the second one, I think we can make two ssr bundles, one for edge and one for node? Or maybe we can get away with having only the edge bundle and just set the runtime to node for that page in .vc-config.json? I'm not sure if code bundled for the edge is fully compatible with node though. I think not.
But if that's too complex, headers support would be enough for now.

@magne4000
Copy link
Owner

For the second one, I think we can make two ssr bundles, one for edge and one for node

This should be fairly straightforward to implement, and I'll probably try this solution first.

support caching on the edge(and node)

I need to see what I can do about that. I probably will not be able to try this soon, so any help is welcome on that matter :)

@magne4000
Copy link
Owner

Vike now supports headers and edge config 🚀

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

No branches or pull requests

2 participants