fix: add specific rewrites for all SSR routes #1105
Conversation
|
✔️ Deploy Preview for netlify-plugin-nextjs-demo ready! 🔨 Explore the source changes: 8be8a32 🔍 Inspect the deploy log: https://app.netlify.com/sites/netlify-plugin-nextjs-demo/deploys/61dd5c637ecf480007a7dd5b 😎 Browse the preview: https://deploy-preview-1105--netlify-plugin-nextjs-demo.netlify.app |
|
✔️ Deploy Preview for netlify-plugin-nextjs-static-root-demo ready! 🔨 Explore the source changes: 8be8a32 🔍 Inspect the deploy log: https://app.netlify.com/sites/netlify-plugin-nextjs-static-root-demo/deploys/61dd5c63816e2d000852745c 😎 Browse the preview: https://deploy-preview-1105--netlify-plugin-nextjs-static-root-demo.netlify.app |
|
✔️ Deploy Preview for netlify-plugin-nextjs-nx-monorepo-demo ready! 🔨 Explore the source changes: 8be8a32 🔍 Inspect the deploy log: https://app.netlify.com/sites/netlify-plugin-nextjs-nx-monorepo-demo/deploys/61dd5c6314e55d0007c9307e 😎 Browse the preview: https://deploy-preview-1105--netlify-plugin-nextjs-nx-monorepo-demo.netlify.app/ |
|
✔️ Deploy Preview for netlify-plugin-nextjs-export-demo ready! 🔨 Explore the source changes: 8be8a32 🔍 Inspect the deploy log: https://app.netlify.com/sites/netlify-plugin-nextjs-export-demo/deploys/61dd5c637bf7110007b35c4d 😎 Browse the preview: https://deploy-preview-1105--netlify-plugin-nextjs-export-demo.netlify.app |
|
Test summary
Run details
View run in Cypress Dashboard ➡️ This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
|
Test summary
Run details
View run in Cypress Dashboard ➡️ This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
|
Test summary
Run details
View run in Cypress Dashboard ➡️ This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
ascorbic
force-pushed
the
mk/ssr-rewrites
branch
from
063e26d to
d9b550b
Compare
|
|
Summary
Currently (with some exceptions) we only add rewrites for routes that need to use an ODB, and rely on a catch-all to rewrite the rest to the regular handler. However this causes problems when an ODB dynamic route overrides a static SSR route, because it never falls-back to the catch-all. This leads to bugs due to it unexpectedly using an ODB, so having the wrong cache behaviour and missing query data etc.
This PR changes the rewrite handling to generate rewrites for every route, including SSR. These are done starting with the most specific, static routes and then the dynamic routes as defined in the routes manifest. We look these up in the prerender manifest so we can see which need ODB and which SSR handlers.
This does lead to a lot of rewrites, particularly for sites with i18n, because we need to generate separate rewrites for all of them. There may be a way to simplify these rewrites, collapsing some into single rules. However I don't want to attempt this until we have better tests.
I have been working on tests, but have had to abandon my original plan because it's really hard to isolate them and just test the rewrites. It looks like the best bet will be to do a new integration test suite, where we do a full build, then use the plugin to generate a toml and run ntl dev, then use fetch and check the
x-render-modefor each page. We could in theory also do this with cypress. I will investigate further and do a follow-up PRTest plan
Relevant links (GitHub issues, Notion docs, etc.) or a picture of cute animal
Fixes #1077, fixes #1068
Standard checks:
🧪 Once merged, make sure to update the version if needed and that it was published correctly.