Skip to content
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(mdx-loader): Remark plugin to report unused MDX / Markdown directives #9394

Merged
merged 27 commits into from Oct 24, 2023

Conversation

OzakIOne
Copy link
Collaborator

@OzakIOne OzakIOne commented Oct 12, 2023

You get a warning?

If you get a warning while running your site, read this section to understand how to fix it.

TLDR:

  • escape directives with an extra \ to make them render as plain text
  • example \:myDirective

Problem

Docusaurus v3 uses the remark-directive plugin to implement Markdown Directives support.

3 sort of Markdown directives exists:

  • Text Directives (:text)
  • Leaf Directives (::leaf)
  • Container Block Directives (:::container)

This is how Docusaurus implements admonitions such as :::note or :::danger.

These directives also allow you to extend Markdown's capabilities, enabling custom functionality by providing simple Remark plugins.

Problem: if your docs already contains text like :abc or ::xyz, you may inadvertently use a directive, and it may not render like you expect

Solution

To avoid the inadvertent usage of directive you can escape the : character by adding a backslash before it, or add a space after it

image

Examples:

  • :text ➡️ \:text or : text
  • ::leaf ➡️ \::leaf or :: leaf
  • :::container ➡️ \:::container or ::: container

Motivation

This plugin aims to warn users of the usage of unused directives, to prevent mistakes and to be sure their documents keep rendering as before after upgrading to Docusaurus v3.

Test Plan

yarn jest unusedDirectives

Test links

Deploy preview: https://deploy-preview-9394--docusaurus-2.netlify.app

Related issues/PRs

remarkjs/remark-directive#12
https://talk.commonmark.org/t/generic-directives-plugins-syntax/444
https://github.com/remarkjs/remark-directive
https://docusaurus.io/docs/next/markdown-features/admonitions

@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Oct 12, 2023
@netlify
Copy link

netlify bot commented Oct 12, 2023

[V2]

Name Link
🔨 Latest commit 65406ac
🔍 Latest deploy log https://app.netlify.com/sites/docusaurus-2/deploys/6537a52f6885690008e456be
😎 Deploy Preview https://deploy-preview-9394--docusaurus-2.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

@github-actions
Copy link

github-actions bot commented Oct 12, 2023

⚡️ Lighthouse report for the deploy preview of this PR

URL Performance Accessibility Best Practices SEO PWA Report
/ 🟢 94 🟢 97 🟢 92 🟢 100 🟠 89 Report
/docs/installation/ 🟠 80 🟢 98 🟢 92 🟢 100 🟠 89 Report
/docs/category/getting-started/ 🟢 92 🟢 100 🟢 92 🟢 90 🟠 89 Report
/blog/ 🟠 87 🟢 100 🟢 92 🟢 90 🟠 89 Report
/blog/preparing-your-site-for-docusaurus-v3/ 🟠 77 🟢 97 🟢 92 🟢 100 🟠 89 Report
/blog/tags/release/ 🟠 83 🟢 100 🟢 92 🟠 80 🟠 89 Report
/blog/tags/ 🟠 88 🟢 100 🟢 92 🟢 90 🟠 89 Report

@slorber slorber added the pr: new feature This PR adds a new API or behavior. label Oct 12, 2023
@slorber slorber added this to the 3.0 milestone Oct 12, 2023
const warningMessage = unusedDirectives
.map((unusedDirective) => {
const positionMessage = unusedDirective.position
? logger.interpolate`number=${unusedDirective.position.line}:number=${unusedDirective.position.column}`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note, I'm not sure if IDEs can provide navigation if the line numbers are surrounded by ANSI sequences.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now we agreed navigation was not a goal because we need to use relative paths anyway otherwise CI tests would fail due to different absolute paths in snapshots.

And navigation apparently doesn't work in VSCode with relative paths.

Copy link
Collaborator Author

@OzakIOne OzakIOne Oct 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tested on VSCode and in fact there is no problem opening the file from terminal even if the path is relative or surrounded by ANSI sequences (colors in the terminal am I right?)

VSCode seems to support multiple path syntax, full path, relative with or without ./ (e.g.: ./website/README.md or website/README.md)

I've tested on WebStorm but it doesn't seem to have this feature (ctrl+click) to open file in editor (not a WebStorm user so I didn't dig too much)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, VS Code supports opening files with paths relative to the current project. I think it even supports arbitrary paths, as long as the name is unambiguous (otherwise it shows a dropdown chooser). For example foo.tsx can open to src/components/foo.tsx.

@slorber slorber changed the title feat(mdx-loader): unused directives plugin feat(mdx-loader): Remark plugin to report unused MDX / Markdown directives Oct 13, 2023
@slorber slorber marked this pull request as ready for review October 24, 2023 09:24
@slorber slorber requested a review from lex111 as a code owner October 24, 2023 09:24
@slorber slorber added the Argos Add this label to run UI visual regression tests. See argos.yml GH action. label Oct 24, 2023
@argos-ci
Copy link

argos-ci bot commented Oct 24, 2023

The latest updates on your projects. Learn more about Argos notifications ↗︎

Build Status Details Updated (UTC)
default (Inspect) ✅ No change detected - Oct 24, 2023, 11:20 AM

@slorber slorber merged commit c6762a2 into facebook:main Oct 24, 2023
30 of 31 checks passed
slorber added a commit that referenced this pull request Oct 24, 2023
…tives (#9394)

Co-authored-by: sebastienlorber <lorber.sebastien@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Argos Add this label to run UI visual regression tests. See argos.yml GH action. CLA Signed Signed Facebook CLA pr: new feature This PR adds a new API or behavior.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants