-
-
Notifications
You must be signed in to change notification settings - Fork 8.8k
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 trailing slash issues (depend on hosting provider) #3372
Comments
Seems related to markdalgleish/static-site-generator-webpack-plugin#53 |
/myDoc.html
instead of /myDoc/index.html
/myDoc.html
instead of /myDoc/index.html
@slorber fwiw this is fixable on Netlify by changing your post-processing settings. They have a UI bug, where the master checkbox to disable post-processing does not disable the pretty URL setting, and that is what causes e.g. |
Thanks @lunelson , didn't know about this bug, do you have a link to track this? Also, I tested these options on my own website, but still have a trailing slash issue 😅 https://sebastienlorber.com/records-and-tuples-for-react |
@slorber If you changed the options and tested your site right away, it might not have worked: the changes seem to take a few minutes to propagate. For me the URL above now works without redirecting to a trailing slash. And no I don't know of an issue that tracks it at Netlify but I've seen it come up in discussions here and on Gatsby about trailing slash problems. |
@slorber Having pointed that out however, I must say that in my company's websites (including 3 Docusaurus v2 sites and potentially soon more), we will probably turn this Netlify feature back on. This is because we have had the strong recommendation from an SEO agency that—while neither format (trailing-slash or not) is "better"—there should be 301 redirects to make sure that only one of the two formats is getting crawled and indexed. And because Netlify treats these URL formats identically, and its redirects system therefore doesn't allow you to 301 from one to the other (it would create an infinite loop), our only alternative is to use this pretty URLs feature (which does create a 301) and to accept the decision which is made for us, namely that trailing-slash will be the final format. This creates a challenge for me right now because currently Docusaurus generates all URL patterns in router/anchor links, sitemap and canonical link tags without trailing slashes 😬 |
The sitemap can be generated with trailing slashes, that's what the option |
@mpsq well, that's one down at least 😅 |
Yes :) On your list, "URL patterns in router/anchor links" and "canonical link tags" are actually the same thing, they depend on how the EDIT: |
YES. This would be ideal. @mpsq is there any chance you could give me an idea of where I need to intervene in my custom theme, to modify that |
You could do it in your theme by tweaking |
Again: yes |
Thanks, didn't notice but it seems my site does not have trailing slashes anymore :) But anyway we should figure out a portable solution that works even on GithubPages where you don't have many hosting options to tweak... still believe that my initial proposal might be a better solution, need to find time to test this on multiple hosts. For workarounds to override the permalink, you may be interested by this issue: #3501 (comment) Not sure but something like that could work to customize the permalink: import React from 'react';
import OriginalLayout from '@theme-original/Layout';
import Head from '@docusaurus/Head';
import {useLocation} from '@docusaurus/router';
export default function Layout(props) {
const location = useLocation();
return (
<>
<OriginalLayout {...props} />
<Head>
<meta
property="canonical_url"
content={location.pathname + "/"}
/>
</Head>
</>
);
} |
@slorber thanks, I'll take a look at that too. One thing about the router and this URL format issue that I haven't tested yet, is whether active link class matching will still work if the router paths are also forced to have trailing-slashes... |
Notes:
We should probably add 2 new options:
We can't add a trailing / by default, not everybody is willing to have / at the end of the URLs (legacy/SEO reasons) Another idea is to resolve all relative links at build time to absolute links, so that the presence of a trailing slash or not does not impact in any way the link target. On Netlify, disabling the Pretty Url option prevent Netlify from adding the trailing slash, yet if user visits the page with a trailing slash, it is not removed client-side, and still potentially breaks relative links. |
/myDoc.html
instead of /myDoc/index.html
Until I figure how to integrate this properly in Docusaurus, here's a PR on the ReactNative website with some ideas to fix trailing slashes issues in userland: facebook/react-native-website#2297 |
I've been solving this issue mainly by conforming URLs to the desired format using a function within the |
Having an option to generate |
This is probably the easiest, safest, and with least consequences. It is probably also the fastest! Any downsides here? It might involve replacing domain with localhost:$PORT for |
Is there an easy workaround (i.e. that can be done on the website repo without changing docusaurus itself or dependencies) to force a trailing slash on all links and URLs? |
@slorber Can you share some instructions for trying this out? So far, I've tried adding the sitePlugin.js to a PS: I'm using Netlify and I'm trying to avoid the duplicate URLs issue. Currently, the site crawlers are seeing two URLs, one without slashes (present in the pages that are build by D2) and with slashes (which Netlify prefers and tries redirecting to when it sees a URL without a slash). |
I double-down on what @lunelson said. For users who are using Netlify, we need the final build to be using trailing-slashes as that decision is made for us, and turning on/off the "Pretty URLs" setting does not help. |
@agentofuser not available now, but under consideration |
@thehappybug are you even sure the plugin runs in the first place? Have you tried adding logs so that you know which files are created? Note you can also run your own script with node after
I claim another time that this is false: disabling pretty URLs works reliably on Netlify and there won't be any redirect. VERY IMPORTANT: the global "disable asset processing" checkbox is broken and does not really disable pretty URLs: you have to uncheck it. Proof: as part of my study to solve this issue, I'm writing a trailing slash guide for the community. There's a live Netlify deployment with pretty URLs disable on which you can see for yourself there is no server-side redirect: https://github.com/slorber/trailing-slash-guide#netlify |
I'm not an SEO expert but I believe having 301 redirects is not mandatory if you have canonical URLs and you explain to the crawlers that the 2 URLs are the same page. Docusaurus sites have canonical URLs by default and crawlers shouldn't penalize your site for publishing duplicated content |
I've opened a draft PR to propose a solution: #4908 |
Thank you for your responses, @slorber. The trailing slash guide was quite useful.
I managed to get your solution working. I had previously made a mistake in registering plugins. For now, I'm not going forwards with deploying this solution because:
I hope this feedback helps. Let me know if I can help you test any future releases related to this issue. |
Thanks for the feedback, I understand why you would want to keep the exact same URLs you had before, and I'll make that possible. As soon as my PR is merged, you'd be able to use the |
As part of #4908 I'm implementing a new
Refer to https://github.com/slorber/trailing-slash-guide for how the files will be served by your host. I've created 3 deployments to test this feature:
Note: using true/false instead of undefined allows to use Netlify with pretty URLs on without having any annoying redirection, but it's not the case for the undefined behavior that will keep redirecting with pretty URLs on (default behavior): I disabled pretty URLs for undefined on purpose. PLEASE: help me review those deployments and let me know if you see any unwanted side-effects: now is a better time to complain than after merging the PR :) |
The PR has been merged, and you can try this right now with the https://www.npmjs.com/package/@docusaurus/core?activeTab=versions
Please let me know if anything does not work asap. Also, in general, can you please let me know:
According to your feedbacks, we may update the doc and recommend a |
Giscus relies on matching the pathName, so we need to ensure that the trailing slash behaves consistently Related tickets: facebook/docusaurus#3372
Giscus relies on matching the pathName, so we need to ensure that the trailing slash behaves consistently Related tickets: facebook/docusaurus#3372
Edit: as part of the analysis to solve this issue, I'm writing a guide to describe factually the behavior of various static site generators and static hosting providers: https://github.com/slorber/trailing-slash-guide
💥 Proposal
Fix trailing host slash issues: write
/myDoc.html
instead of/myDoc/index.html
We have a few issues that are related to docs trailing slashes: #2654, #2394, #2134, #3351, #3380
I suspect these issues are related to the Docusaurus output, as for a doc at
/docs/myDoc
we write/myDoc/index.html
instead of/myDoc.html
Always creating a subfolder is probably a signal for the hosting providers (Netlify, Github Pages...) to add a trailing slash automatically to URLs, which can be annoying.
It can break relative links as now the link resolution is different due to the trailing slash, and we end up with broken links in production, not catched at build time (see #3380)
This behavior is provided by a dependency: https://github.com/markdalgleish/static-site-generator-webpack-plugin/blob/master/index.js#L165
We already run on a fork of this dependency. I guess we could as well include a way to write
/myDoc.html
directly if a route path does not end with a trailing slash.As this is a risky change, I suggest making this optional. We can make it the default behavior (breaking change) later if it works fine for early adopters.
See also #2394
The text was updated successfully, but these errors were encountered: