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
Separate nofollow from noindex tag #117
Conversation
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.
👏 @nik-john very well thought out PR, thank you for the contribution.
Just a couple of typos and a suggestion for one more test, but I can add that later if you don't have time.
@@ -6,6 +6,7 @@ export default () => ( | |||
<> | |||
<NextSeo | |||
noindex={true} | |||
nofollow={true} |
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.
Could you add a quick check to seo.spec.js > SEO overrides apply correctly
to ensure this being set correctly?
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.
@garmeeh I added nofollow
to the spec already - do you want me to update it to go index,nofollow
or noindex,follow
?
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.
😅 Oh yeah, it was already set to nofollow
my bad.
@garmeeh I've updated the description blurb as well as the README comments (should've caught the typos myself 🤦♂) Do let me know if you want anything else added to the tests Btw, I really love the level of test coverage in this lib 💯 |
😅 Ah no worries, I would be surprised if there is not a few typos in the docs from myself. |
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.
Looks good to me 💪
@all-contributors please add @nik-john docs, code, test |
I've put up a pull request to add @nik-john! 🎉 |
@nik-john 🎉 |
Description of Change(s):
According to Google SEO Specs for the
robots
meta tag,noindex
andnofollow
values can be set separately. Currently, settingsno-index
automatically also setsnofollow
as observed #77. A real world example is a website with an internal directory of links that the dev wants google to crawl but not index (no one wants the directory page to show up on the Google Search Results Page). Currently there is no good way of doing this in Next SEO.This PR separates the two into their own props. As updated in the readme the behavior will be as follows:
noindex
nofollow
meta
content ofrobots
,googlebot
index,follow
(default)index,follow
noindex,follow
noindex,follow
index,nofollow
index,nofollow
noindex,nofollow
One thing that came to mind was that codebases that currently use
next-seo
and have setnoindex
to meannoindex,nofollow
will have regression issues because when this PR is merged and the dev updates the version on their codebase,will mean
which is necessarily not something that people might want.
Therefore we will need to mark this as a breaking change and require the user to update the above code to