fix: correctly handle ISR for appDir pages #1855
Conversation
✅ Deploy Preview for netlify-plugin-nextjs-nx-monorepo-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for netlify-plugin-nextjs-export-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for netlify-plugin-nextjs-static-root-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for netlify-plugin-nextjs-next-auth-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for next-plugin-edge-middleware ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for nextjs-plugin-custom-routes-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for next-i18next-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for netlify-plugin-nextjs-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for next-plugin-canary ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
ascorbic
force-pushed
the
mk/rsc-no-isr
branch
from
8823a86 to
ce6c963
Compare
|
When is this getting merged? |
ascorbic
force-pushed
the
mk/rsc-no-isr
branch
2 times, most recently
from
f5d1014 to
884d3be
Compare
ascorbic
force-pushed
the
mk/rsc-no-isr
branch
from
884d3be to
19cba0d
Compare
ascorbic
force-pushed
the
mk/rsc-no-isr
branch
from
c8ed4ee to
c35de7b
Compare
| @@ -15,32 +15,19 @@ jobs: | |||
| strategy: | |||
| matrix: | |||
| os: [ubuntu-latest, macOS-latest, windows-latest] | |||
| node-version: [14, '*'] | |||
There was a problem hiding this comment.
AppDir requires 16+
ascorbic
force-pushed
the
mk/rsc-no-isr
branch
from
bc74b4b to
7faf80d
Compare
| }) | ||
|
|
||
| // This needs trailing slash handling to be fixed | ||
| it.skip('correctly redirects HTML requests for static pages', () => { |
There was a problem hiding this comment.
Prerendered pages do the normal Netlify trailing slash handling. This shoudl work once we have the trialing slash edge funciton
| to: ODB_FUNCTION_PATH, | ||
| force: true, | ||
| buildId, | ||
| dataRoute: isAppDir ? dataRoute : null, |
There was a problem hiding this comment.
appDir routes provide their own data route value
ascorbic
changed the title
|
@ascorbic is there a reason we can't just not normalise the request? so a request to /blog will be rewritten to /blog.rsc and a request to /blog/ will be rewritten to /blog.rsc/ it would make it easier to reason about when looking at logs, etc. but let me know if i've misunderstood something |
|
I tried that at first, and it didn't work. I can't for the life of me rememebr why! Let me see if I can reproduce it. You're right it would be simpler. I'm now wondering if other changes I made sinc ethen will make it work. |
|
@orinokai OK, so it didn't work right away, but with a few changes it does! Good suggestion. I've updated the description too. |
| route: string | ||
| dataRoute: string | ||
| withData: boolean | ||
| }) => [...(withData ? toNetlifyRoute(dataRoute) : []), ...toNetlifyRoute(route)] |
There was a problem hiding this comment.
Will be so good to replace all these helper functions with edge router!
There was a problem hiding this comment.
💯 The number of times I've thought that while working on this!
|
@gpoole yes, this is related to internal changes in Next.js. We're working on some major changes that will fix this and many more issues. Beta will be available soon. |
|
That's great, thanks for the update 👍
|
Summary
Data routes are handled differently for appDir pages. Instead of a JSON file in
/_next/data, the request is made to the same path, passing theRSCheader. This returns the RSC data to stream the update. The fact this varies on a header means we can't use simple ODB handling for these pages, because it will lead to errors like #1844, where the RSC data is returned when HTML is requested, or vice versa.This PR fixes this by adding an edge function which routes appDir requests that include the
rscheader to the appropriate.rscpath instead. The original request path is put into a header, which then replaces the.rscpath in the handler. These are real files but not normally requested directly. The reason for doing this is so that these requests have unique URLs, meaning they're cached separately as ODBs.There's a final extra part that is needed, which is to handle trailing slashes. If we normalise all of the requests to the matching
.rscfile, then there will be no difference to requests with and without the trailing slash. This is a problem, because depending on whether the version with or without the slash is requested first, we may cache the redirect instead of the actual data (or vice versa). To handle this, we do a bit of trickery. If the request has a trailing slash, we include a trailing slash on the rewritten.rscpath. For example, while an rsc request to/blogwill be rewritten to/blog.rsc, a request for/blog/(with trailing slash) will be rewritten to/blog.rsc/. This means that it will be treated as a separate ODB path, and will be cached differently.Basically, all of this is to implement
Varyin ODBs. 🤷🏻Test plan
application/octet-streamRSC data./blogpages directly and ensure that HTML is returnedRelevant links (GitHub issues, Notion docs, etc.) or a picture of cute animal
Fixes #1844
Standard checks:
🧪 Once merged, make sure to update the version if needed and that it was published correctly.