-
-
Notifications
You must be signed in to change notification settings - Fork 8.7k
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
feat(blog): slug/pathname aliases for blog posts #8356
base: main
Are you sure you want to change the base?
Conversation
metadata.aliases.forEach((alias) => { | ||
addRoute({ | ||
path: alias, | ||
component: blogPostComponent, |
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.
IIUC, this would create two pages with the same content? Isn't it simpler to set up redirects instead?
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.
Hey @Josh-Cena,
The context for this is in @slorber's comment here: #8311 (reply in thread)
--- slug: /2022/11/17/azure-ad-claims-static-web-apps-azure-functions aliases: ["/"] # NEW FEATURE --- # Azure AD Claims with Static Web Apps and Azure Functions TextThe aliasing feature would create extra pages for another slug (that can be /)
It gives the ability to keep a shared canonical URL for both pages and avoid SEO content being flagged as duplicate.
Does this make sense?
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.
yes that makes sense to me, and we'll likely want to add something similar to individual docs for consistency
✅ [V2]Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
⚡️ Lighthouse report for the deploy preview of this PR
|
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.
Thanks, added some comments
We should rather have some dogfood cases to see it in action.
Note that we should also take care of the UI aspect.
For example, suppose the blog sidebar highlights the currently browsed blog post. In that case, it should probably keep being highlighted if we browse it from the permalink URL or any of the aliases. The theme code probably doesn't account for this yet. There are also things to consider like navbar items highlighting, although people rarely link to blog post URLs from the navbar and can use the regexp active fn.
An alternative implementation that is likely faster than rendering twice the same blog post could be to just copy the static file in the build output, inside the postBuild
hook?
cp build/blog/folder/myBlogPost.html build/blog/another-folder/myBlogPostAlias.html
Pro: much faster if there are many aliases (unlikely? 🤔 )
Cons: worst DX, aliases not working in dev unless we implement something. A Hybrid solution could also make sense.
We can probably think about perf later, and current implementation is likely good enough.
{aliases: ['../blog']}, | ||
{aliases: ['../../blog']}, | ||
{aliases: ['/api/plugins/@docusaurus/plugin-debug']}, | ||
{aliases: ['@site/api/asset/image.png']}, |
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.
I'm not sure we should accept such value 😅
This code is unlikely to yield a great pathname
normalizeUrl([baseUrl, routeBasePath, alias])
and the tests do not cover these edge cases.
The question to me is: do we want to support relative aliases?
Is this really useful? I mean, you create a relative alias, then update the slug, and then you end up with a different alias. That might be a source of bug/confusion?
If we do want relative aliases, there's a resolvePathname
util that could probably be applied to compute the final non-relative pathname of the alias.
But I would suggest to keep it simple for now and only accept absolute pathname aliases in validation? If someone wants a relative alias someday and has great reason to add it, we'll do an update.
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.
The question to me is: do we want to support relative aliases?
Probably not - will amend
packages/docusaurus-plugin-content-blog/src/__tests__/frontMatter.test.ts
Outdated
Show resolved
Hide resolved
metadata.aliases.forEach((alias) => { | ||
addRoute({ | ||
path: alias, | ||
component: blogPostComponent, |
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.
yes that makes sense to me, and we'll likely want to add something similar to individual docs for consistency
packages/docusaurus-plugin-content-blog/src/plugin-content-blog.d.ts
Outdated
Show resolved
Hide resolved
Thanks for the comments - I'll take a look at them. But before doing I wanted to get your thoughts on the below: (copied from the motivation section up top). Given that this will likely change implementation somewhat, I wanted to tackle that before the code comments. Hope this makes sense! MotivationIt would be great to have alias URLs for blog posts, so as well as Ideally the canonical link remains pointing to the permalink. HOWEVER I'M NOT SURE HOW BEST TO IMPLEMENT THIS PART Consider: docusaurus/packages/docusaurus-theme-classic/src/theme/SiteMetadata/index.tsx Lines 58 to 65 in ca3dba5
The canonical URL is driven just by the pathname of the page. There's not an obvious way to provide a canonical URL to this without serious code changes. Rather than do this unsupervised, I wanted to ask if there was an approach that might be good approach to take? |
I guess this logic should only be used for as a fallback for pages that do not define an explicit canonical URL (which is likely the case for user-created standalone pages) If a content-plugin supports the aliasing feature then it probably needs to declare the canonical url deeper in the tree, to override this fallback value. That probably makes sense to add it to the PageMetadata helper considering it sets 2 meta at once and could be reused in the future It is ok to declare the same meta multiple times in the tree, Head is supposed to deduplicate it. |
Pre-flight checklist
Motivation
It would be great to have alias URLs for blog posts, so as well as
/my-blog-post
you could have/my-other-url-for-blog-post
.Ideally the canonical link remains pointing to the permalink. HOWEVER I'M NOT SURE HOW BEST TO IMPLEMENT THIS PART
Consider:
docusaurus/packages/docusaurus-theme-classic/src/theme/SiteMetadata/index.tsx
Lines 58 to 65 in ca3dba5
The canonical URL is driven just by the pathname of the page. There's not an obvious way to provide a canonical URL to this without serious code changes. Rather than do this unsupervised, I wanted to ask if there was an approach that might be good approach to take?
Test Plan
Please see the new tests / amended tests.
Test links
Deploy preview: https://deploy-preview-8356--docusaurus-2.netlify.app/
Related issues/PRs
#8311