Skip to content

fix: serve static files from basePath #1850

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

Merged
merged 5 commits into from
Dec 22, 2022
Merged

fix: serve static files from basePath #1850

merged 5 commits into from
Dec 22, 2022

Conversation

ascorbic
Copy link
Contributor

@ascorbic ascorbic commented Dec 21, 2022

Summary

When a site defined a basePath, static files were being incorrectly served from the root rather than the basePath.

Test plan

  1. Open https://deploy-preview-1850--netlify-plugin-nextjs-basepath-demo.netlify.app/docs/favicon.ico

Relevant links (GitHub issues, Notion docs, etc.) or a picture of cute animal

Fixes #1849
messi + capy

Standard checks:

  • Check the Deploy Preview's Demo site for your PR's functionality
  • Add docs when necessary

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

@netlify
Copy link

netlify bot commented Dec 21, 2022

Deploy Preview for netlify-plugin-nextjs-export-demo canceled.

Name Link
🔨 Latest commit 7c04c31
🔍 Latest deploy log https://app.netlify.com/sites/netlify-plugin-nextjs-export-demo/deploys/63a415214b3f6e00073f7f56

@netlify
Copy link

netlify bot commented Dec 21, 2022

Deploy Preview for netlify-plugin-nextjs-static-root-demo canceled.

Name Link
🔨 Latest commit 7c04c31
🔍 Latest deploy log https://app.netlify.com/sites/netlify-plugin-nextjs-static-root-demo/deploys/63a41521e7b3280008d5d357

@github-actions github-actions bot added the type: bug code to address defects in shipped code label Dec 21, 2022
@netlify
Copy link

netlify bot commented Dec 21, 2022

Deploy Preview for netlify-plugin-nextjs-nx-monorepo-demo canceled.

Name Link
🔨 Latest commit 7c04c31
🔍 Latest deploy log https://app.netlify.com/sites/netlify-plugin-nextjs-nx-monorepo-demo/deploys/63a41521631bce000981f5bb

@netlify
Copy link

netlify bot commented Dec 21, 2022

Deploy Preview for next-plugin-edge-middleware canceled.

Name Link
🔨 Latest commit 7c04c31
🔍 Latest deploy log https://app.netlify.com/sites/next-plugin-edge-middleware/deploys/63a415229ebcbd0008599a92

@netlify
Copy link

netlify bot commented Dec 21, 2022

Deploy Preview for next-plugin-canary canceled.

Name Link
🔨 Latest commit 7c04c31
🔍 Latest deploy log https://app.netlify.com/sites/next-plugin-canary/deploys/63a41522631bce000981f5c5

@netlify
Copy link

netlify bot commented Dec 21, 2022

Deploy Preview for netlify-plugin-nextjs-demo canceled.

Name Link
🔨 Latest commit 7c04c31
🔍 Latest deploy log https://app.netlify.com/sites/netlify-plugin-nextjs-demo/deploys/63a415224b3f6e00073f7f60

@netlify
Copy link

netlify bot commented Dec 21, 2022

Deploy Preview for nextjs-plugin-custom-routes-demo canceled.

Name Link
🔨 Latest commit 7c04c31
🔍 Latest deploy log https://app.netlify.com/sites/nextjs-plugin-custom-routes-demo/deploys/63a415229d2bde0008f28664

@netlify
Copy link

netlify bot commented Dec 21, 2022

Deploy Preview for netlify-plugin-nextjs-next-auth-demo canceled.

Name Link
🔨 Latest commit 7c04c31
🔍 Latest deploy log https://app.netlify.com/sites/netlify-plugin-nextjs-next-auth-demo/deploys/63a415224b3f6e00073f7f5b

@netlify
Copy link

netlify bot commented Dec 21, 2022

Deploy Preview for next-i18next-demo canceled.

Name Link
🔨 Latest commit 7c04c31
🔍 Latest deploy log https://app.netlify.com/sites/next-i18next-demo/deploys/63a41522e7b3280008d5d35c

@ascorbic ascorbic marked this pull request as ready for review December 21, 2022 11:31
@ascorbic ascorbic requested a review from a team December 21, 2022 11:31
@ascorbic
Copy link
Contributor Author

Re-running the e2e tests now that the incident is mitigated

@Flaysh
Copy link

Flaysh commented Dec 21, 2022

Love it!! Thank you so much!

Copy link

@nickytonline nickytonline left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works as expected. Thanks @ascorbic! 🚀

@nickytonline
Copy link

Looks like there are some E2E tests failing.

@ascorbic
Copy link
Contributor Author

This is "should have correct route params for chained rewrite from middleware to config rewrite", which @orinokai thought was related to the Next update, but seems to be failing in main too. As it's unrelated I don't think it's a blocker, and I'll investigate it properly tomorrow

@ascorbic ascorbic marked this pull request as draft December 22, 2022 08:27
@ascorbic ascorbic marked this pull request as ready for review December 22, 2022 08:27
@kodiakhq kodiakhq bot removed the automerge label Dec 22, 2022
@kodiakhq
Copy link
Contributor

kodiakhq bot commented Dec 22, 2022

This PR currently has a merge conflict. Please resolve this and then re-add the automerge label.

@kodiakhq kodiakhq bot merged commit d4ff894 into main Dec 22, 2022
@kodiakhq kodiakhq bot deleted the mk/static-basepath branch December 22, 2022 08:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge type: bug code to address defects in shipped code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Help wanted]: Some of public folder assets is not accessable when deploying NextJS app with basePath
5 participants