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

adding note that edge api routes are not supported with ISR #43572

Merged
merged 4 commits into from Dec 1, 2022

Conversation

timeyoutakeit
Copy link
Contributor

Bug

  • Related issues linked using fixes #number
  • Integration tests added
  • Errors have a helpful link attached, see contributing.md

Feature

  • Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
  • Related issues linked using fixes #number
  • e2e tests added
  • Documentation added
  • Telemetry added. In case of a feature if it's used or not.
  • Errors have a helpful link attached, see contributing.md

Documentation / Examples

  • Make sure the linting passes by running pnpm build && pnpm lint
  • The "examples guidelines" are followed from our contributing doc

Copy link
Member

@ijjk ijjk left a comment

Choose a reason for hiding this comment

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

Thanks!

@ijjk ijjk merged commit 4ca12bd into vercel:canary Dec 1, 2022
@@ -27,6 +27,8 @@ description: 'Learn how to create or update static pages at runtime with Increme

Next.js allows you to create or update static pages _after_ you’ve built your site. Incremental Static Regeneration (ISR) enables you to use static-generation on a per-page basis, **without needing to rebuild the entire site**. With ISR, you can retain the benefits of static while scaling to millions of pages.

> Note: the `experimental-edge` runtime (https://nextjs.org/docs/api-reference/edge-runtime) is currently not compatible with ISR although can leverage `stale-while-revalidate` by setting the `cache-control` header manually.
Copy link
Member

@leerob leerob Dec 1, 2022

Choose a reason for hiding this comment

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

I think this could be a bit confusing: ISR isn't just caching!

Note: The Edge Runtime is not currently compatible with ISR. Ensure you are using the default Node.js runtime with ISR.

This note is also pretty high up on the page for something I don't think most people need to know.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm yeah it's not just caching although the cache-control header provides an alternative in the mean-time while it's implemented and I don't think we want to burry this note for users who are confused by this.

We potentially could make this a build time warning instead.

Copy link
Member

Choose a reason for hiding this comment

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

Build time warning makes sense to me 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

What does "not currently compatible" look like? Will it just never update? Will it error? Will it do nothing?

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants