-
Notifications
You must be signed in to change notification settings - Fork 93
chore: warn for incorrect _redirects file #1078
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
Conversation
✔️ Deploy Preview for netlify-plugin-nextjs-nx-monorepo-demo ready! 🔨 Explore the source changes: e23850a 🔍 Inspect the deploy log: https://app.netlify.com/sites/netlify-plugin-nextjs-nx-monorepo-demo/deploys/61eeb827fca6e6000781bb1f 😎 Browse the preview: https://deploy-preview-1078--netlify-plugin-nextjs-nx-monorepo-demo.netlify.app/ |
✔️ Deploy Preview for netlify-plugin-nextjs-static-root-demo ready! 🔨 Explore the source changes: e23850a 🔍 Inspect the deploy log: https://app.netlify.com/sites/netlify-plugin-nextjs-static-root-demo/deploys/61eeb827db0f0b00070a413a 😎 Browse the preview: https://deploy-preview-1078--netlify-plugin-nextjs-static-root-demo.netlify.app |
✔️ Deploy Preview for netlify-plugin-nextjs-demo ready! 🔨 Explore the source changes: e23850a 🔍 Inspect the deploy log: https://app.netlify.com/sites/netlify-plugin-nextjs-demo/deploys/61eeb8271c36a80007274538 😎 Browse the preview: https://deploy-preview-1078--netlify-plugin-nextjs-demo.netlify.app |
✔️ Deploy Preview for netlify-plugin-nextjs-export-demo ready! 🔨 Explore the source changes: e23850a 🔍 Inspect the deploy log: https://app.netlify.com/sites/netlify-plugin-nextjs-export-demo/deploys/61eeb827bf30bf00085b5b41 😎 Browse the preview: https://deploy-preview-1078--netlify-plugin-nextjs-export-demo.netlify.app |
Test summaryRun 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 summaryRun 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 summaryRun 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 definitely a small helpful change to the docs
if (existsSync(join(appDir, '_redirects'))) { | ||
console.log( | ||
yellowBright( | ||
`You have a "_redirects" file in your root directory, which is not deployed and will be ignored. If you want it to be used, please move it into "public".`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🙌
3d43d3d
to
d27b539
Compare
Test summaryRun 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 summaryRun 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 summaryRun 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 |
@@ -187,4 +187,14 @@ export const warnForProblematicUserRewrites = ({ | |||
`), | |||
) | |||
} | |||
|
|||
export const warnForRootRedirects = ({ appDir }: { appDir: string }) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
export const warnForRootRedirects = ({ appDir }: { appDir: string }) => { | |
export const warnForRootRedirects = ({ appDir }: { appDir: string }): void => { |
Summary
Adds a warning for
_redirects
files in the site root. We used to silently add these to the user's redirects, but we now follow the documented behaviour, which is that the files should go in the publish dir not site root. #1130 adds this to the docs, and the CLI warns about it too, but this adds Next.js-specific logs, with instructions on where it needs to go.Test plan
_redirects
file to the root of a demo siteRelevant links (GitHub issues, Notion docs, etc.) or a picture of cute animal
Issue that shows the problem: #1008

Standard checks:
🧪 Once merged, make sure to update the version if needed and that it was published correctly.