feat(v2): enable feeds by default in blog plugin #3842

cindygu4 · 2020-11-29T21:55:43Z

Motivation

As #3720 said, Docusaurus 2 extracts the blog into a separate plugin which means that blogs aren't enabled by default. However, feeds should probably be enabled by default in blog plugin.

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

Locally tested using yarn build:v2 to see if /blog/rss.xml and /blog/atom.xml exist when no type is specified. Created 2 unit tests in pluginOptionSchema.test.ts and passed all unit tests.

Related PRs

No

facebook-github-bot · 2020-11-29T21:55:52Z

Hi @cindygu4!

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file.

In order for us to review and merge your code, 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.

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

netlify · 2020-11-29T22:04:43Z

✔️ Deploy preview for docusaurus-2 ready!
Built without sensitive environment variables

🔨 Explore the source changes: 0e25e50

🔍 Inspect the deploy logs: https://app.netlify.com/sites/docusaurus-2/deploys/5fce73e329a32a0007acd389

😎 Browse the preview: https://deploy-preview-3842--docusaurus-2.netlify.app

facebook-github-bot · 2020-11-29T22:07:55Z

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

nickserv

We may want to make the type property optional in packages/docusaurus-plugin-content-blog/src/types.ts, depending on how this plugin gets used with/without the schema. We should also update the documentation in website/docs/blog.md and website/docs/using-plugins.md (and possibly the versioned copies, though I'm not sure how those updates are handled in this repository).

nickserv · 2020-11-30T08:39:35Z

packages/docusaurus-plugin-content-blog/src/__tests__/pluginOptionSchema.test.ts

+ expect(value).toEqual({
+ ...userOptions,
+ feedOptions: {type: ['rss', 'atom']},
+ });


Shouldn't this be added to DEFAULT_OPTIONS or userOptions?

I believe I tried what you suggested when trying to resolve the issue with this unit test. However, it won't pass the test unless we add feedOptions: {type: ['rss', 'atom']} here

why exactly this test does not pass?

I can't tell you if I can't see the code, but what I'm sure of is that we 100% want to put default options in the DEFAULT_OPTIONS constant

I've now added type: 'all' into DEFAULT_OPTIONS.feedOptions and this test passes. I think it would still make sense to have this line of code here since it would convert type: 'all' into the array (['rss', 'atom']) right?

@cindygu4 I'd rather have type: ['rss', 'atom'] and allow the validation to work with arrays syntax, so that the default options are not "transformed"

Thanks for the suggestion @slorber! @vikhramt007 and I have implemented the feature using the type: ['rss', 'atom'] as the default and allowed the validation to work with arrays. It now passes all of the unit tests :)

slorber

Thanks, almost looks good

Use DEFAULT_OPTIONS to be consistent with the way we handle other default options

slorber · 2020-11-30T14:13:05Z

packages/docusaurus-plugin-content-blog/src/pluginOptionSchema.ts

+ })
+ .empty(undefined)
+ .empty(null)
+ .default(['rss', 'atom']),


Should use DEFAULT_OPTIONS.feedOptions.type, similarly to how we handle default options in other places

Hi,

I've tried setting DEFAULT_OPTIONS.feedOptions.type to 'all' as well as ['rss', 'atom'] and both give me these errors

Also, setting DEFAULT_OPTIONS.feedOptions.type to ['rss', 'atom'] also gives me an additional error [ValidationError: "feedOptions.type" does not match any of the allowed types].

Do you have any suggestions as to how to resolve these? The only way I've found to make it work is by adding the empty() and default() methods within the Joi object.

Thanks!

I can't debug remotely code that I can't fully see, You'd rather commit your failed attempt and ask a new review.

Also, setting DEFAULT_OPTIONS.feedOptions.type to ['rss', 'atom'] also gives me an additional error [ValidationError: "feedOptions.type" does not match any of the allowed types].

You should make the validator accept an array for this to work, currently it only accept strings

Ok yeah, I'll make another commit with the failed attempt.

slorber · 2020-11-30T14:18:24Z

Also: if we enable these feeds by default, is it still possible to disable them?
Can we be sure that passing null or something can actually disable the feeds with a test?

As @nickmccurdy mentioned we should also add/update appropriate doc for this

committing a failed attempt for review

nickserv · 2020-11-30T16:31:16Z

Thanks, but it looks like the docs are still outdated (see #3842 (review))

- allow validation to work with arrays syntax - using DEFAULT_OPTIONS.feedOptions.type instead

github-actions · 2020-12-04T02:31:33Z

⚡️ Lighthouse report for the changes in this PR:

Category	Score
🟠 Performance	57
🟢 Accessibility	99
🟢 Best practices	100
🟢 SEO	100
🟢 PWA	95

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

github-actions · 2020-12-04T02:40:07Z

Size Change: +16 B (0%)

Total Size: 154 kB

ℹ️ View Unchanged

Filename	Size	Change
`website/build/blog/2017/12/14/introducing-docusaurus/index.html`	20.7 kB	-2 B (0%)
`website/build/docs/introduction/index.html`	180 B	0 B
`website/build/index.html`	5.82 kB	-1 B
`website/build/main.********.js`	109 kB	+19 B (0%)
`website/build/styles.********.css`	17.5 kB	0 B

_{compressed-size-action}

- added documentation for feedOptions

slorber

Thanks, that looks good to me, just a few details left to ensure that we can still disable the feeds

slorber · 2020-12-07T15:43:36Z

packages/docusaurus-plugin-content-blog/src/__tests__/pluginOptionSchema.test.ts

+ feedOptions: {type: ['rss', 'atom'], title: 'title'},
+ });
+});
+


