feat(content-docs, content-blog): global tags list file & better validation #6025

mambax · 2021-11-27T19:36:26Z

Addresses #5913

Motivation

Have you read the Contributing Guidelines on pull requests?

Yes.

Test Plan

Provide automated tests as in https://github.com/facebook/docusaurus/pull/5396/files

- Tests missing yet - Logic is not 100% as Josh-Cena asked for (its too complex for me tbh) - Some code duplicates can be refactored - Docs missing?

netlify · 2021-11-27T19:43:11Z

✔️ [V2]
Built without sensitive environment variables

🔨 Explore the source changes: b611ce1

🔍 Inspect the deploy log: https://app.netlify.com/sites/docusaurus-2/deploys/61a288bbe9c51b0007be8fda

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

github-actions · 2021-11-27T19:43:20Z

⚡️ Lighthouse report for the changes in this PR:

Category	Score
🟢 Performance	93
🟢 Accessibility	98
🟢 Best practices	100
🟢 SEO	100
🟢 PWA	95

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

Josh-Cena · 2021-12-31T00:58:18Z

Hey, I'll work on this for the coming week and hopefully make it ready for merge

Signed-off-by: Joshua Chen <sidachen2003@gmail.com>

mambax · 2022-01-11T10:22:35Z

After discussion with @Josh-Cena I set this to Ready for review.

slorber

Thanks, that looks like a great first working version 👍

I left a few comments and a few changes that we should probably discuss

The PR mention docs but I don't see any impl for the docs plugin yet: it's probably better to do the impl in both plugins at once for consistency.

Also need some documentation :)

slorber · 2022-01-12T17:51:22Z

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

