feat(sitemap): add ignorePatterns option #6979

ApsarasX · 2022-03-24T05:30:50Z

Motivation

When I generate a sitemap, I want to ignore some paths, such as the path starting with /tags, so I add an option named ignorePatterns to plugin-sitemap, which accepts an array of regular expressions and allows users to ignore some paths that do not need to appear in the sitemap.

Have you read the Contributing Guidelines on pull requests?

Yes.

Test Plan

I've added several test cases in the following files.

packages/docusaurus-plugin-sitemap/src/__tests__/createSitemap.test.ts
packages/docusaurus-plugin-sitemap/src/__tests__/options.test.ts.

Related PRs

No other PRs related to this request as far as I could find.

facebook-github-bot · 2022-03-24T05:30:54Z

Hi @ApsarasX!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

netlify · 2022-03-24T05:39:29Z

✅ [V2]

Built without sensitive environment variables

Name	Link
🔨 Latest commit	`188b76b`
🔍 Latest deploy log	https://app.netlify.com/sites/docusaurus-2/deploys/623ec159be30e600093d8892
😎 Deploy Preview	https://deploy-preview-6979--docusaurus-2.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

github-actions · 2022-03-24T05:39:58Z

⚡️ Lighthouse report for the changes in this PR:

Category	Score
🟠 Performance	62
🟢 Accessibility	100
🟢 Best practices	92
🟢 SEO	100
🟢 PWA	90

Lighthouse ran on https://deploy-preview-6979--docusaurus-2.netlify.app/

facebook-github-bot · 2022-03-24T06:48:25Z

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

Josh-Cena · 2022-03-24T06:54:20Z

packages/docusaurus-plugin-sitemap/src/options.ts