That looks good

Can you please add a test to ensure that type: null leads to type: null after validation? To ensure it's not using default values as a fallback?

slorber · 2020-12-07T15:44:15Z

website/docs/blog.md

@@ -89,11 +89,11 @@ Or this.

 ## Feed

-You can generate RSS/ Atom feed by passing feedOptions.
+You can generate RSS/ Atom feed by passing feedOptions. By default, RSS and Atom feeds are generated.


As we now generate feeds by default, can you document how to disable the feed generation?

Merging master to check unit tests

- modified implementation to allow feeds to be disable without error - added unit test to ensure type: null leads to type: null after validation - added documentation to explain how to disable feed generation

slorber · 2020-12-10T17:35:12Z

thanks, LGTM 👍

seems it was not a bad idea to add a test for disabling the feed 😉

feat: enables feeds by default in blog

38af5f4

cindygu4 requested review from lex111 and slorber as code owners November 29, 2020 21:55

facebook-github-bot added the CLA Signed Signed Facebook CLA label Nov 29, 2020

cindygu4 changed the title ~~feat(v2): enables feeds by default in blog plugin~~ feat(v2): enable feeds by default in blog plugin Nov 29, 2020

nickserv suggested changes Nov 30, 2020

View reviewed changes

slorber requested changes Nov 30, 2020

View reviewed changes

feat(v2): enable feeds by default

4f3199a

committing a failed attempt for review

cindygu4 requested a review from slorber November 30, 2020 15:27

cindygu4 added 2 commits December 3, 2020 21:23

feat(v2): enable feeds in blog plugin

7f18351

feat(v2): enable feeds by default in blog plugin

9d2fe88

- allow validation to work with arrays syntax - using DEFAULT_OPTIONS.feedOptions.type instead

feat(v2): enable feeds by default in blog plugin

612687f

- added documentation for feedOptions

slorber requested changes Dec 7, 2020

View reviewed changes

cindygu4 added 2 commits December 7, 2020 12:26

feat(v2): enable feeds by default

7ce4fec

Merging master to check unit tests

feat(v2): enable feeds by default in blog plugin

0e25e50

- modified implementation to allow feeds to be disable without error - added unit test to ensure type: null leads to type: null after validation - added documentation to explain how to disable feed generation

cindygu4 requested a review from slorber December 7, 2020 18:40

slorber added the pr: new feature This PR adds a new API or behavior. label Dec 10, 2020

slorber merged commit 0b05806 into facebook:master Dec 10, 2020

slorber linked an issue Dec 10, 2020 that may be closed by this pull request

V2: Enable feeds by default in blog plugin #3720

Closed

slorber mentioned this pull request Dec 10, 2020

V2: Enable feeds by default in blog plugin #3720

Closed

slorber added the pr: breaking change Existing sites may not build successfully in the new version. Description contains more details. label Dec 10, 2020

renchap mentioned this pull request Apr 6, 2021

Enable RSS & Atom feeds react-navigation/react-navigation.github.io#966

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(v2): enable feeds by default in blog plugin #3842

feat(v2): enable feeds by default in blog plugin #3842

cindygu4 commented Nov 29, 2020

facebook-github-bot commented Nov 29, 2020

netlify bot commented Nov 29, 2020 •

edited

Loading

facebook-github-bot commented Nov 29, 2020

nickserv left a comment •

edited

Loading

nickserv Nov 30, 2020

slorber Nov 30, 2020

cindygu4 Nov 30, 2020

slorber Nov 30, 2020

cindygu4 Nov 30, 2020 •

edited

Loading

slorber Nov 30, 2020

cindygu4 Dec 4, 2020

slorber left a comment

slorber Nov 30, 2020

cindygu4 Nov 30, 2020 •

edited

Loading

slorber Nov 30, 2020

cindygu4 Nov 30, 2020 •

edited

Loading

slorber commented Nov 30, 2020

nickserv commented Nov 30, 2020

github-actions bot commented Dec 4, 2020 •

edited

Loading

github-actions bot commented Dec 4, 2020 •

edited

Loading

slorber left a comment

slorber Dec 7, 2020

slorber Dec 7, 2020

slorber commented Dec 10, 2020

feat(v2): enable feeds by default in blog plugin #3842

feat(v2): enable feeds by default in blog plugin #3842

Conversation

cindygu4 commented Nov 29, 2020

Motivation

Have you read the Contributing Guidelines on pull requests?

Test Plan

Related PRs

facebook-github-bot commented Nov 29, 2020

netlify bot commented Nov 29, 2020 • edited Loading

facebook-github-bot commented Nov 29, 2020

nickserv left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cindygu4 Nov 30, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

slorber left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cindygu4 Nov 30, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cindygu4 Nov 30, 2020 • edited Loading

Choose a reason for hiding this comment

slorber commented Nov 30, 2020

nickserv commented Nov 30, 2020

github-actions bot commented Dec 4, 2020 • edited Loading

github-actions bot commented Dec 4, 2020 • edited Loading

slorber left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

slorber commented Dec 10, 2020

netlify bot commented Nov 29, 2020 •

edited

Loading

nickserv left a comment •

edited

Loading

cindygu4 Nov 30, 2020 •

edited

Loading

cindygu4 Nov 30, 2020 •

edited

Loading

cindygu4 Nov 30, 2020 •

edited

Loading

github-actions bot commented Dec 4, 2020 •

edited

Loading

github-actions bot commented Dec 4, 2020 •

edited

Loading