fix(sitemap): exclude pages with robots noindex from sitemap #7143
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-7143--docusaurus-2.netlify.app/ |
|
Size Change: -25 B (0%) Total Size: 798 kB ℹ️ View Unchanged
|
Josh-Cena
added
the
status: awaiting review
| @@ -9,8 +9,6 @@ import type {LoadContext, Plugin} from '@docusaurus/types'; | |||
| import {docuHash, normalizeUrl, posixPath} from '@docusaurus/utils'; | |||
| import path from 'path'; | |||
|
|
|||
| export const routeBasePath = '__docusaurus/debug'; | |||
There was a problem hiding this comment.
doesn't it make sense to keep this part?
There was a problem hiding this comment.
At least not as a public API. I also don't find this abstraction particularly useful
| await plugin.postBuild({...props, content: plugin.content}); | ||
| await plugin.postBuild({ | ||
| ...props, | ||
| helmet: headTags, |
There was a problem hiding this comment.
maybe rename to head or headTags?
Not fan of using "helmet" name here, lib is an implementation detail and we already use "head" name in other places
Also not fan of the API surface it introduces.
Users shouldn't use something like helmet[route]?.meta.toComponent(), this is a quite confusing that we'd struggle to document well and ensure retrocompatibility on.
Maybe we could only expose a few route attributes and transform all HelmetDatum types to a convenient format, easy to document?
|
Not a fan of the API design either, but I can't find a nicer abstraction of the helmet state in a structured way. |
Josh-Cena
force-pushed
the
jc/sitemap-exclude-noindex
branch
from
759d867
to
4f3d7b3
Compare
I suggest we move on and merge this as it's a nice improvement for internal usage in the sitemaps plugin We can document this behind an experimental flag, warn it may change and recommend to use "head" only in site plugins, not public/open-source plugins? We don't flag any feature as experimental for now but we probably could as we have a few undocumented apis (like |
|
We also have a |
🤷♂️ we might as well keep |
|
I guess we can keep all this undocumented for now, let's merge as is 👍 Hopefull if someone plans to use this they'll reach out and explain their use-case |
Josh-Cena
removed
the
status: awaiting review
Motivation
fix #6247. I'm not sure if providing the entire helmet state would be too big as part of the plugin API, but currently it's the only way to get structured data about the head state. Otherwise, we can only export a string and then parse it again in
postBuild, which sounds like too much overhead.Have you read the Contributing Guidelines on pull requests?
Yes
Test Plan
I reverted #7122 and added
<meta name="robots" content="noindex" />to the debug layout. It turns out that these routes are still not included in the sitemap.