fix(v2): respect baseUrl in serving command #4924
Conversation
|
✔️ [V2] 🔨 Explore the source changes: da26cf5 🔍 Inspect the deploy log: https://app.netlify.com/sites/docusaurus-2/deploys/60c0d48db89505000798bf76 😎 Browse the preview: https://deploy-preview-4924--docusaurus-2.netlify.app |
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-4924--docusaurus-2.netlify.app/ |
|
✔️ [V1] 🔨 Explore the source changes: da26cf5 🔍 Inspect the deploy log: https://app.netlify.com/sites/docusaurus-1/deploys/60c0d48de639c6000801b91d 😎 Browse the preview: https://deploy-preview-4924--docusaurus-1.netlify.app |
|
Size Change: +85 B (0%) Total Size: 620 kB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Thanks @lex111 , looks like the solution to modify req.url that Anshul suggested works fine.
Was able to get it working with serve so was wondering why you added an extra lib?
Kept your modifications for now but suggesting a slightly different implementation and only using serve, what do you think?
packages/docusaurus/package.json
Outdated
| @@ -79,6 +79,7 @@ | |||
| "html-minifier-terser": "^5.1.1", | |||
| "html-tags": "^3.1.0", | |||
| "html-webpack-plugin": "^5.3.1", | |||
| "http-proxy": "^1.18.1", | |||
There was a problem hiding this comment.
not too fan of introducing another static asset serve library for a specific case
those 2 libs may behave differently regarding a few things like pretty URLs, trailing slashes etc so I'd rather keep only one
| const server = http.createServer((req, res) => { | ||
| serveHandler(req, res, { | ||
| // Automatically redirect requests to /baseUrl/ | ||
| if (!req.url?.startsWith(baseUrl)) { |
There was a problem hiding this comment.
if user access "/" but there is /baseUrl/, user could be automatically redirected to that /baseUrl/, I find this a convenient way to avoid potential bug reports (even if the log is quite clear...)
| */ | ||
|
|
||
| // Remove baseUrl before calling serveHandler | ||
| req.url = req.url?.replace(baseUrl, '/'); |
There was a problem hiding this comment.
no need for any if here, / could be replaced by / (noop)
|
I thought this was a working solution, but if you can avoid using a proxy server, then I agree with that. |
|
great, so I'll just do some cleanup and merge, thanks |
Motivation
Resolves #3291
Have you read the Contributing Guidelines on pull requests?
Yes
Test Plan
Set config field
baseUrlwith value other than "/" and runyarn servecommand.Related PRs
(If this PR adds or changes functionality, please take some time to update the docs at https://github.com/facebook/docusaurus, and link to your PR here.)