Fix: Legacy fallback env var #2074
Conversation
✅ 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-export-demo 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 netlify-plugin-nextjs-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-i18next-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ 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 next-plugin-canary 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. |
test/helpers/utils.spec.ts
Outdated
| }, | ||
| ] | ||
|
|
||
| const middleware = await getMiddleware(path.resolve('.next')) |
There was a problem hiding this comment.
Does the middleware affect the tests in any way? If so consider tests with and without it.
There was a problem hiding this comment.
I think if we wanted to test the matchesMiddleware functionality then it would matter. For these tests, we can just set the middleware to default to an empty array so that we can reach the logic we need to test related to the env var.
| } else if ( | ||
| prerenderedDynamicRoutes[route.page].fallback === false && | ||
| !is404Isr && | ||
| !process.env.LEGACY_FALLBACK_FALSE |
There was a problem hiding this comment.
This won't work if folks use LEGACY_FALLBACK_FALSE = 'false' so we need to test for strings too. In other areas of the Runtime we've used destr so I'd suggest that as a convention: https://github.com/netlify/next-runtime/blob/split-api-routes/packages/runtime/src/index.ts#L106
There was a problem hiding this comment.
Thanks for the heads up Rob and the ref code.
Co-authored-by: Nick Taylor <nick@iamdeveloper.com>
Co-authored-by: Nick Taylor <nick@iamdeveloper.com>
|
Linting in unit tests isn't happy |
Summary
This PR allows users to opt-in to the legacy behavior for dynamic routes set to
fallback:falseby adding the env varLEGACY_FALLBACK_FALSE='true'. When hitting a non-existent path on a dynamic route, it will hit the origin server which will give users the ability to add redirects for those paths.closes https://github.com/netlify/pod-ecosystem-frameworks/issues/446
Test plan
LEGACY_FALLBACK_FALSE='true'x-nf-render-mode:odbRelevant links (GitHub issues, Notion docs, etc.) or a picture of cute animal
Standard checks:
🧪 Once merged, make sure to update the version if needed and that it was published correctly.