feat(blog): warn duplicate and inline authors #10224
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: +75 B (0%) Total Size: 1.85 MB
ℹ️ View Unchanged
|
| ); | ||
| } | ||
|
|
||
| // TODO need title check otherwise reports weird cases |
There was a problem hiding this comment.
Reports authors from changelog, maybe there is a better way to ignore that
| const inlineAuthors = updatedAuthors.filter((author) => author.inline); | ||
|
|
||
| const duplicateList = updatedAuthors.filter( | ||
| (author, index, self) => | ||
| index !== self.findIndex((t) => t.name === author.name), | ||
| ); | ||
|
|
||
| // TODO need title check otherwise reports weird cases | ||
| if (inlineAuthors.length > 0 && params.frontMatter.title) { | ||
| logger.warn( | ||
| `Inline authors found in blog [${ | ||
| params.frontMatter.title | ||
| }] ${inlineAuthors.map((author) => author.name).join(', ')}`, | ||
| ); | ||
| } |
There was a problem hiding this comment.
We need an option onInlineAuthors, it must be configurable because most blogs use inline authors atm.
// TODO need title check otherwise reports weird cases
What are those cases exactly? We need to handle those and cover with unit test edge cases
Overall the code should look quite similar to the tags code:
export function reportInlineTags({
tags,
source,
options,
}: {
tags: TagMetadata[];
source: string;
options: TagsPluginOptions;
}): void {
if (options.onInlineTags === 'ignore') {
return;
}
const inlineTags = tags.filter((tag) => tag.inline);
if (inlineTags.length > 0) {
const uniqueUnknownTags = [...new Set(inlineTags.map((tag) => tag.label))];
const tagListString = uniqueUnknownTags.join(', ');
logger.report(options.onInlineTags)(
`Tags [${tagListString}] used in ${source} are not defined in ${
options.tags ?? 'tags.yml'
}`,
);
}
}I'd prefer if the side effect (logging) wasn't performed inside getAuthors but as a separate standalone dedicated method, tested independently.
const authors = getBlogPostAuthors({authorsMap, frontMatter, baseUrl});
reportAuthorProblems(paramsYouNeed)If logging requires more info about the blog (like the title/source/whatever) you can inject more as params.
| `Duplicate authors found in blog post ${params.frontMatter.title} [${ | ||
| params.frontMatter.slug | ||
| }] front matter: ${duplicateList | ||
| .map((author) => author.name) | ||
| .join(', ')}`, |
There was a problem hiding this comment.
front matter might have no title, no slug, and author might have no name
All these cases should be covered with unit tests
| const duplicateList = updatedAuthors.filter( | ||
| (author, index, self) => | ||
| index !== self.findIndex((t) => t.name === author.name), | ||
| ); |
There was a problem hiding this comment.
Author might have no name
What happens if we have 2 authors with just an image? Are they considered as duplicate?
What happens if one predefined author has key "seb" and name "Seb", and there's another inline author named "Seb", are they considered as duplicate?
The current implementation is not good enough, and the method to detect duplicates is complex enough to deserve its own unit tests
There was a problem hiding this comment.
This case should work and not warn
https://deploy-preview-10224--docusaurus-2.netlify.app/tests/blog/2022/01/20/image-only-authors/
|
@OzakIOne, can you point me to the logic that computes the unique permalink for each author from the author name? As I pointed you in another post, in my fork this logic required quite an elaborate code, to account for diacritics which must be replaced by latin letter and possible parenthesis like 'Liviu Ionescu (ilg)', which I would expect it transformed into 'liviu-ionescu-ilg'. |
For now the logic is very basic (just a check of name), I implemented it quickly in #10216 then extracted it in this PR, the logic of a duplicated author needs to be defined. |
Ok, then I suggest you take a closer look at my implementation. |
Or, could you consider making the logic that generates the permalink for authors configurable, for example via a hook? |
Pre-flight checklist
Motivation
Warn the user of inline authors and also if duplicated authors were found
Test Plan
Unit test + ci
Test links
Unit tests and deploy
Deploy preview: https://deploy-preview-10224--docusaurus-2.netlify.app/
Related issues/PRs
Authors page