-
-
Notifications
You must be signed in to change notification settings - Fork 8.6k
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
fix(core): fix docusaurus serve
broken for assets when using trailingSlash
#10142
Conversation
✅ [V2]
To edit notification comments on pull requests, go to your Netlify site configuration. |
⚡️ Lighthouse report for the deploy preview of this PR
|
Size Change: 0 B Total Size: 1.71 MB ℹ️ View Unchanged
|
docusaurus serve
is broken for assetsdocusaurus serve
broken for assets when using trailingSlash
if (baseUrl !== '/') { | ||
// Not super robust, but should be good enough for our use case | ||
// See https://github.com/facebook/docusaurus/pull/10090 | ||
const looksLikeAsset = !!req.url.match(/.[a-z]{1,4}$/); |
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.
Maybe *.mp3
?
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.
Good point, not sure why I assumed extensions could only contain lowercase letters 😅
This fixed our trailing slash bug via facebook/docusaurus#10142 Signed-off-by: Tim Smith <[email protected]>
This fixed our trailing slash bug via facebook/docusaurus#10142 Signed-off-by: Tim Smith <[email protected]>
Motivation
Fix #10139
The workaround applied in #10090 lead to a regression for serving assets. I'm now applying this workaround more defensively.
The workaround impl is not 100% robust but should work fine for 99.9% of websites and only applies to sites using
docusauris serve
with a baseurl + trailingSlash.Test Plan
no 😅 it works locally