-
-
Notifications
You must be signed in to change notification settings - Fork 8.6k
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): add onUntruncatedBlogPosts
blog options
#10375
Conversation
✅ [V2]
To edit notification comments on pull requests, go to your Netlify site configuration. |
⚡️ Lighthouse report for the deploy preview of this PR
|
Size Change: -604 B (-0.01%) Total Size: 11.5 MB
ℹ️ View Unchanged
|
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 almost good but I'd prefer to move the error emission later in the process and improve DX
function reportTruncateMarkerProblem({ | ||
blogSourceRelative, | ||
truncateMarker, | ||
options, | ||
content, | ||
}: { | ||
content: string; | ||
truncateMarker: RegExp; | ||
blogSourceRelative: string; | ||
options: Pick<PluginOptions, 'onUntruncatedBlogPost'>; | ||
}): void { | ||
if (!truncateMarker.test(content)) { | ||
logger.report(options.onUntruncatedBlogPost)( | ||
logger.interpolate`Blog post path=${blogSourceRelative} is not truncated.`, | ||
); | ||
} | ||
} |
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.
We shouldn't run this Regex against the content twice: this can be expensive and is useless work. We already run it later in the blog post processing to compute the hasTruncateMarker
meta attribute.
Note that in the future we'll probably handle that with Remark (the RegExp is the historical solution).
I'd prefer if:
- we emit the warning based on
blogPost.metadata.hasTruncateMarker
- we emit a single report for all the blog posts
- we explain in the message that this can be turned off with the blog
onUntruncatedBlogPost: 'ignore'
option. Many sites run with untruncated blogs on purpose, let's give them a better upgrading experience if warn is the default.
The function should rather be something like this:
function reportTruncateMarkerProblem({
blogPosts,
onUntruncatedBlogPost,
}: {
blogPosts: BlogPost[],
onUntruncatedBlogPost: PluginOptions['onUntruncatedBlogPost']
}): void {
const untruncatedBlogPosts = blogPosts.filter(p => !p.metadata.hasTruncateMarker)
if (onUntruncatedBlogPost !== "ignore" && onUntruncatedBlogPost.length > 0) {
const message = `Blabla there are untruncated blog posts:
${untruncatedBlogPosts.map(getPath).join('\n- ')}
Blabla additional help`
logger.report(options.onUntruncatedBlogPost)(message);
}
}
onUntruncatedBlogPost
blog optionsonUntruncatedBlogPost
blog options
onUntruncatedBlogPost
blog optionsonUntruncatedBlogPosts
blog options
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.
LGTM 👍
Motivation
Fix #10289
Using
onUntruncatedBlogPosts: "throw"
is useful for blogs that want to ensure a consistent usage of truncation markers.By default, we emit a warning, but it's also possible to throw and fail a build:
The default is quite opinionated so the error message makes it clear how to disable it by using
onUntruncatedBlogPosts: 'ignore'
Test Plan
Unit test & ci
Test links
Deploy preview: https://deploy-preview-10375--docusaurus-2.netlify.app/docs/api/plugins/@docusaurus/plugin-content-blog/#onUntruncatedBlogPost