-
-
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): prevent useBaseUrl returning /base/base when on /base #6993
Conversation
✅ [V2]
To edit notification comments on pull requests, go to your Netlify site settings. |
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-6993--docusaurus-2.netlify.app/ |
Size Change: +36 B (0%) Total Size: 805 kB
ℹ️ View Unchanged
|
I'm not sure to understand how this fixes the other issue. To me the problem is that if user use expect(mockUseBaseUrl('/docusaurus')).toBe('/docusaurus/docusaurus'); (which we prevented on purpose as a workaround/security) instead of expect(mockUseBaseUrl('/docusaurus')).toBe('/docusaurus/'); Can we build our own site with |
No, this one fixes the other issue, which is exactly that the base URL is prepended twice, because we only check |
oh I see, it's not a fix for https://github.com/facebook/docusaurus/issues/6294🤦♂️ |
👍 that looks good yes |
But in practice does it really solve the problem? Because afaik the problem is when accessing the site on Have you tested this with a host that serves Docusaurus with We may need a client-side redirect or something |
This is only about the canonical and nothing else (not about content hoster serving the right file or redirecting). The problem is that if you are on |
👍 agree, this fixes the issue above Although serving the site through |
Motivation
Fix #6315. It's a pretty trivial fix; not sure if a better solution exists. Since we have this duplicate base URL detection anyways, this fix only exists to make the behavior more consistent. Related to #6294
Have you read the Contributing Guidelines on pull requests?
Yes
Test Plan
Added two tests