fix: ensure isr conditional 404 fix works for next 11 #1765
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-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 next-plugin-edge-middleware 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-canary 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-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 nextjs-plugin-next-11-demo ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
| strategy: | ||
| fail-fast: false | ||
| matrix: | ||
| containers: [1, 2, 3, 4] |
There was a problem hiding this comment.
If there's not many tests that are included in this suite, it's likely not worth having so many containers dedicated to this. However, I'd leave it as a matrix so that as we add more tests it's really easy to add containers to keep the time to run the tests low.
| containers: [1, 2, 3, 4] | |
| containers: [1] |
| @@ -0,0 +1,4 @@ | |||
| { | |||
| "extends": "next", | |||
| "root": true | |||
There was a problem hiding this comment.
In case anyone else is interested in what this property does (because I didn't know offhand either), from the ESLint docs site:
By default, ESLint looks for configuration files in all parent folders up to the root directory. This can be useful if you want all of your projects to follow a certain convention, but can sometimes lead to unexpected results. To limit ESLint to a specific project, place "root": true inside the .eslintrc.* file or eslintConfig field of the package.json file or in the .eslintrc.* file at your project’s root level. ESLint stops looking in parent folders once it finds a configuration with "root": true.
@orinokai This is a bit of a nitpick, but it might be worth adding a small note here about ESLint not looking for configuration in parent directories past this point with this setting since this info is a little buried in the docs
| ignore = "if [ $CACHED_COMMIT_REF == $COMMIT_REF ]; then (exit 1); else git diff --quiet $CACHED_COMMIT_REF $COMMIT_REF ../..; fi;" | ||
|
|
||
| [[plugins]] | ||
| package = "@netlify/plugin-nextjs" |
There was a problem hiding this comment.
I don't think this needs to be included since
[[plugins]]
package = "@netlify/plugin-local-install-core"
[[plugins]]
package = "../plugin-wrapper"
is present in the file
|
Going to go ahead and close this for now as we've done other work for 404s in relation to ISR. |
Summary
A previous PR that fixed an issue with
fallback: falsealso patched the Next server so that 404 pages could return a SWR header and so ISR pages that conditionally 404 will still have a TTL. However, it seems this patch was not compatible with Next 11. This PR addresses that.Test plan
Tests have been added as a new Next 11 demo site and should run and pass.
Relevant links (GitHub issues, Notion docs, etc.) or a picture of cute animal
#1541