+  return Joi.attempt(content, TagsMapSchema);
+}
+
+export async function getTagsMap(params: {


shouldn't this be in "utils" if we want it for docs too?

I't fine. It's a very lightweight wrapper around a utils function anyways.

I think I meant we'd need this in the docs plugin too?

slorber · 2022-01-12T17:51:57Z

packages/docusaurus-utils/src/tags.ts

    return {
      label: tagString,
      permalink: kebabCase(tagString),
    };
  }

+  function normalizeTagObject(tag: Partial<Tag>) {
+    return {
+      label: tag.label!,


in which case there's no label here? maybe the type is wrong?

Yeah, the label is the only required value. I just don't feel like it's useful to override one key to required🤦‍♂️

slorber · 2022-01-12T17:54:24Z

packages/docusaurus-utils/src/tags.ts


 export type Tag = {
  label: string;
  permalink: string;
 };

-export type FrontMatterTag = string | Tag;
+export type FrontMatterTag = string | Partial<Tag>;
+export type TagsMap = Record<string, Partial<Tag>>;


IMHO the Tag object shouldn't be partial.

Maybe we should normalize the tagsMap on load so that referenced tags contain at least a label

Maybe create a type like "TagConfig" ?

The problem is that we may add more stuff in the future like description, permalink, etc. I feel like it's valuable to make it remain partial, because maybe not everything makes sense to have a default?

@Josh-Cena I don't imagine any case where a tag without label make sense.

At the end of the day, we must display that tag somewhere in the UI, so there should always be at least a label

slorber · 2022-01-12T17:57:12Z

website/blog/2018-04-30-How-I-Converted-Profilo-To-Docusaurus.md

@@ -4,7 +4,7 @@ author: Christine Abernathy
 authorURL: http://twitter.com/abernathyca
 authorImageURL: https://github.com/caabernathy.png
 authorTwitter: abernathyca
-tags: [profilo, adoption]
+tags: [{label: profilo}, adoption]


do we really want to enable such syntax?

IMHO the ability to pass a permalink inline (and any other shred config that we might introduce later) does not make much sense as it leads to potential non-deterministic behavior when 2 tags are using 2 different inline configs due to bad copy-pasting.

IMHO we should deprecate (or breaking change) the usage of permalink and just support a simple array: (Label | Key)[]

I know that we support inline blog authors with config, but unlike inline authors, tags always have a dedicated page (inline or not) so we probably always force enforce a unique tag config here.

Josh-Cena · 2022-01-18T12:29:08Z

@slorber The current design is like: there are two modes, I would call them "lax mode" and "strict mode". If there isn't a tags.yml file, the app is in "lax mode" (the legacy mode) and every string is a tag label. If there is a tags map file, every string is a tag key and non-recognized keys will cause errors.

However, I'd like to retain the ability to have inline tags in case it's not worth to declare a special key for this, maybe because the tag will only ever be used once. If you don't think this feature is worth keeping I'm fine to remove it, but then we would have a too bloated tags.yml file.

slorber · 2022-01-27T17:15:11Z

However, I'd like to retain the ability to have inline tags in case it's not worth to declare a special key for this, maybe because the tag will only ever be used once. If you don't think this feature is worth keeping I'm fine to remove it, but then we would have a too bloated tags.yml file.

I don't like this idea, particularly because 2 inline tags with the same key can have conflicting data

I'd rather be strict by default but fail-safe: emit a warning for tags not found in the yml file, and convert them to {label: tagKey} so that it can still render

Unlike blog posts and inline authors, tags always have a page with a permalink and SEO data.

It's fine for a blog post to have the same author twice with different descriptions, maybe because the role or image of the author is updated over time and it's a snapshot.

I don't think it's the case for tags where you'd rather be consistent. Reconciling/merging data for 2 similar inline tags seems risky and non-deterministic.

You may ask: what if we later introduce a page for each blog author => Similarly, I think we'll only create pages for declared authors, not inline ones

This comment has been minimized.

Sign in to view

Initial version of tags

e5dca8c

- Tests missing yet - Logic is not 100% as Josh-Cena asked for (its too complex for me tbh) - Some code duplicates can be refactored - Docs missing?

This comment has been minimized.

Sign in to view

facebook-github-bot added the CLA Signed Signed Facebook CLA label Nov 27, 2021

This comment has been minimized.

Sign in to view

Josh-Cena changed the title ~~DRAFT: Predefined tag list~~ feat(content-docs, content-blog): global tags list file & better validation Dec 3, 2021

Josh-Cena added the pr: new feature This PR adds a new API or behavior. label Dec 3, 2021

Josh-Cena linked an issue Dec 3, 2021 that may be closed by this pull request

Predefined tag lists per plugin/preset #5913

Closed

2 tasks

Josh-Cena added 2 commits December 18, 2021 18:15

Merge branch 'main' into mambax/main

ab55b62

Merge branch 'main' into mambax/main

fdcf380

Josh-Cena mentioned this pull request Dec 31, 2021

refactor(utils): reorganize functions; move authors file resolution to utils #6229

Merged

Josh-Cena added 6 commits December 31, 2021 12:01

Merge branch 'main' into mambax/main

39365e4

revert

704075e

Complete function

1c25a65

Fix tests

973023c

Merge branch 'main' into mambax/main

7cec3c1

Signed-off-by: Joshua Chen <sidachen2003@gmail.com>

Merge branch 'main' into mambax/main

68734d8

mambax marked this pull request as ready for review January 11, 2022 10:22

mambax requested review from Josh-Cena, lex111 and slorber as code owners January 11, 2022 10:22

slorber requested changes Jan 12, 2022

View reviewed changes

Josh-Cena requested a review from slorber January 18, 2022 12:22

mambax closed this by deleting the head repository Apr 9, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(content-docs, content-blog): global tags list file & better validation #6025

feat(content-docs, content-blog): global tags list file & better validation #6025

mambax commented Nov 27, 2021 •

edited

Loading

This comment has been minimized.

netlify bot commented Nov 27, 2021

github-actions bot commented Nov 27, 2021 •

edited

Loading

This comment has been minimized.

This comment has been minimized.

Josh-Cena commented Dec 31, 2021 •

edited

Loading

mambax commented Jan 11, 2022

slorber left a comment

slorber Jan 12, 2022

Josh-Cena Jan 18, 2022

slorber Jan 27, 2022

slorber Jan 12, 2022

Josh-Cena Jan 18, 2022

slorber Jan 12, 2022

Josh-Cena Jan 18, 2022

slorber Jan 27, 2022

slorber Jan 12, 2022

Josh-Cena commented Jan 18, 2022

slorber commented Jan 27, 2022 •

edited

Loading

feat(content-docs, content-blog): global tags list file & better validation #6025

feat(content-docs, content-blog): global tags list file & better validation #6025

Conversation

mambax commented Nov 27, 2021 • edited Loading

Motivation

Have you read the Contributing Guidelines on pull requests?

Test Plan

This comment has been minimized.

netlify bot commented Nov 27, 2021

github-actions bot commented Nov 27, 2021 • edited Loading

This comment has been minimized.

This comment has been minimized.

Josh-Cena commented Dec 31, 2021 • edited Loading

mambax commented Jan 11, 2022

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Josh-Cena commented Jan 18, 2022

slorber commented Jan 27, 2022 • edited Loading

mambax commented Nov 27, 2021 •

edited

Loading

github-actions bot commented Nov 27, 2021 •

edited

Loading

Josh-Cena commented Dec 31, 2021 •

edited

Loading

slorber commented Jan 27, 2022 •

edited

Loading