@@ -24,6 +25,9 @@ const PluginOptionSchema = Joi.object({
    .valid(...Object.values(EnumChangefreq))
    .default(DEFAULT_OPTIONS.changefreq),
  priority: Joi.number().min(0).max(1).default(DEFAULT_OPTIONS.priority),
+  ignorePatterns: Joi.array()
+    .items(Joi.object().instance(RegExp))


Can we allow strings as well, which will be glob patterns like /tags/**? We have a utils.createMatcher for that.

this runs in Node side so why not expose a callback as well (and thus rename to simply "ignore"?)

a callback is always the most flexible option

now I'm not against providing support for RegExp/globs too but all this can be implemented in userland with a callback and this is a quite niche feature 🤷‍♂️

I don't really like callback-based APIs unless we can't implement a good solution otherwise—Webpack loaders don't use callbacks everywhere only because they can. I do prefer serializable APIs if they provide the right level of abstraction. While ignorePatterns: ["/tags/**"] is probably as useful as ignore: (path) => path.startsWith("/tags/"), the former is more well-understood by everyone and more approachable for those not well-versed with JS.

🤷‍♂️ I don't know, it's also more ambiguous because some edge cases might depend on the underlying glob library etc... A callback always behaves explicitly, other apis are just shortcuts.

Not saying that we shouldn't provide shortcuts, but this use-case seems niche enough that a lower-level but more flexible API might be enough

I agree that glob patterns are better than regular expressions, and I've used utils.createMatcher instead of regular matching.

In addition, I don't quite agree to use the callback based API, because it will make docusaurus.config.js looks disorganized. And not all docusaurus users are professional programmers.

packages/docusaurus-plugin-sitemap/src/__tests__/options.test.ts

slorber · 2022-03-24T09:53:01Z

packages/docusaurus-plugin-sitemap/src/createSitemap.ts


  const sitemapStream = new SitemapStream({hostname});

  routesPaths
-    .filter((route) => !route.endsWith('404.html'))
+    .filter(


should we apply trailingSlash before the filtering? I don't know what is best in this case 🤷‍♂️

404s should not be applied trailing slash anyways, I think this is fine. The odds that someone gets tricked by this seems low to me, and they can always check their glob.

packages/docusaurus-plugin-sitemap/src/createSitemap.ts

slorber

Added some comments

Josh-Cena · 2022-03-24T13:34:21Z

That looks rather nice, thanks!

I will do a few refactors, hold back while I commit 😄

ApsarasX · 2022-03-24T13:38:39Z

That looks rather nice, thanks!

I will do a few refactors, hold back while I commit 😄

In addition, I found a problem about utils.createMatcher. The return value of utils.createMatcher([]) will match arbitrary string.

Josh-Cena · 2022-03-24T13:40:45Z

Ah yes, I think matching none seems like better semantics. Will fix it in this commit

ApsarasX · 2022-03-26T07:04:06Z

Is there any problem with this PR?

Josh-Cena · 2022-03-26T07:10:25Z

Nope, it looks good to me. But new public APIs have to go through @slorber, who unfortunately will be on holiday next. Sorry @ApsarasX you probably have to wait a bit longer than usual.

slorber · 2022-04-06T13:44:02Z

Thanks 👍

Not 100% convinced that ignorePatterns is future-proof (compared to just ignore) in case we want a callback later, but that wouldn't be a very complex breaking change for users if we change our mind so let's move on for now 👍

feat(plugin-sitemap): add ignorePatterns option

54b16f5

ApsarasX requested review from slorber, lex111 and Josh-Cena as code owners March 24, 2022 05:30

facebook-github-bot added the CLA Signed Signed Facebook CLA label Mar 24, 2022

Josh-Cena reviewed Mar 24, 2022

View reviewed changes

packages/docusaurus-plugin-sitemap/src/__tests__/options.test.ts Outdated Show resolved Hide resolved

slorber reviewed Mar 24, 2022

View reviewed changes

packages/docusaurus-plugin-sitemap/src/createSitemap.ts Outdated Show resolved Hide resolved

slorber requested changes Mar 24, 2022

View reviewed changes

Josh-Cena added the pr: new feature This PR adds a new API or behavior. label Mar 24, 2022

refactor(plugin-sitemap): modify ignorePatterns option

6371064

Merge branch 'main' into feature/sitemap-ignore

dc06fec

Josh-Cena added 2 commits March 24, 2022 21:42

minor refactor

24bd1e8

docs

0a66920

Josh-Cena changed the title ~~feat(plugin-sitemap): add ignorePatterns option~~ feat(sitemap): add ignorePatterns option Mar 24, 2022

add more tests

645ec3c

dogfood

188b76b

Josh-Cena mentioned this pull request Mar 28, 2022

Pages with noindex meta still showing in sitemap #6247

Closed

7 tasks

Josh-Cena added the status: awaiting review This PR is ready for review, will be merged after maintainers' approval label Mar 30, 2022

slorber merged commit 103ea04 into facebook:main Apr 6, 2022

Josh-Cena removed the status: awaiting review This PR is ready for review, will be merged after maintainers' approval label Apr 6, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(sitemap): add ignorePatterns option #6979

feat(sitemap): add ignorePatterns option #6979

ApsarasX commented Mar 24, 2022

facebook-github-bot commented Mar 24, 2022

netlify bot commented Mar 24, 2022 •

edited

github-actions bot commented Mar 24, 2022 •

edited

facebook-github-bot commented Mar 24, 2022

Josh-Cena Mar 24, 2022 •

edited

slorber Mar 24, 2022 •

edited

Josh-Cena Mar 24, 2022 •

edited

slorber Mar 24, 2022

ApsarasX Mar 24, 2022 •

edited

slorber Mar 24, 2022

Josh-Cena Mar 24, 2022

slorber left a comment

Josh-Cena commented Mar 24, 2022

ApsarasX commented Mar 24, 2022

Josh-Cena commented Mar 24, 2022

ApsarasX commented Mar 26, 2022

Josh-Cena commented Mar 26, 2022

slorber commented Apr 6, 2022

feat(sitemap): add ignorePatterns option #6979

feat(sitemap): add ignorePatterns option #6979

Conversation

ApsarasX commented Mar 24, 2022

Motivation

Have you read the Contributing Guidelines on pull requests?

Test Plan

Related PRs

facebook-github-bot commented Mar 24, 2022

Action Required

Process

netlify bot commented Mar 24, 2022 • edited

✅ [V2]

github-actions bot commented Mar 24, 2022 • edited

facebook-github-bot commented Mar 24, 2022

Josh-Cena Mar 24, 2022 • edited

Choose a reason for hiding this comment

slorber Mar 24, 2022 • edited

Choose a reason for hiding this comment

Josh-Cena Mar 24, 2022 • edited

Choose a reason for hiding this comment

slorber Mar 24, 2022

Choose a reason for hiding this comment

ApsarasX Mar 24, 2022 • edited

Choose a reason for hiding this comment

slorber Mar 24, 2022

Choose a reason for hiding this comment

Josh-Cena Mar 24, 2022

Choose a reason for hiding this comment

slorber left a comment

Choose a reason for hiding this comment

Josh-Cena commented Mar 24, 2022

ApsarasX commented Mar 24, 2022

Josh-Cena commented Mar 24, 2022

ApsarasX commented Mar 26, 2022

Josh-Cena commented Mar 26, 2022

slorber commented Apr 6, 2022

netlify bot commented Mar 24, 2022 •

edited

github-actions bot commented Mar 24, 2022 •

edited

Josh-Cena Mar 24, 2022 •

edited

slorber Mar 24, 2022 •

edited

Josh-Cena Mar 24, 2022 •

edited

ApsarasX Mar 24, 2022 •

edited